Alan Modra [Thu, 15 Dec 2022 12:37:10 +0000 (23:07 +1030)]
Re: The problem with warning in elf_object_p
Commit 5aa0f10c424e added a per_xvec_warn array to provide support for
warnings from elf_object_p (and a later patch for warnings from
pe_bfd_object_p) to be cached and then only printed if the target
matches. It was quite limited in the style of message supported, only
one message could be printed, and didn't really meet the stated aim of
only warning when a target matches: There are many other errors and
warnings that can be emitted by functions called from elf_object_p.
So this patch extends the error handler functions to support printing
to a string buffer, extends per_xvec_warn to support multiple errors/
warnings, and hooks this all into bfd_check_format_matches. If
bfd_check_format_matches succeeds then any errors/warnings are printed
for the matching target. If bfd_check_format_matches fails either due
to no match or to multiple matches and only one target vector produced
errors, then those errors are printed.
* bfd.c (MAX_ARGS): Define, use throughout.
(print_func): New typedef.
(_bfd_doprnt): Add new print param. Replace calls to fprintf
with print.
(PRINT_TYPE): Similarly.
(error_handler_fprintf): Renamed from error_handler_internal.
Use _bfd_get_error_program_name. Add fprintf arg. Move code
setting up args..
(_bfd_doprnt_scan): ..to here. Add ap param.
(struct buf_stream): New.
(err_sprintf): New function.
(error_handler_bfd): New static variable.
(error_handler_sprintf): New function.
(_bfd_set_error_handler_caching): New function.
(_bfd_get_error_program_name): New function.
* elfcode.h (elf_swap_shdr_in): Use _bfd_error_handler in
warning messages.
(elf_object_p): Likewise.
* format.c (print_warnmsg): New function.
(clear_warnmsg): Rewrite.
(null_error_handler): New function.
(bfd_check_format_matches): Ignore warnings from recursive calls
checking first element of an archive. Use caching error handler
otherwise. Print warnings on successful match, or when only one
target has emitted warnings/errors.
* peicode.h (pe_bfd_object_p): Use _bfd_error_handler in
warning messages.
* targets.c (per_xvec_warn): Change type of array elements.
(struct per_xvec_message): New.
(_bfd_per_xvec_warn): Rewrite.
* Makefile.am (LIBBFD_H_FILES): Add bfd.c.
* Makefile.in: Regenerate.
* bfd-in2.h: Regenerate.
* libbfd.h: Regenerate.
Indu Bhagat [Sat, 17 Dec 2022 06:02:07 +0000 (22:02 -0800)]
gas: sframe: testsuite: add testcase for .cfi_negate_ra_state
Add a new test to check that .cfi_negate_ra_state on aarch64 is handled
well (a non-empty SFrame section with valid SFrame FREs is generated).
ChangeLog:
* testsuite/gas/cfi-sframe/cfi-sframe-aarch64-2.d: New test.
* testsuite/gas/cfi-sframe/cfi-sframe-aarch64-2.s: Likewise.
* testsuite/gas/cfi-sframe/cfi-sframe.exp: Adjust the list
accordingly.
Indu Bhagat [Sat, 17 Dec 2022 06:02:02 +0000 (22:02 -0800)]
objdump/readelf: sframe: emit marker for FREs with mangled RA
In the textual dump of the SFrame section, when an SFrame FRE recovers a
mangled RA, use string "[s]" in the output to indicate that the return
address is a signed (mangled) one.
ChangeLog:
* libsframe/sframe-dump.c (dump_sframe_func_with_fres): Postfix
with "[s]" if RA is signed with authorization code.
Indu Bhagat [Sat, 17 Dec 2022 06:01:40 +0000 (22:01 -0800)]
sframe.h: add support for .cfi_negate_ra_state
Use the last remaining bit in the 'SFrame FRE info' word to store whether
the RA is signed/unsigned with PAC authorization code: this bit is named
as the "mangled RA" bit. This bit is still unused for x86-64.
The behaviour of the mangled-RA info bit in SFrame format closely
follows the behaviour of DW_CFA_AARCH64_negate_ra_state in DWARF. During
unwinding, whenever an SFrame FRE with non-zero "mangled RA" bit is
encountered, it means the upper bits of the return address contain Pointer
Authentication code. The unwinder, hence, must use appropriate means to
restore LR correctly in such cases.
include/ChangeLog:
* sframe.h (SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P): New macro.
(SFRAME_V1_FRE_MANGLED_RA_P): Likewise.
Pedro Alves [Fri, 16 Dec 2022 12:04:54 +0000 (12:04 +0000)]
Delay checking whether /proc/pid/mem is writable (PR gdb/29907)
As of 1bcb0708f229 ("gdb/linux-nat: Check whether /proc/pid/mem is
writable"), GDB checks if /proc/pid/mem is writable. This is done
early at GDB startup, in order to get a consistent warning, instead of
a warning that depends on whenever GDB writes to inferior memory.
PR gdb/29907 points out that some build systems (like QEMU's,
apparently) may call 'gdb --version' to check GDB's presence & its
version on the system, and that Gentoo's build process has sandboxing
which blocks the /proc/pid/mem access and thus GDB warns, which
results in build fails.
To help with that, this patch delays the /proc/pid/mem check until we
start or attach to an inferior. Ends up potentially emiting a warning
close where we already emit other ptrace- and /proc- related warnings,
which just Feels Right.
Tom de Vries [Fri, 16 Dec 2022 14:28:27 +0000 (15:28 +0100)]
[gdb/testsuite] Fix race in gdb.threads/detach-step-over.exp
Once in a while I run into:
...
FAIL: gdb.threads/detach-step-over.exp: \
breakpoint-condition-evaluation=host: target-non-stop=off: non-stop=off: \
displaced=off: iter 1: all threads running
...
In can easily reproduce this by doing:
...
# Wait a bit, to give time for the threads to hit the
# breakpoint.
- sleep 1
return true
...
Fix this by counting the running threads in a loop, effectively allowing 10
seconds (instead of 1) for the threads to start running, but only sleeping if
needed.
Andrew Burgess [Mon, 12 Dec 2022 14:05:22 +0000 (14:05 +0000)]
gdb: fix crash when getting the value of a label symbol
When the source program contains a goto label, it turns out it's
actually pretty hard for a user to find out more about that label.
For example:
(gdb) p some_label
No symbol "some_label" in current context.
(gdb) disassemble some_label
No symbol "some_label" in current context.
(gdb) x/10i some_label
No symbol "some_label" in current context.
(gdb) break some_label
Breakpoint 2 at 0x401135: file /tmp/py-label-symbol-value.c, line 35.
In all cases, some_label is a goto label within the current frame.
Only placing a breakpoint on the label worked.
This all seems a little strange to me, it feels like asking about a
goto label would not be an unreasonable thing for a user to do.
This commit doesn't fix any of the above issues, I mention them just
to provide a little context for why the following issue has probably
not been seen before.
It turns out there is one way a user can access the symbol for a goto
label, through the Python API:
The problem is that label symbols are created using the
builtin_core_addr type, which is a pure integer type.
When GDB tries to fetch the value of a label symbol then we end up in
findvar.c, in the function language_defn::read_var_value, in the
LOC_LABEL case. From here store_typed_address is called to store the
address of the label into a value object with builtin_core_addr type.
The problem is that store_typed_address requires that the destination
type be a pointer or reference, which the builtin_core_addr type is
not.
Now it's not clear what type a goto label address should have, but
GCC has an extension that allows users to take the address of a goto
label (using &&), in that case the result is of type 'void *'.
I propose that when we convert the CORE_ADDR value to a GDB value
object, we use builtin_func_ptr type instead of builtin_core_addr,
this means the result will be of type 'void (*) ()'. The benefit of
this approach is that when gdbarch_address_to_pointer is called the
target type will be correctly identified as a pointer to code, which
should mean any architecture specific adjustments are done correctly.
We can then cast the new value to 'void *' type with a call to
value_cast_pointer, this should not change the values bit
representation, but will just update the type.
After this asking for the value of a label symbol works just fine:
Simon Marchi [Thu, 20 Oct 2022 01:56:47 +0000 (21:56 -0400)]
gdbsupport: add string_xml_appendf
Add a version of buffer_xml_printf (defined in gdbsupport/buffer.{c,h})
that appends to an std::string, rather than a struct buffer. Call it
"string" rather than "buffer" since it operates on an std::string rather
than a buffer. And call it "appendf" rather than "printf", since it
appends to and does not replace the string's content. This mirrors
string_appendf.
Place the new version in gdbsupport/xml-utils.h.
The code is a direct copy of buffer_xml_printf. The old version is
going to disappear at some point, which is why I didn't do any effort to
share code.
Change-Id: I30e030627ab4970fd0b9eba3b7e8cec78fa561ba Approved-By: Pedro Alves <pedro@palves.net>
Jan Vrany [Fri, 16 Dec 2022 11:38:28 +0000 (11:38 +0000)]
gdb/testsuite: add test for Python commands redefining itself
This commit adds a test that creates a Python command that redefines
itself during its execution. This is to test use-after-free in
execute_command ().
This test needs run with ASan enabled in order to fail when it
should.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Luis Machado [Tue, 24 May 2022 22:31:09 +0000 (23:31 +0100)]
[aarch64] Fix removal of non-address bits for PAuth
PR gdb/28947
The address_significant gdbarch setting was introduced as a way to remove
non-address bits from pointers, and it is specified by a constant. This
constant represents the number of address bits in a pointer.
Right now AArch64 is the only architecture that uses it, and 56 was a
correct option so far.
But if we are using Pointer Authentication (PAuth), we might use up to 2 bytes
from the address space to store the required information. We could also have
cases where we're using both PAuth and MTE.
We could adjust the constant to 48 to cover those cases, but this doesn't
cover the case where GDB needs to sign-extend kernel addresses after removal
of the non-address bits.
This has worked so far because bit 55 is used to select between kernel-space
and user-space addresses. But trying to clear a range of bits crossing the
bit 55 boundary requires the hook to be smarter.
The following patch renames the gdbarch hook from significant_addr_bit to
remove_non_address_bits and passes a pointer as opposed to the number of
bits. The hook is now responsible for removing the required non-address bits
and sign-extending the address if needed.
While at it, make GDB and GDBServer share some more code for aarch64 and add a
new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.
Jan Beulich [Fri, 16 Dec 2022 08:01:14 +0000 (09:01 +0100)]
gas: restore Dwarf info generation after macro diagnostic adjustments
While 6fdb723799e2 ("gas: re-work line number tracking for macros and
their expansions") was meant to leave generated Dwarf as is, it really
didn't (and the testcase intended to catch that wasn't covering the case
which broke). Its adjustment to buffer_and_nest() didn't go far enough,
leading to the "linefile" directive inserted at the top to also be
processed later in the PR gas/16908 workaround (which clearly isn't
intended - it's being put there for processing during macro expansion
only). That unnoticed flaw in turn led me to work around it by a
(suspicious to me already at the time) conditional in as_where().
Jan Beulich [Fri, 16 Dec 2022 08:00:23 +0000 (09:00 +0100)]
x86: change representation of extension opcode
Having a "None" field in the vast majority of entries is needlessly
cluttering the overall table. Instead of this being a separate field,
use a representation matching that of Intel SDM and AMD PM for the main
use of the field: Append the value after a / as the separator.
Simon Marchi [Thu, 15 Dec 2022 19:06:25 +0000 (14:06 -0500)]
gdb: remove static buffer in command_line_input
[I sent this earlier today, but I don't see it in the archives.
Resending it through a different computer / SMTP.]
The use of the static buffer in command_line_input is becoming
problematic, as explained here [1]. In short, with this patch [2] that
attempt to fix a post-hook bug, when running gdb.base/commands.exp, we
hit a case where we read a "define" command line from a script file
using command_command_line_input. The command line is stored in
command_line_input's static buffer. Inside the define command's
execution, we read the lines inside the define using command_line_input,
which overwrites the define command, in command_line_input's static
buffer. After the execution of the define command, execute_command does
a command look up to see if a post-hook is registered. For that, it
uses a now stale pointer that used to point to the define command, in
the static buffer, causing a use-after-free. Note that the pointer in
execute_command points to the dynamically-allocated buffer help by the
static buffer in command_line_input, not to the static object itself,
hence why we see a use-after-free.
Fix that by removing the static buffer. I initially changed
command_line_input and other related functions to return an std::string,
which is the obvious but naive solution. The thing is that some callees
don't need to return an allocated string, so this this an unnecessary
pessimization. I changed it to passing in a reference to an std::string
buffer, which the callee can use if it needs to return
dynamically-allocated content. It fills the buffer and returns a
pointers to the C string inside. The callees that don't need to return
dynamically-allocated content simply don't use it.
So, it started with modifying command_line_input as described above, all
the other changes derive directly from that.
One slightly shady thing is in handle_line_of_input, where we now pass a
pointer to an std::string's internal buffer to readline's history_value
function, which takes a `char *`. I'm pretty sure that this function
does not modify the input string, because I was able to change it (with
enough massaging) to take a `const char *`.
A subtle change is that we now clear a UI's line buffer using a
SCOPE_EXIT in command_line_handler, after executing the command.
This was previously done by this line in handle_line_of_input:
/* We have a complete command line now. Prepare for the next
command, but leave ownership of memory to the buffer . */
cmd_line_buffer->used_size = 0;
Indu Bhagat [Thu, 15 Dec 2022 21:12:01 +0000 (13:12 -0800)]
libsframe asan: avoid generating misaligned loads
There are two places where unaligned loads were seen on aarch64:
- #1. access to the SFrame FRE stack offsets in the in-memory
representation/abstraction provided by libsframe.
- #2. access to the SFrame FRE start address in the on-disk representation
of the frame row entry.
For #1, we can fix this by reordering the struct members of
sframe_frame_row_entry in libsframe/sframe-api.h.
For #2, we need to default to using memcpy instead, and copy out the bytes
to a location for output.
SFrame format is an unaligned on-disk format. As such, there are other blobs
of memory in the on-disk SFrame FRE that are on not on their natural
boundaries. But that does not pose further problems yet, because the users
are provided access to the on-disk SFrame FRE data via libsframe's
sframe_frame_row_entry, the latter has its' struct members aligned on their
respective natural boundaries (and initialized using memcpy).
PR 29856 libsframe asan: load misaligned at sframe.c:516
ChangeLog:
PR libsframe/29856
* bfd/elf64-x86-64.c: Adjust as the struct members have been
reordered.
* libsframe/sframe.c (sframe_decode_fre_start_address): Use
memcpy to perform 16-bit/32-bit reads.
* libsframe/testsuite/libsframe.encode/encode-1.c: Adjust as the
struct members have been reordered.
include/ChangeLog:
PR libsframe/29856
* sframe-api.h: Reorder fre_offsets for natural alignment.
Tom Tromey [Wed, 14 Dec 2022 19:28:32 +0000 (12:28 -0700)]
Move streq and compare_cstrings to gdbsupport
It seems to me that streq and compare_cstrings belong near the other
string utility functions in common-utils.h; and furthermore that streq
ought to be inlined. This patch makes this change.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Wed, 14 Dec 2022 14:17:44 +0000 (14:17 +0000)]
gdb: make more use of make_target_connection_string
I noticed that we have a function make_target_connection_string which
wraps all the logic for creating a string that describes a target
connection - but in some places we are not calling this function,
instead we duplicate the function's logic.
This commit cleans this up, and calls make_target_connection_string
where possible.
There should be no user visible changes after this commit.
Tom de Vries [Thu, 15 Dec 2022 11:30:36 +0000 (12:30 +0100)]
[gdb/testsuite] Fix gdb.base/condbreak-multi-context.exp with gcc 4.8.5
With gcc 4.8.5, I run into:
...
Running gdb.base/condbreak-multi-context.exp ...
gdb compile failed, condbreak-multi-context.cc:21:11: warning: non-static \
data member initializers only available with -std=c++11 or -std=gnu++11 \
[enabled by default]
int b = 20;
^
...
Fix this by making it a static const.
Tested on x86_64-linux, with gcc 4.8.5, 7.5.0 and clang 13.0.1.
Andrew Burgess [Thu, 22 Sep 2022 17:11:30 +0000 (18:11 +0100)]
gdb: ensure all targets are popped before an inferior is destructed
Now that the inferiors target_stack automatically manages target
reference counts, we might think that we don't need to unpush targets
when an inferior is deleted...
...unfortunately that is not the case. The inferior::unpush function
can do some work depending on the type of target, so it is important
that we still pass through this function.
To ensure that this is the case, in this commit I've added an assert
to inferior::~inferior that ensures the inferior's target_stack is
empty (except for the ever present dummy_target).
I've then added a pop_all_targets call to delete_inferior, otherwise
the new assert will fire in, e.g. the gdb.python/py-inferior.exp test.
Andrew Burgess [Thu, 22 Sep 2022 11:22:22 +0000 (12:22 +0100)]
gdb: remove the pop_all_targets (and friends) global functions
This commit removes the global functions pop_all_targets,
pop_all_targets_above, and pop_all_targets_at_and_above, and makes
them methods on the inferior class.
As the pop_all_targets functions will unpush each target, which
decrements the targets reference count, it is possible that the target
might be closed.
Right now, closing a target, in some cases, depends on the current
inferior being set correctly, that is, to the inferior from which the
target was popped.
To facilitate this I have used switch_to_inferior_no_thread within the
new methods. Previously it was the responsibility of the caller to
ensure that the correct inferior was selected.
In a couple of places (event-top.c and top.c) I have been able to
remove a previous switch_to_inferior_no_thread call.
In remote_unpush_target (remote.c) I have left the
switch_to_inferior_no_thread call as it is required for the
generic_mourn_inferior call.
Andrew Burgess [Thu, 22 Sep 2022 18:10:52 +0000 (19:10 +0100)]
gdb: remove decref_target
The decref_target function is not really needed. Calling
target_ops::decref will just redirect to decref_target anyway, so why
not just rename decref_target to target_ops::decref?
That's what this commit does.
It's not exactly renaming to target_ops::decref, because the decref
functionality is handled by a policy class, so the new name is now
target_ops_ref_policy::decref.
There should be no user visible change after this commit.
Andrew Burgess [Thu, 22 Sep 2022 10:17:39 +0000 (11:17 +0100)]
gdb: have target_stack automate reference count handling
This commit changes the target_stack class from using a C style array
of 'target_ops *' to using a C++ std::array<target_ops_ref, ...>. The
benefit of this change is that some of the reference counting of
target_ops objects is now done automatically.
This commit fixes a crash in gdb.python/py-inferior.exp where GDB
crashes at exit, leaving a core file behind.
The crash occurs in connpy_connection_dealloc, and is actually
triggered by this assert:
gdb_assert (conn_obj->target == nullptr);
Now a little aside...
... the assert is never actually printed, instead GDB crashes due
to calling a pure virtual function. The backtrace at the point of
crash looks like this:
#7 0x00007fef7e2cf747 in std::terminate() () from /lib64/libstdc++.so.6
#8 0x00007fef7e2d0515 in __cxa_pure_virtual () from /lib64/libstdc++.so.6
#9 0x0000000000de334d in target_stack::find_beneath (this=0x4934d78, t=0x2bda270 <the_dummy_target>) at ../../s>
#10 0x0000000000df4380 in inferior::find_target_beneath (this=0x4934b50, t=0x2bda270 <the_dummy_target>) at ../.>
#11 0x0000000000de2381 in target_ops::beneath (this=0x2bda270 <the_dummy_target>) at ../../src/gdb/target.c:3047
#12 0x0000000000de68aa in target_ops::supports_terminal_ours (this=0x2bda270 <the_dummy_target>) at ../../src/gd>
#13 0x0000000000dde6b9 in target_supports_terminal_ours () at ../../src/gdb/target.c:1112
#14 0x0000000000ee55f1 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_li>
Notice in frame #12 we called target_ops::supports_terminal_ours,
however, this is the_dummy_target, which is of type dummy_target,
and so we should have called dummy_target::supports_terminal_ours.
I believe the reason we ended up in the wrong implementation of
supports_terminal_ours (which is a virtual function) is because we
made the call during GDB's shut-down, and, I suspect, the vtables
were in a weird state.
Anyway, the point of this patch is not to fix GDB's ability to
print an assert during exit, but to address the root cause of the
assert. With that aside out of the way, we can return to the main
story...
Connections are represented in Python with gdb.TargetConnection
objects (or its sub-classes). The assert in question confirms that
when a gdb.TargetConnection is deallocated, the underlying GDB
connection has itself been removed from GDB. If this is not true then
we risk creating multiple different gdb.TargetConnection objects for
the same connection, which would be bad.
To ensure that we have one gdb.TargetConnection object for each
connection, the all_connection_objects map exists, this maps the
process_stratum_target object (the connection) to the
gdb.TargetConnection object that represents the connection.
When a connection is removed in GDB the connection_removed observer
fires, which we catch with connpy_connection_removed, this function
then sets conn_obj->target to nullptr, and removes the corresponding
entry from the all_connection_objects map.
The first issue here is that connpy_connection_dealloc is being called
as part of GDB's exit code, which is run after the Python interpreter
has been shut down. The connpy_connection_dealloc function is used to
deallocate the gdb.TargetConnection Python object. Surely it is
wrong for us to be deallocating Python objects after the interpreter
has been shut down.
The reason why connpy_connection_dealloc is called during GDB's exit
is that the global all_connection_objects map is still holding a
reference to the gdb.TargetConnection object. When the map is
destroyed during GDB's exit, the gdb.TargetConnection objects within
the map can finally be deallocated.
The reason why all_connection_objects has contents when GDB exits, and
the reason the assert fires, is that, when GDB exits, there are still
some connections that have not yet been removed from GDB, that is,
they have a non-zero reference count.
If we take a look at quit_force (top.c) you can see that, for each
inferior, we call pop_all_targets before we (later in the function)
call do_final_cleanups. It is the do_final_cleanups call that is
responsible for shutting down the Python interpreter. The
pop_all_targets calls should, in theory, cause all the connections to
be removed from GDB.
That this isn't working indicates that some targets have a non-zero
reference count even after this final pop_all_targets call, and
indeed, when I debug GDB, that is what I see.
I tracked the problem down to delete_inferior where we do some house
keeping, and then delete the inferior object, which calls
inferior::~inferior.
In neither delete_inferior or inferior::~inferior do we call
pop_all_targets, and it is this missing call that means we leak some
references to the target_ops objects on the inferior's target_stack.
In this commit I will provide a partial fix for the problem. I say
partial fix, but this will actually be enough to resolve the crash.
In a later commit I will provide the final part of the fix.
As mentioned at the start of the commit message, this commit changes
the m_stack in target_stack to hold target_ops_ref objects. This
means that when inferior::~inferior is called, and m_stack is
released, we automatically decrement the target_ops reference count.
With this change in place we no longer leak any references, and now,
in quit_force the final pop_all_targets calls will release the final
references. This means that the targets will be correctly closed at
this point, which means the connections will be removed from GDB and
the Python objects deallocated before the Python interpreter shuts
down.
There's a slight oddity in target_stack::unpush, where we std::move
the reference out of m_stack like this:
auto ref = std::move (m_stack[stratum]);
the `ref' isn't used explicitly, but it serves to hold the
target_ops_ref until the end of the scope while allowing the m_stack
entry to be reset back to nullptr. The alternative would be to
directly set the m_stack entry to nullptr, like this:
m_stack[stratum] = nullptr;
The problem here is that when we set the m_stack entry to nullptr we
first decrement the target_ops reference count, and then set the array
entry to nullptr.
If the decrement means that the target_ops object reaches a zero
reference count then the target_ops object will be closed by calling
target_close. In target_close we ensure that the target being closed
is not in any inferiors target_stack.
As we decrement before clearing, then this check in target_close will
fail, and an assert will trigger.
By using std::move to move the reference out of m_stack, this clears
the m_stack entry, meaning the inferior no longer contains the
target_ops in its target_stack. Now when the REF object goes out of
scope and the reference count is decremented, target_close can run
successfully.
I've made use of the Python connection_removed listener API to add a
test for this issue. The test installs a listener and then causes
delete_inferior to be called, we can then see that the connection is
then correctly removed (because the listener triggers).
Andrew Burgess [Thu, 22 Sep 2022 17:57:27 +0000 (18:57 +0100)]
gdb/remote: remove some manual reference count handling
While working on some other target_ops reference count related code, I
spotted that in remote.c we do some manual reference count handling,
i.e. we call target_ops::incref and decref_target (which wraps
target_ops::decref).
I think it would be better to make use of gdb::ref_ptr to automate the
reference count management.
So, this commit updates scoped_mark_target_starting in two ways,
first, I use gdb::ref_ptr to handle the reference counts. Then,
instead of using the scoped_mark_target_starting constructor and
destructor to set and reset the starting_up flag, I now use a
scoped_restore_tmpl object to set and restore the flag.
The above changes mean that the scoped_mark_target_starting destructor
can be completely removed, and the constructor body is now empty.
I've also fixed a typo in the class comment.
The only change in behaviour after this commit is that previously we
didn't care what the value of starting_up was, we just set it to true
in the constructor and false in the destructor.
Now, I assert that the flag is initially false, then set the flag true
when the scoped_mark_target_starting is created.
As the starting_up flag is initialized to false then, for the assert
to fire, we would need to recursively enter
remote_target::start_remote_1, which I don't think is something we
should be doing, so I think the new assert is an improvement.
Andrew Burgess [Tue, 6 Dec 2022 12:49:55 +0000 (12:49 +0000)]
gdb: add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT
After the previous commit converted symbol-lookup debug to use the new
debug scheme, this commit adds SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT.
The previous commit didn't add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT
because symbol-lookup debug is controlled by an 'unsigned int' rather
than a 'bool' control variable, we use the numeric value to offer
different levels of verbosity for symbol-lookup debug.
The *_SCOPED_DEBUG_ENTER_EXIT mechanism currently relies on capturing
a reference to the bool control variable, and evaluating the variable
both on entry, and at exit, this is done in the scoped_debug_start_end
class (see gdbsupport/common-debug.h).
This commit templates scoped_debug_start_end so that the class can
accept either a 'bool &' or an invokable object, e.g. a lambda
function, or a function pointer.
The existing scoped_debug_start_end and scoped_debug_enter_exit macros
in common-debug.h are updated to support scoped_debug_enter_exit being
templated, however, nothing outside of common-debug.h needs to change.
I've then added SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT in symtab.h, and
added a couple of token uses in symtab.c. I didn't want to add too
much in this first commit, this is really about updating
common-debug.h to support this new functionality.
Within symtab.h I created a couple of global functions that can be
used to query the status of the symbol_lookup_debug control variable,
these functions are then used within the two existing macros:
Andrew Burgess [Fri, 2 Dec 2022 12:10:10 +0000 (12:10 +0000)]
gdb: convert 'set debug symbol-lookup' to new debug printing scheme
Convert the implementation of 'set debug symbol-lookup' to the new
debug printing scheme.
In a few places I've updated the debug output to remove places where
the printed debug message included the function name, the new debug
scheme already adds that, but I haven't done all the possible updates.
Which resolves PR gdb/29105. My reason for writing this test was a
desire to better understand the above commit, my process was to study
the commit until I thought I understood it, then write a test to
expose the issue. As the original commit didn't have a test, I
thought it wouldn't hurt to commit this upstream.
The problem tested for here is already described in the above commit,
but I'll give a brief description here. This description describes
GDB prior to the above commit:
- Builtin types are added to GDB using their canonical name,
e.g. "short", not "signed short",
- When the user does something like 'p sizeof(short)', then this is
handled in c-exp.y, and results in a call to lookup_signed_type
for the name "int". The "int" here is actually being looked up as
the type for the result of the 'sizeof' expression,
- In lookup_signed_type GDB first adds a 'signed' and looks for that
type, so in this case 'signed int', and, if that lookup fails, GDB
then looks up 'int',
- The problem is that 'signed int' is not the canonical name for a
signed int, so no builtin type with that name will be found, GDB
will then go to each object file in turn looking for a matching
type,
- When checking each object file, GDB will first check the partial
symtab to see if the full symtab should be expanded or not.
Remember, at this point GDB is looking for 'signed int', there
will be no partial symbols with that name, so GDB will not expand
anything,
- However, GDB checks each partial symbol using multiple languages,
not just the current language (C in this case), so, when GDB
checks using the C++ language, the symbol name is first
canonicalized (the code that does this can be found
lookup_name_info::language_lookup_name). As the canonical form of
'signed int' is just 'int', GDB then looks for any symbols with
the name 'int', most partial symtabs will contain such a symbol,
so GDB ends up expanding pretty much every symtab.
The above commit fixes this by avoiding the use of non-canonical names
with C, now the initial builtin type lookup will succeed, and GDB
never even considers whether to expand any additional symtabs.
The test case creates a library that includes char, short, int, and
long types, and a test program that links against the library.
In the test script we start the inferior, but don't allow it to
progress far enough that the debug information for the library has
been fully expanded yet.
Then we evaluate some 'sizeof(TYPE)' expressions.
In the buggy version of GDB this would cause the debug information
for the library to be fully expanded, while in the fixed version of
GDB this will not be the case.
We use 'info sources' to determine if the debug information has been
fully expanded or not.
Remove some "OBJF_READNOW" code from dwarf2_debug_names_index
The testsuite checks if GDB was started with the -readnow flag by
using the 'maintenance print objfiles' command, and looking for the
string 'faked for "readnow"' in the output. This is implemented in
two helper procs `readnow` (gdb.exp) and `mi_readnow` (mi-support.exp).
The following tests all currently depend on this detection:
The following test also includes detection of 'readnow', but does the
detection itself by checking $::GDBFLAGS for the readnow flag:
gdb.opt/break-on-_exit.exp
The above commit removed from GDB the code that produced the 'faked
for "readnow"' string, as a consequence the testsuite can no longer
correctly spot when readnow is in use, and many of the above tests
will fail (at least partially).
When looking at the above tests, I noticed that gdb.rust/traits.exp
does call `readnow`, but doesn't actually use the result, so I've
removed the readnow call, this simplifies the next part of this patch
as gdb.rust/traits.exp was the only place an extra regexp was passed
to the readnow call.
Next I have rewritten `readnow` to check the $GDBFLAGS for the
-readnow flag, and removed the `maintenance print objfiles` check. At
least for all the tests above, when using the readnow board, this is
good enough to get everything passing again.
For the `mi_readnow` proc, I changed this to just call `readnow` from
gdb.exp, I left the mi_readnow name in place - in the future it might
be the case that we want to do some different checks here.
Finally, I updated gdb.opt/break-on-_exit.exp to call the `readnow`
proc.
With these changes, all of the tests listed above now pass correctly
when using the readnow board.
Li Xu [Wed, 14 Dec 2022 07:32:40 +0000 (07:32 +0000)]
RISC-V: Add string length check for operands in AS
The current AS accepts invalid operands due to miss of operands length check.
For example, "e6" is an invalid operand in (vsetvli a0, a1, e6, mf8, tu, ma),
but it's still accepted by assembler. In detail, the condition check "strncmp
(array[i], *s, len) == 0" in arg_lookup function passes with "strncmp ("e64",
"e6", 2)" in the case above. So the generated encoding is same as that of
(vsetvli a0, a1, e64, mf8, tu, ma).
This patch fixes issue above by prompting an error in such case and also adds
a new testcase.
gas/ChangeLog:
* config/tc-riscv.c (arg_lookup): Add string length check for operands.
* testsuite/gas/riscv/vector-insns-fail-vsew.d: New testcase for an illegal vsew.
* testsuite/gas/riscv/vector-insns-fail-vsew.l: Likewise.
* testsuite/gas/riscv/vector-insns-fail-vsew.s: Likewise.
Jan Beulich [Wed, 14 Dec 2022 09:07:44 +0000 (10:07 +0100)]
x86: adjust type checking constructs
As Alan points out, ASAN takes issue with these constructs, for
current_templates being NULL. Wrap them in sizeof(), so the expressions
aren't actually evaluated.
Alan Modra [Wed, 14 Dec 2022 01:44:13 +0000 (12:14 +1030)]
asan: signed integer overflow in display_debug_frames
* dwarf.c (struct Frame_Chunk): Make col_offset an int64_t.
Adjust all places allocating col_offset and col_type to use
the size of the array element rather than the size of a type.
(frame_display_row): Adjust printing of col_offset.
(display_debug_frames): Factor out multiplication by
code_factor and data_factor. Avoid signed overflow. Use
64-bit variables.
Tom Tromey [Mon, 5 Dec 2022 17:56:23 +0000 (10:56 -0700)]
Fix control-c handling on Windows
As Hannes pointed out, the Windows target-async patches broke C-c
handling there. Looking into this, I found a few oddities, fixed
here.
First, windows_nat_target::interrupt calls GenerateConsoleCtrlEvent.
I think this event can be ignored by the inferior, so it's not a great
way to interrupt. Instead, using DebugBreakProcess (or a more
complicated thing for Wow64) seems better.
Second, windows_nat_target did not implement the pass_ctrlc method.
Implementing this lets us remove the special code to call
SetConsoleCtrlHandler and instead integrate into gdb's approach to C-c
handling. I believe that this should also fix the race that's
described in the comment that's being removed.
Initially, I thought a simpler version of this patch would work.
However, I think what happens is that some other library (I'm not sure
what) calls SetConsoleCtrlHandler while gdb is running, and this
intercepts and handles C-c -- so that the gdb SIGINT handler is not
called. C-break continues to work, presumably because whatever
handler is installed ignores it.
This patch works around this issue by ensuring that the gdb handler
always comes first.
Tom Tromey [Mon, 5 Dec 2022 18:15:09 +0000 (11:15 -0700)]
Refactor code to check for terminal sharing
This refactors the code to check for terminal sharing.
is_gdb_terminal is exported, and sharing_input_terminal_1 is renamed,
slightly refactored, and moved to posix-hdep.c. A new
Windows-specific implementation of this function is added to
mingw-hdep.c.
MSDN has a warning about GetConsoleProcessList
This API is not recommended and does not have a virtual terminal
equivalent. [...] Applications remoting via cross-platform
utilities and transports like SSH may not work as expected if
using this API.
However, we believe this isn't likely to be an issue for gdb.
Tom Tromey [Mon, 12 Dec 2022 17:17:18 +0000 (10:17 -0700)]
Use gdb::optional for sigint_ours
sigint_ours (and sigquit_ours) can be used without being set. Avoid
this problem by changing them to gdb::optional and checking that they
are in fact set before using the value.
On s390x-linux, I run into:
...
(gdb) continue^M
Continuing.^M
breakpoint.c:5784: internal-error: bpstat_stop_status_nowatch: \
Assertion `!target_stopped_by_watchpoint ()' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: \
breakpoint after the first fork (GDB internal error)
...
What happens is the follow:
- a watchpoint event triggers
- the event is processed, s390_linux_nat_target::stopped_by_watchpoint is called and
it returns true, as expected
- the watchpoint event is reported by gdb, and gdb stops
- we issue a continue command
- a fork event triggers
- the event is processed, and during processing that event
s390_linux_nat_target::stopped_by_watchpoint is called again, and returns
true
- the assertion fails, because the function is expected to return false
The function s390_linux_nat_target::stopped_by_watchpoint returns true the
second time, because it looks at the exact same data that was looked at when
it was called the first time, and that data hasn't changed.
There's code in the same function that intends to prevent that from happening:
...
/* Do not report this watchpoint again. */
memset (&per_lowcore, 0, sizeof (per_lowcore));
if (ptrace (PTRACE_POKEUSR_AREA, s390_inferior_tid (), &parea, 0) < 0)
perror_with_name (_("Couldn't clear watchpoint status"));
...
and that probably used to work for older kernels, but no longer does since
linux kernel commit 5e9a26928f55 ("[S390] ptrace cleanup").
Fix this by copying this:
...
siginfo_t siginfo;
if (!linux_nat_get_siginfo (inferior_ptid, &siginfo))
return false;
if (siginfo.si_signo != SIGTRAP
|| (siginfo.si_code & 0xffff) != TRAP_HWBKPT)
return false;
...
from aarch64_linux_nat_target::stopped_data_address and remove the
obsolete watchpoint status clearing code.
Tom Tromey [Tue, 13 Dec 2022 15:58:50 +0000 (08:58 -0700)]
Remove two unnecessary casts
A couple of calls to parse_probe_linespec had an unnecessary cast. I
suspect this cast was never needed, but once commands were changed to
take a 'const' argument, they became completely obsolete. Tested by
rebuilding.
gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang
The gdb.trace/unavailable-dwarf-piece.exp test script was creating a
temporary file in the build/gdb/testsuite/ directory, instead of in
the expected place in the outputs directory.
Fix this by adding a call to standard_output_file.
Jan Beulich [Tue, 13 Dec 2022 08:11:53 +0000 (09:11 +0100)]
gas: re-work line number tracking for macros and their expansions
The PR gas/16908 workaround aimed at uniformly reporting line numbers
to reference macro invocation sites. As mentioned in a comment this may
be desirable for small macros, but often isn't for larger ones. As a
first step improve diagnostics to report both locations, while aiming at
leaving generated debug info unaltered.
Note that macro invocation context is lost for any diagnostics issued
only after all input was processed (or more generally for any use of
as_*_where(), as the functions can't know whether the passed in location
is related to [part of] the present stack of locations). To maintain the
intended workaround behavior for PR gas/16908, a new as_where() is
introduced to "look through" macro invocations, while the existing
as_where() is renamed (and used in only very few places for now). Down
the road as_where() will likely want to return a list of (file,line)
pairs.
Jan Beulich [Tue, 13 Dec 2022 08:11:16 +0000 (09:11 +0100)]
Arm: avoid unhelpful uses of .macro in testsuite
Macros with just a single use site are a little pointless to have, and
even in further cases .irp is more suitable for the purpose. Expand such
inline, avoiding the need to touch the testcases when diagnostics are
changed for code resulting from macro expansion.
While there also make what was "iter_mla" in sp-usage-thumb2-relax cover
smlatt as well, rather than testing smlabt twice.
Tom Tromey [Sun, 11 Dec 2022 19:48:07 +0000 (12:48 -0700)]
Fix crash in is_nocall_function
is_nocall_function anticipates only being called for a function or a
method. However, PR gdb/29871 points out a situation where an unusual
expression -- but one that parses to a valid, if extremely weird,
function call -- breaks this assumption.
This patch changes is_nocall_function to remove this assert and
instead simply return 'false' in this case.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29871
Alan Modra [Mon, 12 Dec 2022 23:35:17 +0000 (10:05 +1030)]
asan: mips_hi16_list segfault in bfd_get_section_limit_octets
static variables like mips_hi16_list are nasty for applications using
bfd. It is possible when opening and closing bfds with mis-matched
hi/lo relocs to leave a stale section pointer on the list. That can
cause a segfault if multiple bfds are being processed.
Tidying the list when closing is sufficient to stop this happening
(and fixes small memory leaks). This patch goes further and moves
mips_hi16_list to where it belongs in the bfd tdata.
Indu Bhagat [Mon, 12 Dec 2022 18:41:06 +0000 (10:41 -0800)]
libctf: remove unnecessary zstd constructs
This patch is essentially a revert of
commit-id: 8818c80cbd4116ef5af171ec47c61167179e225c
(libctf: Add ZSTD_LIBS to LIBS so that ac_cv_libctf_bfd_elf can be true)
As the specific configure check now uses libtool, this explicit mention of the
dependency $ZSTD_LIBS is not needed anymore.
Tom de Vries [Mon, 12 Dec 2022 14:21:33 +0000 (15:21 +0100)]
[gdb/testsuite] Fix PR20630 regression test in gdb.base/printcmds.exp
On s390x-linux, I run into:
...
(gdb) print {unsigned char}{65}^M
$749 = 0 '\000'^M
(gdb) FAIL: gdb.base/printcmds.exp: print {unsigned char}{65}
...
In contrast, on x86_64-linux, we have:
...
(gdb) print {unsigned char}{65}^M
$749 = 65 'A'^M
(gdb) PASS: gdb.base/printcmds.exp: print {unsigned char}{65}
...
The first problem here is that the test is supposed to be a regression test
for PR20630, which can be reproduced (for an unfixed gdb) like this:
...
(gdb) p {unsigned char[]}{0x17}
gdbtypes.c:4641: internal-error: copy_type: \
Assertion `TYPE_OBJFILE_OWNED (type)' failed.
...
but it's not due to insufficient quoting (note the dropped '[]').
That's easy to fix, but after that we have on s390 (big endian):
...
(gdb) print {unsigned char[]}{65}^M
$749 = ""^M
...
and on x86_64 (little endian):
...
(gdb) print {unsigned char[]}{65}^M
$749 = "A"^M
...
Fix this by using 0xffffffff, such that in both cases we have:
...
(gdb) print {unsigned char[]}{0xffffffff}^M
$749 = "\377\377\377\377"^M
...
Tom Tromey [Mon, 12 Dec 2022 13:36:55 +0000 (06:36 -0700)]
Another Rust operator precedence bug
My earlier patch to fix PR rust/29859 introduced a new operator
precedence bug in the Rust parser. Assignment operators are
right-associative in Rust. And, while this doesn't often matter, as
Rust assignments always have the value (), still as a matter of
principle we should get this correct.
Tom de Vries [Mon, 12 Dec 2022 13:25:58 +0000 (14:25 +0100)]
[gdb/testsuite] Fix gdb.base/write_mem.exp for big endian
On s390x-linux (big endian), I run into:
...
(gdb) x /xh main^M
0x1000638 <main>: 0x0000^M
(gdb) FAIL: gdb.base/write_mem.exp: x /xh main
...
In contrast, on x86_64-linux (little endian), we have the expected:
...
(gdb) x /xh main^M
0x4004a7 <main>: 0x4242^M
(gdb) PASS: gdb.base/write_mem.exp: x /xh main
...
The problem is that the test-case hard-codes expectations about endiannes by
writing an int-sized value (4 bytes in this case) and then printing only a
halfword by using "/h" (so, two bytes).
If we print 4 bytes, we have for s390x:
...
0x1000638 <main>: 0x00004242^M
...
and for x86_64:
...
0x4004a7 <main>: 0x00004242^M
...
Jan Vrany [Mon, 12 Dec 2022 13:16:14 +0000 (13:16 +0000)]
gdb: fix possible use-after-free when executing commands
In principle, `execute_command()` does following:
struct cmd_list_element *c;
c = lookup_cmd ( ... );
...
/* If this command has been pre-hooked, run the hook first. */
execute_cmd_pre_hook (c);
...
/* ...execute the command `c` ...*/
...
execute_cmd_post_hook (c);
This may lead into use-after-free error. Imagine the command
being executed is a user-defined Python command that redefines
itself. In that case, struct `cmd_list_element` pointed to by
`c` is deallocated during its execution so it is no longer valid
when post hook is executed.
To fix this case, this commit looks up the command once again
after it is executed to get pointer to (possibly newly allocated)
`cmd_list_element`.
Jan Beulich [Mon, 12 Dec 2022 13:01:02 +0000 (14:01 +0100)]
x86: further re-work insn/suffix recognition to also cover MOVSX
PR gas/29524
Having templates with a suffix explicitly present has always been
quirky. After prior adjustment all that's left to also eliminate the
anomaly from move-with-sign-extend is to consolidate the insn templates
and to make may_need_pass2() cope (plus extend testsuite coverage).
Jan Beulich [Mon, 12 Dec 2022 12:53:40 +0000 (13:53 +0100)]
x86: drop (now) stray IsString
The need for them on the operand-less string insns has gone away with
the removal of maybe_adjust_templates() and associated logic. Since
i386_index_check() needs adjustment then anyway, take the opportunity
and also simplify it, possible again as a result of said removal (plus
the opcode template adjustments done here).
Jan Beulich [Mon, 12 Dec 2022 12:53:13 +0000 (13:53 +0100)]
x86: move bad-use-of-TLS-reloc check
Having it in match_template() is unhelpful. Neither does looking for the
next template to possibly match make any sense in that case, nor is the
resulting diagnostic making clear what the problem is.
While moving the check, also generalize it to include all SIMD and VEX-
encoded insns. This way an existing conditional can be re-used in
md_assemble(). Note though that this still leaves a lof of insns which
are also wrong to use with these relocations.
Further fold the remaining check (BFD_RELOC_386_GOT32) with the XRELEASE
related one a few lines down. This again allows re-using an existing
conditional.
Jan Beulich [Mon, 12 Dec 2022 12:52:47 +0000 (13:52 +0100)]
x86-64: allow HLE store of accumulator to absolute 32-bit address
In commit 1212781b35c9 ("ix86: allow HLE store of accumulator to
absolute address") I was wrong to exclude 64-bit code. Dropping the
check also leads to better diagnostics in 64-bit code ("MOV", after
all, isn't invalid with "XRELEASE").
While there also limit the amount of further checks done: The operand
type checks that were there were effectively redundant with other ones
anyway, plus it's quite fine to also have "xrelease mov <disp>, %eax"
look for the next MOV template (in fact again also improving
diagnostics).
Jan Beulich [Mon, 12 Dec 2022 12:52:15 +0000 (13:52 +0100)]
ix86: don't recognize/derive Q suffix in the common case
Have its use, except where actually legitimate, result in the same "only
supported in 64-bit mode" diagnostic as emitted for other 64-bit only
insns. Also suppress deriving of the suffix in Intel mode except in the
legitimate cases. This in exchange allows dropping the respective code
from match_template().
To maintain reasonable diagnostics (in particular to avoid "`mov' is
only supported in 64-bit mode" on the SIMD forms of MOVQ) we need to
defer parse_insn()'s emitting of errors unrelated to prefix parsing.
Utilize i.error just like match_template() does.
Oddly enough despite gcc's preference towards FILDQ and FIST{,T}Q we
had no testcase whatsoever for these. Therefore such tests are being
added. Note that the removed line in the x86-64-lfence-load testcase
was redundant with the exact same one a few lines up.
Jan Beulich [Mon, 12 Dec 2022 12:51:46 +0000 (13:51 +0100)]
x86: re-work insn/suffix recognition
Having templates with a suffix explicitly present has always been
quirky. Introduce a 2nd matching pass in case the 1st one couldn't find
a suitable template _and_ didn't itself already need to trim off a
suffix to find a match at all. This requires error reporting adjustments
(albeit luckily fewer than I was afraid might be necessary), as errors
previously reported during matching now need deferring until after the
2nd pass (because, obviously, we must not emit any error if the 2nd pass
succeeds). While also related to PR gas/29524, it was requested that
move-with-sign-extend be left as broken as it always was.
PR gas/29525
Note that with the dropped CMPSD and MOVSD Intel Syntax string insn
templates taking operands, mixed IsString/non-IsString template groups
(with memory operands) cannot occur anymore. With that
maybe_adjust_templates() becomes unnecessary (and is hence being
removed).
PR gas/29526
Note further that while the additions to the intel16 testcase aren't
really proper Intel syntax, we've been permitting all of those except
for the MOVD variant. The test therefore is to avoid re-introducing such
an inconsistency.
Jan Beulich [Mon, 12 Dec 2022 12:50:31 +0000 (13:50 +0100)]
x86: constify parse_insn()'s input
The function doesn't alter its input buffer: Reflect this in its
prototype. To avoid using any kind of cast, simply calculate the update
of "line" from the function's input and output.
Jan Beulich [Mon, 12 Dec 2022 12:49:56 +0000 (13:49 +0100)]
x86: revert disassembler parts of "x86: Allow 16-bit register source for LAR and LSL"
This reverts the disassembler parts of 859aa2c86dc9 ("x86: Allow 16-bit
register source for LAR and LSL"), adjusting testcases as necessary.
That change was itself a partial revert of c9f5b96bdab0 ("x86: correct
handling of LAR and LSL"), without actually saying so. While the earlier
commit was properly agreed upon, the partial revert was not, and hence
should not have been committed. This is even more so that the revert
part of that change wasn't even necessary to address PR gas/29844.
Jan Beulich [Mon, 12 Dec 2022 08:49:44 +0000 (09:49 +0100)]
gas/codeview: avoid "shadowing" of glibc function name
While not "index" this time, old enough glibc also has an (unguarded)
declaration of fileno() in stdio.h, which triggers a "shadows a global
declaration" warning with our choice of warning level and with at least
some gcc versions.
Jan Beulich [Mon, 12 Dec 2022 07:49:26 +0000 (08:49 +0100)]
x86: generate template sets data at build time
Speed up gas startup by avoiding runtime allocation of the instances of
type "templates". At the same time cut the memory requirement to just
very little over half (not even accounting for any overhead
notes_alloc() may incur) by reusing the "end" slot of a preceding entry
for the "start" slot of the subsequent one.
Jan Beulich [Mon, 12 Dec 2022 07:49:00 +0000 (08:49 +0100)]
x86: drop sentinel from i386_optab[]
Now that the table is local to gas, ARRAY_SIZE() can be used to
determine the end of the table. Re-arrange the processing loop in
md_begin() accordingly, at the same time folding the two calls to
notes_alloc() into just one.
Jan Beulich [Mon, 12 Dec 2022 07:48:25 +0000 (08:48 +0100)]
x86: add generated tables dependency check to gas
As requested by H.J., just for the sake of people potentially building
in gas/ alone, add a check that the generated files in opcodes/ are
actually up-to-date. Personally I think this should at best be a
warning, but I can see how this may not be easily noticable among other
make output (depending in particular on the verbosity level).
Jan Beulich [Mon, 12 Dec 2022 07:47:26 +0000 (08:47 +0100)]
x86: remove i386-opc.c
Remove the now empty i386-opc.c. To compensate, tie table generation in
opcodes/ to the building of i386-dis.o, despite the file not really
depending on the generated data.
Jan Beulich [Mon, 12 Dec 2022 07:46:47 +0000 (08:46 +0100)]
x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes
Unlike many other architectures, x86 does not share an opcode table
between assembly and disassembly. Any consumer of libopcodes would only
ever access one of the two. Since gas is the only consumer of the
assembly data, move it there. While doing so mark respective entities
"static" in i386-gen (we may want to do away with i386_regtab_size
altogether).
This also shrinks the number of relocations to be processed for
libopcodes.so by about 30%.
Tom de Vries [Sat, 10 Dec 2022 14:40:34 +0000 (15:40 +0100)]
[gdb/tdep] Fix larl handling in s390_displaced_step_fixup
On s390x-linux with target board unix/-m31, I run into:
...
(gdb) PASS: gdb.guile/scm-lazy-string.exp: bad length
print ptr^M
$1 = 0x804006b0 <error: Cannot access memory at address 0x804006b0>^M
(gdb) FAIL: gdb.guile/scm-lazy-string.exp: ptr: print ptr
...
A minimal example is:
...
$ gdb -q -batch -ex "set trace-commands on" -x gdb.in
+file scm-lazy-string
+break main
Breakpoint 1 at 0x4005d2: file scm-lazy-string.c, line 23.
+run
If we delete the breakpoint after running to it, we have instead the expected:
...
+delete
+step
24 const char array[] = "array";
+print ptr
$1 = 0x4006b0 "pointer"
...
The problem is in displaced stepping, forced by the presence of the breakpoint,
when stepping over this insn:
...
0x4005d2 <main+10> larl %r1,0x4006b0
...
With normal stepping we have:
...
(gdb) p /x $r1
$2 = 0x3ff004006b0
...
but with displaced stepping we have instead (note the 0x80000000 difference):
...
(gdb) p /x $r1
$1 = 0x3ff804006b0
(gdb)
...
The difference comes from this code in s390_displaced_step_fixup:
...
/* Handle LOAD ADDRESS RELATIVE LONG. */
else if (is_ril (insn, op1_larl, op2_larl, &r1, &i2))
{
/* Update PC. */
regcache_write_pc (regs, from + insnlen);
/* Recompute output address in R1. */
regcache_cooked_write_unsigned (regs, S390_R0_REGNUM + r1,
amode | (from + i2 * 2));
}
...
where the "amode |" adds the 0x80000000.
Fix this by removing the "amode |".
Tested on s390-linux, with native and target board unix/-m31.
Indu Bhagat [Fri, 9 Dec 2022 18:25:31 +0000 (10:25 -0800)]
libsframe: rename API sframe_fde_func_info to sframe_fde_create_func_info
The new name better reflects the purpose of the function.
ChangeLog:
* bfd/elfxx-x86.c (_bfd_x86_elf_create_sframe_plt): Use new
name.
* libsframe/sframe.c (sframe_fde_create_func_info): Rename
sframe_fde_func_info to this.
* libsframe/testsuite/libsframe.encode/encode-1.c: Use new name.
include/ChangeLog:
* sframe-api.h (sframe_fde_create_func_info): Rename
sframe_fde_func_info to this.
projects / thirdparty / binutils-gdb.git / log
