| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Bug PR gdb/32079 highlights an issue where GDB will try to remove a
breakpoint for a shared library that has been unloaded. This will
trigger an error from GDB like:
(gdb) next
61 dlclose (handle[dl]);
(gdb) next
warning: error removing breakpoint 0 at 0x7ffff78169b9
warning: error removing breakpoint 0 at 0x7ffff7730b57
warning: error removing breakpoint 0 at 0x7ffff7730ad3
54 for (dl = 0; dl < 4; ++dl)
(gdb)
What happens is that as the inferior steps over the dlclose() call,
GDB notices that the library has been unloaded and calls
disable_breakpoints_in_unloaded_shlib. However, this function only
operates on user breakpoints and tracepoints.
In the example above what is happening is that the test loads multiple
copies of libc into different linker namespsaces. When we 'next' over
the dlclose call one of the copies of libc is unloaded. As GDB placed
longjmp master breakpoints within the copy of libc that was just
unloaded, the warnings we see are GDB trying (and failing) to remove
these breakpoints.
I think the solution is for disable_breakpoints_in_unloaded_shlib to
handle all breakpoints, even internal ones like the longjmp master
breakpoints.
If we do this then the breakpoint will be marked as shlib_disabled and
also will be marked as not inserted. Later when we call
breakpoint_re_set() and the longjmp breakpoints are deleted we will no
longer try to remove them.
This solution is inspired by a patch suggested in the bug report:
https://sourceware.org/bugzilla/show_bug.cgi?id=32079#c3
There are some differences with my approach compared to the patch
suggested in the bug. First I have no need to delete the breakpoint
inside disable_breakpoints_in_unloaded_shlib as an earlier patch in
this series arranged for breakpoint_re_set to be called when shared
libraries are removed. Calling breakpoint_re_set will take care of
deleting the breakpoint for us. For details see the earlier commit
titled:
gdb: fixes for code_breakpoint::disabled_by_cond logic
Next, rather than only handling bp_longjmp and bp_longjmp_master, I
allow all breakpoints to be handled. I also only give the warning
about disabling breakpoints for user breakpoints, I don't see the
point of warning the user about internal b/p changes.
With this done the issues in PR gdb/32079 are resolved.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32079
Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This commit removes the clearing of bp_location::inserted from
disable_breakpoints_in_unloaded_shlib, my claim is that this call is
not needed (any more), and with the next commit, this line actually
causes some problems.
The disable_breakpoints_in_unloaded_shlib function was added back in
2004 with commit 84acb35a5a97c, and from this first version the
function cleared the bp_location::inserted flag. The motivation for
this is that the shared library might have already been unmapped, in
which case, a later attempt to remove the location could fail.
In 2013 a similar function disable_breakpoints_in_freed_objfile was
added. This function also cleared bp_location::inserted for similar
reasons. This code was added in commit:
commit 63644780babdca3f40e1978a236b6cd78473c91b
Date: Tue Mar 12 11:10:18 2013 +0100
New remove-symbol-file command.
Then in 2014 the clearing of bp_location::inserted was removed from
disable_breakpoints_in_freed_objfile in the commit:
commit 08351840eabb44799e3d01026610420758f4fa40
Date: Tue Apr 22 23:19:19 2014 +0100
Stale breakpoint instructions, spurious SIGTRAPS.
The reason that clearing the ::inserted flag was removed in this
commit is that if the disable_breakpoints_in_freed_objfile function
was called when the b/p were actually inserted, and the memory for the
associated objfile wasn't actually unmapped, then we could end up
leaving breakpoints inserted into the inferior, which leads to
spurious SIGTRAPs.
In the next commit I'll change disable_breakpoints_in_unloaded_shlib
so that all breakpoints, not just user breakpoints, will be
disabled (via shlib_disabled) when a shared library is unloaded. This
addresses PR gdb/32079, see the next commit for a fuller justification
for this change.
The problem is that when I tested the next commit I ran into some
regressions from the gdb.base/nostdlib.exp test when run on an AArch64
GNU/Linux system where executables are compiled as PIE by default.
This test compiles a simple binary with the -nostdlib flag.
What happens is this:
- The executable is compiled as PIE, this means that we get a
dynamically linked executable, the dynamic linker is used to
perform the PIE relocation, but the executable uses no other
shared libraries.
- When GDB starts the inferior, initially the dynamic linker is
discovered as a shared library being used by the application, GDB
loads in the library and its debug symbols, placing the internal
"shlib event" breakpoints so that future shared library events can
be tracked.
- For the target I tested on systemtap probes were not used, instead
GDB fell back to the old style even breakpoint.
- As the inferior progresses, after the PIE relocation has been
performed, the dynamic linker performs some house keeping on the
list of shared libraries being used by the application. During
this process the dynamic linker is removed from the list of shared
libraries being used by the inferior, this causes GDB see a shared
library event, which GDB understands to mean that it should unload
the dynamic linker from the inferior.
I spoke with the glibc engineers at RH, and the feeling is that
this is likely a bug (it's still being investigated). But I don't
think it really matters if this is a bug or not. There are
versions of glibc in the wild that have this behaviour, so GDB
should (if the cost is not too great) be updated to handle this.
Obviously after removing the dynamic linker from the list of
shared libraries, the dynamic linker is not actually unmapped,
that would not be possible, it's the dynamic linker that does the
unmapping, so the dynamic linker is left mapped into the
inferior's address space.
- With the next patch in place all breakpoints (user and internal)
within the dynamic linker are disabled (shlib_disabled) and
currently marked as not inserted (bp_location::inserted flag is
cleared).
- Having processed the shared library event GDB then resumes the
inferior. As the shared library event is not a full stop of the
inferior (i.e. we don't remove all breakpoints before handling the
event), all of the breakpoints in the dynamic linker are still
inserted, but are now marked as not-inserted.
- GDB then resumes the inferior and immediately hits the breakpoint
that is still inserted. As GDB thinks this breakpoint is not
inserted, this is reported to the user as a SIGTRAP.
The fix I think is just to not clear the bp_location::inserted flag in
disable_breakpoints_in_unloaded_shlib. This will leave the breakpoint
as inserted in the case above. GDB will now be able to successfully
resume the inferior after the shared library event (knowing there is a
breakpoint inserted GDB will step over it and continue as expected).
The next time the inferior performs a full stop the now shlib_disabled
breakpoint will be removed from the inferior we would want.
For the usual case, where a shared library is being unloaded due to
say a dlclose, the breakpoints in the library will be marked as
disabled, but will be left inserted. The next time remove_breakpoints
is called GDB will try to remove those breakpoint locations. If the
removal fails, as the breakpoint is marked shlib_disabled, GDB will
hide the error message from the user and just assume that the shared
library has been unmapped. This functionality was first added in 2008
in commit 879d1e6b4674bc8.
There are two aspects to testing this change. First whether no
clearing the ::inserted flag causes general problems. That is tested
by running the full testsuite (I see no regressions).
Then there is the specific problem that caused me to make this
change. That issue only occurs on AArch64, with GNU/Linux using
glibc, when the executable is compiled as PIE, and doesn't use any
shared libraries other than the dynamic linker (which can be the
gdb.base/nostdlib.exp test if run on the right system). What I don't
know is how to recreate this setup in a more general form.
We can't use add-symbol-file/remove-symbol-file as that passes through
disable_breakpoints_in_freed_objfile instead, which the ::installed
flag is already not adjusted.
Also the bug doesn't trigger on x86 targets due to code in
handle_signal_stop which sees the inserted breakpoint, and decides
this must be a breakpoint that actually exists in the program, and
then because gdbarch_decr_pc_after_break returns non-zero for x86, GDB
steps the inferior past the breakpoint. This is the big difference
from AArch64 where gdbarch_decr_pc_after_break returns zero, and so
the inferior gets stuck hitting the unexpectedly inserted breakpoint.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
While working on the previous commit I realised that GDB would not
handle dprintf breakpoints correctly when a shared library was
unloaded.
Consider this example using the test binary from shlib-unload.exp. In
the function 'foo' we create a dprintf is in a shared library:
(gdb) b 59
Breakpoint 1 at 0x401215: file /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload.c, line 59.
(gdb) r
Starting program: /tmp/projects/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/shlib-unload/shlib-unload
Breakpoint 1, main () at /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload.c:59
59 res = dlclose (handle); /* Break here. */
(gdb) dprintf foo,"In foo"
Dprintf 2 at 0x7ffff7fc50fd: file /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload-lib.c, line 23.
(gdb) n
Error in re-setting breakpoint 2: Function "foo" not defined.
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
warning: error removing breakpoint 2 at 0x7ffff7fc50fd
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
60 assert (res == 0);
(gdb)
What happens here is that as the inferior steps over the dlclose call
the shared library containing 'foo' is unloaded and
disable_breakpoints_in_unloaded_shlib is called. However in
disable_breakpoints_in_unloaded_shlib we have this check:
if (b.type != bp_breakpoint
&& b.type != bp_jit_event
&& b.type != bp_hardware_breakpoint
&& !is_tracepoint (&b))
continue;
As the dprintf has type bp_dprintf then this check triggers and we
ignore the dprintf, meaning the dprintf is not disabled. When the
inferior stops after the 'next' GDB tries to remove all breakpoints
but the dprintf can no longer be removed, the memory in which it was
placed has been unmapped from the inferior.
The fix is to start using is_breakpoint() in
disable_breakpoints_in_unloaded_shlib instead of the bp_breakpoint and
bp_hardware_breakpoint checks. The is_breakpoint() function also
checks for bp_dprintf.
With this fix in place GDB now correctly disables the breakpoint and
we no longer see the warning about removing the breakpoint.
During review it was pointed out that PR gdb/23149 and PR gdb/20208
both describe something similar, though for these bugs, the inferior
is restarted (which unloads all currently loaded shlib) rather than
passing over the dlclose. But the consequences are pretty similar.
I've included a test which covers this case.
One additional thing that these two bugs did show though is that
disable_breakpoints_in_shlibs also needs to start using is_breakpoint
for the same reason. Without this change, when an inferior is
restarted we get a warning like this for dprintf breakpoints:
warning: Temporarily disabling breakpoints for unloaded shared library "..."
but we don't get a similar warning for "normal" breakpoints. This is
because disable_breakpoints_in_shlibs is called from clear_solib,
which is called when an inferior is restarted.
It is best not to think too hard about disable_breakpoints_in_shlibs,
as this function is pretty broken, e.g. it doesn't call
notify_breakpoint_modified, despite modifying the breakpoints. But
for now I'm ignoring that, but fixing this is definitely on my list
for my next set of breakpoint related fixes, it's just that a lot of
these breakpoint fixes end up being depending on one another, but I
want to avoid making this series too long. So for now, I'm ignoring
the existing bug (missing breakpoint modified events), and fixing
disable_breakpoints_in_shlibs to cover dprintf.
With these fixes in place, the two bugs mentioned above should be
fixed.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23149
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20208
Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This commit rewrites disable_breakpoints_in_unloaded_shlib to be more
like disable_breakpoints_in_freed_objfile. Instead of looping over
all b/p locations, we instead loop over all b/p and then over all
locations for each b/p.
The advantage of doing this is that we can fix the small bug that was
documented in a comment in the code:
/* This may cause duplicate notifications for the same breakpoint. */
notify_breakpoint_modified (b);
By calling notify_breakpoint_modified() as we modify each location we
can potentially send multiple notifications for a single b/p.
Is this a bug? Maybe not. After all, at each notification one of the
locations will have changed, so its probably fine. But it's not
ideal, and we can easily do better, so lets do that.
There's a new test which checks that we only get a single notification
when the shared library is unloaded. Note that the test is written as
if there are multiple related but different tests within the same test
file ... but there aren't currently! The next commit will add another
test proc to this test script at which point the comments will make
sense. I've done this to avoid unnecessary churn in the next commit.
Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I spotted that the code_breakpoint::disabled_by_cond flag doesn't work
how I'd expect it too. The flag appears to be "sticky" in some
situations; once a code_breakpoint::disabled_by_cond flag is marked
true, then, in some cases the flag wont automatically become false
again, even when you'd think it should.
The problem is in update_breakpoint_locations. In this function,
which is called as a worker of code_breakpoint::re_set, GDB computes a
new set of locations for a breakpoint, the new locations are then
installed into the breakpoint.
However, before installing the new locations GDB attempts to copy the
bp_location::enabled and bp_location::disabled_by_cond flag from the
old locations into the new locations.
The reason for copying the ::enabled flag makes sense. This flag is
controlled by the user. When we create the new locations if GDB can
see that a new location is equivalent to one of the old locations, and
if the old location was disabled by the user, then the new location
should also be disabled.
However, I think the logic behind copying the ::disabled_by_cond flag
is wrong. The disabled_by_cond flag is controlled by GDB and should
toggle automatically. If the condition string can be parsed then the
flag should be false (b/p enabled), if the condition string can't be
parsed then the flag should be true (b/p disabled).
As we always parse the condition string in update_breakpoint_locations
before we try to copy the ::enabled flag value then the
::disabled_by_cond flag should already be correct, there's no need to
copy over the ::disabled_by_cond value from the old location.
As a concrete example, consider a b/p placed within the main
executable, but with a condition that depends on a variable within a
shared library.
When the b/p is initially created the b/p will be disabled as the
condition string will be invalid (the shared library variable isn't
available yet).
When the inferior starts the shared library is loaded and the
condition variable becomes available to GDB. When the shared library
is loaded breakpoint_re_set is called which (eventually) calls
update_breakpoint_locations.
A new location is computed for the breakpoint and the condition string
is parsed. As the shared library variable is now know the expression
parses correctly and ::disabled_by_cond is left false for the new
location.
But currently GDB spots that the new location is at the same address
as the old location and copies disabled_by_cond over from the old
location, which marks the b/p location as disabled. This is not what
I would expect.
The solution is simple, don't copy over disabled_by_cond.
While writing a test I found another problem though. The
disabled_by_cond flag doesn't get set true when it should! This is
the exact opposite of the above.
The problem here is in solib_add which is (despite the name) called
whenever the shared library set changes, including when a shared
library is unloaded.
Imagine an executable that uses dlopen/dlclose to load a shared
library. Given an example of a b/p in the main executable that has a
condition that uses a variable from our shared library, a library
which might be unloaded with dlclose.
My expectation is that, when the library is unloaded, GDB will
automatically mark the breakpoint as disabled_by_cond, however, this
was not happening.
The problem is that in solib_add we only call breakpoint_re_set when
shared libraries are added, not when shared libraries are removed.
The solution I think is to just call breakpoint_re_set in both cases,
now the disabled_by_cond flag is updated as I'd expect.
Unfortunately, making this change causes a regression when running:
make check-gdb \
TESTS="gdb.trace/change-loc.exp" \
RUNTESTFLAGS="--target_board=native-gdbserver"
This test unloads a shared library and expects breakpoints within the
shared library to enter the PENDING state (because the bp_location's
shlib_disabled flag will be set). However, the new call to
breakpoint_re_set means that this is no longer the case.
The breakpoint_re_set call means that update_breakpoint_locations is
called, which then checks if all locations for a breakpoint are
pending or not. In this test not all locations are pending, and so
GDB recalculates the locations of each breakpoint, this means that
pending locations are discarded.
There is a but report PR gdb/32404 which mentions the problems with
shlib_disabled pending breakpoints, and how they are prone to being
randomly deleted if the user can cause GDB to trigger a call to
breakpoint_re_set. This patch just adds another call to
breakpoint_re_set, which triggers this bug in this one test case.
For now I have marked this test as KFAIL. I do plan to try and
address the pending (shlib_disabled) breakpoint problem in the future,
but I'm not sure when that will be right now.
There are, of course, tests to cover all these cases.
During review I was pointed at bug PR gdb/32079 as something that this
commit might fix, or help in fixing.
And this commit is part of the fix for that bug, but is not the
complete solution. However, the remaining parts of the fix for that
bug are not really related to the content of this commit.
The problem in PR gdb/32079 is that the inferior maps multiple copies
of libc in different linker namespaces using dlmopen (actually libc is
loaded as a consequence of loading some other library into a different
namespace, but that's just a detail). The user then uses a 'next'
command to move the inferior forward.
GDB sets up internal breakpoints on the longjmp symbols, of which
there are multiple copies (there is a copy in every loaded libc).
However, the 'next' command is, in the problem case, stepping over a
dlclose call which unloads one of the loaded libc libraries.
In current HEAD GDB in solib_add we fail to call breakpoint_re_set()
when the library is unloaded; breakpoint_re_set() would delete and
then recreate the longjmp breakpoints. As breakpoint_re_set() is not
called GDB thinks that the the longjmp breakpoint in the now unloaded
libc still exists, and is still inserted.
When the inferior stops after the 'next' GDB tries to delete and
remove the longjmp breakpoint which fails as the libc in which the
breakpoint was inserted is no longer mapped in.
When the user tries to 'next' again GDB tries to re-insert the still
existing longjmp breakpoint which again fails as the memory in which
the b/p should be inserted is no longer part of the inferior memory
space.
This commit helps a little. Now when the libc library is unmapped GDB
does call breakpoint_re_set(). This deletes the longjmp breakpoints
including the one in the unmapped library, then, when we try to
recreate the longjmp breakpoints (at the end of breakpoint_re_set) we
don't create a b/p in the now unmapped copy of libc.
However GDB does still think that the deleted breakpoint is inserted.
The breakpoint location remains in GDB's data structures until the
next time the inferior stops, at which point GDB tries to remove the
breakpoint .... and fails.
However, as the b/p is now deleted, when the user tries to 'next' GDB
no longer tries to re-insert the b/p, and so one of the problems
reported in PR gdb/32079 is resolved.
I'll fix the remaining issues from PR gdb/32079 in a later commit in
this series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32079
Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
|
|
I noticed a spot in dwarf2/index-write.c that was mis-formatted. This
fixes it.
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
correcting indentation as required.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
converting spaces to tabs and fixing alignment as appropriate.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
capitalizing sentences and adding full stops and spaces after them.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
removing and adding new lines as appropriate.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
Format mips-tdep.c code as described on links:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
https://www.gnu.org/prep/standards/standards.html#Comments
removing trailing space.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Approved-by: Maciej W. Rozycki <macro@orcam.me.uk>
|
|
The oss-fuzz gas fuzzer is quite broken in that it doesn't
reinitialise all gas and bfd static variables between runs. Since gas
naughtily modifies bfd_und_section and bfd_abs_section those bfd
statics can hold pointers into freed memory between runs.
This patch fixes oss-fuzz issue 398060144.
|
|
tic4x-coff and mcore-pe tickle this bug by a peculiarity of their
default ld scripts.
PR 32731
* ldlang.c (lang_add_wild): Init filenames_reversed when no
filespec.
|
|
|
|
Add gpr and fpr names for the o64 ABI to objdump.
With the recent addition of both EABIs, this completes support for the
standard ABI options (ABI-breaking options such as -modd-spreg or
-mabi=32 -mfp64 notwithstanding). The names have been verified against
GCC's usage of the registers. Notably, the only(?) documentation that
defines the o64 ABI at
https://gcc.gnu.org/projects/mipso64-abi.html
appears to contain a mistake w.r.t. floating-point arguments. In
particular:
> If the first and second arguments floating-point arguments to a
> function are 32-bit values, they are passed in $f12 and $f14.
As from 4.0.0 this does not happen in GCC's implementation of the ABI;
a pair of single-float arguments are still passed in $f12 and $f13, the
same as when one or both of the arguments are double-precision floats.
The registers $f12, $f13 and $f14 have been named $fa0, $fa1 and $ft10
to match the implementation.
Signed-off-by: Maximilian Ciric <max.ciric@gmail.com>
|
|
|
|
gdb/testsuite/rocm.exp: Use system GPU(s) to detect features
Background
----------
This patch revisits the purpose of hcc_amdgpu_targets{} in
order to address the separation of concerns between:
- GPU targets passed to the compiler. This kind of target
is passed as an argument to flags like "--offload-arch=...",
"--targets=...", etc.
- GPU targets as in available GPU devices on the system. This
is crucial for finding which capabilities are available,
and therefore which tests should be executed or skipped.
Code change
-----------
- A new "find_amdgpu_devices{}" procedure is added. It is
responsible for listing the GPU devices that are available
on the system.
- "hcc_amdgpu_targets{}" is rewritten to use the newly added
"find_amdgpu_devices{}" when there's no environment variable
(HCC_AMDGPU_TARGET) set.
- The output of "hcc_amdgpu_targets{}" is now only used in
places that set the target for the building toolchains.
- The output of "find_amdgpu_devices{}" is used anywhere that
needs to evaluate the GPU features.
Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)
Change-Id: Ib11021dbe674aa40192737ede78284a1bc531513
|
|
After I found his email bouncing, Stan, via private communication which
Nick helped with, has indicated that - having retired - he won't any
longer fulfill the maintainer role here.
|
|
Make md_undefined_symbol() conditional upon dealing with ELF, much like
other architectures (e.g. Arm32 and Arm64) have it. This avoids errors
in gas and even assertions in libbfd when "accidentally" e.g. a COFF-
targeting source file uses "_GLOBAL_OFFSET_TABLE_" for whatever reason.
While there also convert the final return statement to properly use
NULL.
NB: In principle 64-bit Mach-O knows GOT, too. Yet only an i?86-macho
assembler can be built right now, as per configure.tgt. Pretty clearly
adjustments to gotrel[] would also be necessary before these targets
could actually work reasonably cleanly.
|
|
A few lines up we would have already bailed when "baseless && is_pic".
|
|
The ELF linker rejects use of this reloc type without a base register
for PIC code. Suppress its use by gas in such cases.
To keep things building for non-ELF, include the entire containing if()
in an #ifdef: All consumers of ->fx_tcbit* live in such conditionals as
well, hence there's no reason to keep the producer active.
|
|
REX2.M affects what insn we're actually dealing with, so we better check
this to avoid transforming (future) insns we must not touch.
|
|
With us doing the transformation to an immediate operand for MOV and
various ALU insns, there's little reason to then not support the same
conversion for the other two insns which have respective immediate
operand forms. Unfortunately for IMUL (due to the 0F opcode prefix)
there's no suitable relocation, so the pre-APX forms cannot be marked
for relaxation in the assembler.
|
|
H.J. requested this adjustment; I'm unaware of any specific technical
background.
|
|
Just like was done recently for x86-64 (commit 4998f9ea9d35): Even if
the assembler avoids using the relaxable relocation for inapplicable
insns, the relocation type can still appear for other reasons. Be more
thorough in the opcode checking we do, to avoid bogusly altering other
insns.
Furthermore correct an opcode mask (even if with the added condition
that's now fully benign).
|
|
|
|
The part "... this command by default creates it produces a single ..."
sounds wrong. Replace with "... this command by default produces a
single ...".
Change-Id: I39cc533fa5a2bf473ca9e361ee0e6426d7d37ac6
|
|
Today I learned that, at least on my system (Fedora 40), the printf
"%#x" format will produce "0" rather than "0x0" when given 0 as an
argument.
This causes dwarf-mode.el to not correctly fontify the very first
"Compilation Unit" line it sees.
This patch adapts dwarf-mode.el. As always, this patch bumps the
version number for easier installation.
I am checking this in.
|
|
Change-Id: Ibd8d6c35c2cc02e309f83b11b5fd1172dfa05283
|
|
clang 19 fails to build gdb with this error:
/home/simark/src/binutils-gdb/gdb/compile/compile-object-load.c:302:3: error: cannot initialize a member subobject of type 'void (*)(const char *, ...) __attribute__((noreturn))' with an lvalue of type 'void (const char *, ...)'
302 | link_callbacks_einfo, /* einfo */
| ^~~~~~~~~~~~~~~~~~~~
This illustrates that the bfd_link_callbacks array is missing an entry
for the "fatal" callback, add it.
The fatal field was added very recently, in d26161914 ("PR 32603, more
ld -w misbehaviour"). We're lucky that the new callback was marked with
the noreturn attribute and that clang checks that, otherwise this would
have gone unnoticed.
Change-Id: I68b63d89f2707359e6254da23bdc0776b0e03ba2
|
|
While working on another series, I discovered that the existing code
in gdb.ada/complete.exp that conditionally accepts a completion does
not work correctly. The code assumes that wrapping a line in "(...)?"
will make the entire line optional, but really this will only match a
blank line.
Meanwhile, I needed this same patch for a second series I'm working
on, so I've pulled this out. As it only affects Ada, I am going to
check it in.
|
|
I noticed a non-bool-like use of target_get_tib_address in
windows-tdep.c. After fixing this I thought it would be good to
document the target method; and this also lead to some non-bool-like
commentary in remote.c. This patch fixes all of these nits.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Now that stabs is deprecated, we should probably warn our users of it
before removing support, so that they have time to react and either make
themselves heard, or fix things on their end so that they can still debug
their applications.
This commit adds a new function that emits a warning whenever GDB does
stabs reading. Since there are several places where stabs is
re-invented, this warning had to be added to many places, but I think I
managed to warn everywhere relevant without duplicating warnings.
Also, the test gdb.stabs/weird.exp explicitly checks for GDB warnings
when reading stabs, so it had to be updated to account for the
deprecation warning. It is done generically, since it will be removed in
the next release anyway.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
pr30117 showed one of the assertions added by 4d1bb7955a8b was too
strict. oss-fuzz also found the second assertion to be too strict,
with this testcase distilled from 7k of garbage source:
A=%eax%%!
Y=A
Z=A
or $6,Z
PR 32721
* config/tc-i386.c (parse_register): Move "know" into
condition. Simplify.
|
|
Right now, cooked_index_functions::expand_symtabs_matching computes
the language for each component of a split name, using the language of
the corresponding entry.
Instead, I think that we want to do all the comparisons using the
final entry's language. I don't think there's a way to trigger bad
behavior here right now, but with another series I'm working on, we
end up with some entries whose language can't reliably be determined;
and in this case using the final entry's language avoids issues.
I suspect we could also dispense with the per-segment name-matcher
lookup as well.
|
|
This changes the various producer-checking functions to be methods on
dwarf2_cu. It adds a few new caching members as well -- every one
that could reasonably be done this way has been converted, with the
only exception being a gdbarch hook.
Note the new asserts in the accessors. Without the earlier
prepare_one_comp_unit change, these could trigger in some modes.
|
|
This changes prepare_one_comp_unit to be a private method of
cutu_reader. This should make it somewhat simpler to reason about.
|
|
Currently, prepare_one_comp_unit is called somewhat haphazardly: it is
mostly called when a CU is read, but some places manage to instantiate
a cutu_reader* without calling it, and some code (e.g.,
read_file_scope) calls it without really needing to.
Aside from contributing to the general confusion around CU reading,
this doesn't really cause problems in the current tree. However, it
is possible for the DWARF reader to check the CU's producer before it
is ever set -- which is certainly unintended.
|
|
This moves the producer_is_realview to producer.c.
|
|
In dwarf2/read.c:new_symbol, DW_TAG_namelist is listed in the same
part of the "switch" as other tags. However, it effectively shares no
code with these. This patch splits it into its own case.
Longer term I think new_symbol should be split up drastically.
|
|
|
|
gas/
* config/tc-avr.c (md_assemble): Fix indentation.
|
|
When using the MI interpreter, if someone was to attach to a ROCm
process which has active GPU waves, GDB would issue a segfault as
follows:
attach 1994813
&"attach 1994813\n"
~"Attaching to process 1994813\n"
=thread-group-started,id="i1",pid="1994813"
=thread-created,id="1",group-id="i1"
=thread-created,id="2",group-id="i1"
~"[New LWP 1994828]\n"
*running,thread-id="2"
=thread-created,id="3",group-id="i1"
~"[New LWP 1994825]\n"
*running,thread-id="3"
=thread-created,id="4",group-id="i1"
~"[New LWP 1994823]\n"
*running,thread-id="4"
^done
=library-loaded,...
[...]
~"[Thread debugging using libthread_db enabled]\n"
~"Using host libthread_db library \"/lib/x86_64-linux-gnu/libthread_db.so.1\".\n"
=thread-created,id="5",group-id="i1"
&"\n\n"
&"Fatal signal: "
&"Segmentation fault"
&"\n"
&"----- Backtrace -----\n"
&"Backtrace unavailable\n"
&"---------------------\n"
&"A fatal error internal to GDB has been detected, further\ndebugging is not possible. GDB will now terminate.\n\n"
&"This is a bug, please report it."
&" For instructions, see:\n"
&"<https://github.com/ROCm-Developer-Tools/ROCgdb/issues>"
&"."
&"\n\n"
Segmentation fault
The issue comes from using a non-initialized pointer in mi_on_resume_1:
if (!mi->running_result_record_printed && mi->mi_proceeded)
{
gdb_printf (mi->raw_stdout, "%s^running\n",
mi->current_token ? mi->current_token : "");
}
In this instance, "mi->current_token" has an uninitialized value. This is a
regression introduced by:
commit def2803789208a617c429b5dcf2026decb25ce0c
Date: Wed Sep 6 11:02:00 2023 -0400
gdb/mi: make current_token a field of mi_interp
Before this patch, current_token was a global implicitly 0-initialized. Since
it is now a class field, it is not 0-initialized by default anymore. This
patch changes this.
Change-Id: I3f00b080318a70405d881ff0abe02b2c5cb1f9d8
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I was trying to get an understanding of which CUs were expanded when,
and how much time it was taking. I wrote this patch to add some logging
related to that, and I think it would be useful to have upstream, to
better understand performance problems related to over-eager CU
expansion, for example.
- add DWARF_READ_SCOPED_DEBUG_START_END
- use it in process_queue, to wrap the related expansion messages
together
- add a message in maybe_queue_comp_unit when enqueuing a comp unit
- add timing information to messages in process_queue, indicating how
much time it took to expand a given symtab
- count the number of expansions done in a single call to process_queue
[dwarf-read] process_queue: start: Expanding one or more symtabs of objfile /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw-form-ref-addr-with-type-units/dw-form-ref-addr-with-type-units ...
[dwarf-read] process_queue: Expanding symtab of CU at offset 0xc
[dwarf-read] maybe_queue_comp_unit: Queuing CU for expansion: section offset = 0x38b, queue size = 2
[dwarf-read] process_queue: Done expanding CU at offset 0xc, took 0.001s
[dwarf-read] process_queue: Expanding symtab of CU at offset 0x38b
[dwarf-read] process_queue: Done expanding CU at offset 0x38b, took 0.000s
[dwarf-read] process_queue: Done expanding 2 symtabs.
[dwarf-read] process_queue: end: Expanding one or more symtabs of objfile /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw-form-ref-addr-with-type-units/dw-form-ref-addr-with-type-units ...
Change-Id: I5237d50e0c1d06be33ea83a9120b5fe1cf7ab8c2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This makes it more obvious that all created signatured_type objects have
this flag set.
Also, remove an unnecessary assignment in create_cus_hash_table: when
constructing the dwarf2_per_cu_data object, is_debug_types is already
initialized to 0/false.
Change-Id: I6d28b17ac77edc040172254f6970d05ebc4a47f4
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Same as the previous patch, but for the containing section.
Change-Id: I469147cce21525d61b3cf6edd9a9f4b12027c176
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Similar to the previous patch, but for the offset within the containing
section.
Change-Id: I1d76e1f88002bca924e0b12fd78c7ea49d36c0ec
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Pass a dwarf2_per_bfd to the constructor of dwarf2_per_cu_data and set
the per_bfd field there. All "real" instantiations of
dwarf2_per_cu_data must have a valid, non-nullptr dwarf2_per_bfd
backlink, this makes it a bit more obvious. The instantiations of
dwarf2_per_cu_data that receive a nullptr dwarf2_per_bfd are the ones
used to do hash map lookups and the ones used in selftests.
Remove an unnecessary assignment of per_bfd in
fill_in_sig_entry_from_dwo_entry: the per_bfd field is already set when
the signatured_type object is constructor (before that, it was set in
allocate_signatured_type).
Change-Id: Ifeebe55fdb1bc2de4de9c852033fafe8abdfde8a
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I noticed that the following functions accept a "dwarf2_per_objfile",
but they can actually accept a less specific "dwarf2_per_bfd". This
makes it more obvious that the work they do is per BFD and not per
objfile.
- add_type_unit
- lookup_dwo_file_slot
- create_dwo_unit_in_dwp_v1
- create_dwp_v2_or_v5_section
- create_dwo_unit_in_dwp_v2
- create_dwo_unit_in_dwp_v5
- lookup_dwo_unit_in_dwp
Change-Id: I200cd77850ce0ffa29fc1b9d924056fdce2559f8
Approved-By: Tom Tromey <tom@tromey.com>
|
