I noticed the gdb.LazyString documentation did not mention how to
create one. Then, while adding this, I found a couple other ways that
this documentation could be clarified.
Approved-By: Eli Zaretskii <eliz@gnu.org>
It was pointed out that the recently added gdb.opt/inline-entry.exp
test would fail when run using gcc 7 and earlier, on an x86-64 target:
https://inbox.sourceware.org/gdb-patches/9fe35ea1-d99b-444d-bd1b-e3a1f108dd77@suse.de
Bernd Edlinger points out that, for gcc, the test relies on the
-gstatement-frontiers work which was added in gcc 8.x:
https://inbox.sourceware.org/gdb-patches/DU2PR08MB10263357597688D9D66EA745CE4242@DU2PR08MB10263.eurprd08.prod.outlook.com
For gcc 7.x and older, without the -gstatement-frontiers work, the
compiler uses DW_AT_entry_pc differently, which leads to a poorer
debug experience.
Here is the interesting source line from inline-entry.c:
if ((global && bar (1)) || bar (2))
And here's some of the relevant disassembly output:
Dump of assembler code for function main:
0x401020 <+0>: mov 0x3006(%rip),%eax (1)
0x401026 <+6>: test %eax,%eax (2)
0x401028 <+8>: mov 0x2ffe(%rip),%eax (3)
0x40102e <+14>: je 0x401038 <main+24> (4)
0x401030 <+16>: sub $0x1,%eax (5)
0x401033 <+19>: jne 0x40103d <main+29> (6)
Lines (1), (2), and (4) represent the check of 'global'. However,
line (3) is actually the first instruction for 'bar' which has been
inlined. Lines (5) and (6) are also part of the first inlined 'bar'
function.
If the check of 'global' returns false then the first call to 'bar'
should never happen, this is accomplished by the branch at (4) being
taken.
For gcc 8+, gcc generates a DW_AT_entry_pc with the value 0x401030,
this is where GDB places a breakpoint for 'bar', and this address is
after the branch at line (4), and so, if the call to 'bar' never
happens, the breakpoint is never hit.
For gcc 7 and older, gcc generates a DW_AT_entry_pc with the value
0x401028, which is the first address associated with the inline 'bar'
function. Unfortunately, this address is also before the check of
'global' has completed, this means that GDB hits the 'bar' breakpoint
before the inferior has decided if 'bar' should actually be called or
not.
I don't think there's really much GDB can do in the older gcc
versions, we are placing the breakpoint at the entry point, and this
is within bar. Given that this test does really depend on the newer
gcc behaviour, I think the only sensible solution is to skip this test
when an older version of gcc is being used.
I've incorporated the check for -gstatement-frontiers support that
Bernd suggested and now the test will be skipped for older versions of
GCC.
Approved-By: Tom de Vries <tdevries@suse.de>
As with the previous two commits, this commit fixes a location where
we called PyObject_IsTrue without including an error check, this time
in bppy_init.
The 'qualified' argument is supposed to be a bool, the docs say:
The optional QUALIFIED argument is a boolean that allows
interpreting the function passed in 'spec' as a fully-qualified
name. It is equivalent to 'break''s '-qualified' flag (*note
Linespec Locations:: and *note Explicit Locations::).
It's not totally clear that the only valid values are True or False,
but I'm choosing to interpret the docs that way, and so I've added a
PyBool_Type check during argument parsing. Now, if a non-bool is
passed the user will get a TypeError during argument parsing. I've
added a test to cover this case.
This is a potentially breaking change to the Python API, but hopefully
this will not impact too many people. I've added a NEWS entry to
highlight this change.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
Like the previous commit, I discovered that in micmdpy_set_installed
we were calling PyObject_IsTrue, but not checking for a possible error
value being returned.
The micmdpy_set_installed function implements the
gdb.MICommand.installed attribute, and the documentation indicates that
this attribute should only be assigned a bool:
This attribute is read-write, setting this attribute to 'False'
will uninstall the command, removing it from the set of available
commands. Setting this attribute to 'True' will install the
command for use.
So I propose that instead of using PyObject_IsTrue we use
PyBool_Check, and if the new value fails this check we raise an
error. We can then compare the new value to Py_True directly instead
of calling PyObject_IsTrue.
This is a potentially breaking change to the Python API, but hopefully
this will not impact too many people, and the fix is pretty
easy (switch to using a bool). I've added a NEWS entry to draw
attention to this change.
Approved-By: Tom Tromey <tom@tromey.com>
Building on the previous two commits, I was auditing our uses of
PyObject_IsTrue looking for places where we were missing an error
check.
The gdb.Architecture.integer_type() function takes a 'signed' argument
which should be a 'bool', and the docs do say:
If SIGNED is not specified, it defaults to 'True'. If SIGNED is
'False', the returned type will be unsigned.
Currently we use PyObject_IsTrue, but we are missing an error check.
To fix this I've tightened the code to enforce the bool requirement at
the point that the arguments are parsed. With that done I can remove
the call to PyObject_IsTrue and instead compare to Py_True directly,
the object in question will always be a PyBool_Type.
However, we were testing that passing a non-bool argument for 'signed'
is treated as Py_False, this was added with this commit:
commit 90fe61ced1
Date: Mon Nov 29 13:53:06 2021 +0000
gdb/python: don't use the 'p' format for parsing args
which is when the PyObject_IsTrue call was added. Given that the docs
do seem pretty clear that only True or False are suitable argument
values, my proposal is that we just remove these tests and instead
test that any non-bool argument value for 'signed' gives a TypeError.
This is a breaking change to the Python API, however, my hope is that
this is such a edge case that it will not cause too many problem.
I've added a NEWS entry to highlight this change though.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
After the previous commit I audited all our uses of PyObject_IsTrue
looking for places where we were missing an error check. I did find
some that are missing error checks in places where we really should
have error checks, and I'll fix those in later commits.
This commit however, focuses on those locations where PyObject_IsTrue
is called, there is no error check, and the error check isn't really
necessary because we already know that the object we are dealing with
is of type PyBool_Type.
Inline with the previous commit, in these cases I have removed the
PyObject_IsTrue call, and replaced it with a comparison against
Py_True. In one location where it is not obvious that the object we
have is PyBool_Type I've added an assert, but in the other cases the
comparison to Py_True immediately follows a PyBool_Check call, so an
assert would be redundant.
I've added a test for the gdb.Value.format_string styling argument
being passed a non-bool value as this wasn't previously being tested,
though this new test will pass before and after this commit.
There should be no functional change after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
In this review:
https://inbox.sourceware.org/gdb-patches/87wmirfzih.fsf@tromey.com
Tom pointed out that using PyObject_IsTrue as I was doing, though
technically fine, at least appears to be missing an error check, and
that it would be better to compare to Py_True directly. I made that
change in the patch Tom was commenting on, but I'd actually copied
that code from elsewhere in python/python.c, so this commit updates
the original code inline with Tom's review feedback.
There should be no functional change after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
When running test-case gdb.reverse/time-reverse.exp on arm-linux, I run into:
...
(gdb) continue^M
Continuing.^M
Process record and replay target doesn't support syscall number 403^M
Process record does not support instruction 0xdf00 at address 0xf7ebf774.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0xf7ebf774 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
(gdb) FAIL: $exp: mode=c: continue to breakpoint: marker2
...
Syscall number 403 stands for clock_gettime64 on arm-linux.
Fix this by handling 403 in arm_canonicalize_syscall, and handling
gdb_sys_clock_gettime64 elsewhere.
Since i386_canonicalize_syscall is the identity function, enum value
gdb_sys_clock_gettime64 gets a value to match i386, which also happens to be
403.
Tested on arm-linux.
Approved-By: Guinevere Larsen <guinevere@redhat.com> (record-full)
The dictionary contains a few entries with capital letters:
...
$ grep -E '[A-Z]' .git/wikipedia-common-misspellings.txt | wc -l
143
...
but they don't look too interesting in the gdb context (for instance,
Habsbourg->Habsburg), so filter them out.
That leaves us with entries looking only like "foobat->foobar", so add
handling of capitalized words, such that we also rewrite "Foobat" to "Foobar".
Tested on aarch64-linux. Verified with shellcheck.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Add "doens't->doesn't" to gdb/contrib/common-misspellings.txt, and run
gdb/contrib/spellcheck.sh to fix this in a few files.
Tested on x86_64-linux.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Add handling of double quotes in gdb/contrib/spellcheck.sh, and fix the
following typos:
...
inheritence -> inheritance
psuedo -> pseudo
succeded -> succeeded
...
Tested on x86_64-linux.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Currently, text adjacent to parentheses is not spell-checked:
...
$ cat tmp.txt
(upto)
$ ./gdb/contrib/spellcheck.sh tmp.txt
$
...
Add handling of parentheses, such that we get:
...
$ ./gdb/contrib/spellcheck.sh tmp.txt
upto -> up to
$
...
Rerun spellcheck.sh, resulting in a few "thru->through" replacements.
Tested on x86_64-linux.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Add some documention to .pre-commit-config.yaml that explains:
- what the file is,
- how it can be used, and
- how to skip specific hooks in case of trouble.
Approved-By: Tom Tromey <tom@tromey.com>
When running test-case gdb.reverse/recursion.exp on arm-linux with target
board unix/-mthumb, I run into:
...
(gdb) PASS: gdb.reverse/recursion.exp: Skipping recursion from inside
reverse-next^M
bar (x=4195569) at /home/linux/gdb/src/gdb/testsuite/gdb.reverse/recursion.c:34^M
34 int r = foo (x);^M
(gdb) FAIL: gdb.reverse/recursion.exp: print frame when stepping out
...
The problem is the recording of the T1 push instruction [1,2], specifically:
...
000004d8 <foo>:
4d8: b580 push {r7, lr}
...
The current code fails to add a memory record for the memory written with the
value of the lr register.
Fix this by adding the missing memory record.
Tested on arm-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Luis Machado <luis.machado@arm.com>
[1] https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Encoding-of-lists-of-ARM-core-registers
[2] https://developer.arm.com/documentation/ddi0597/2024-09/T32-Instructions-by-Encoding/16-bit?lang=en#pushpop16
When running test-case gdb.reverse/fstatat-reverse.exp on arm-linux, I run
into:
...
(gdb) continue^M
Continuing.^M
Process record and replay target doesn't support syscall number 397^M
Process record does not support instruction 0xdf00 at address 0xf7ebf774.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0xf7ebf774 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
(gdb) FAIL: gdb.reverse/fstatat-reverse.exp: continue to breakpoint: marker2
...
Syscall number 397 stands for statx on arm-linux.
Fix this by handling 397 in arm_canonicalize_syscall.
Tested on arm-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Luis Machado <luis.machado@arm.com>
I (Andrew) have split this small change from a larger patch which was
posted here:
https://inbox.sourceware.org/gdb-patches/AS1PR01MB9465608EBD5D62642C51C428E4922@AS1PR01MB9465.eurprd01.prod.exchangelabs.com
And I have written the stand alone test for this issue. The original
patch included this paragraph to explain this change (I've fixed one
typo in this text replacing 'program' with 'function'):
... it may happen that the infrun machinery steps from one inline
range to another inline range of the same inline function. That can
look like jumping back and forth from the calling function to the
inline function, while really the inline function just jumps from a
hot to a cold section of the code, i.e. error handling.
The important thing that happens here is that both the outer function
and the inline function must both have multiple ranges. When the
inferior is within the inline function and moves from one range to
another it is critical that the address we stop at is the start of a
range in both the outer function and the inline function.
The diagram below represents how the functions are split and aligned:
(A) (B)
bar: |------------| |---|
foo: |------------------| |--------|
The inferior is stepping through 'bar' and eventually reaches
point (A) at which point control passes to point (B).
Currently, when the inferior stops, GDB notices that both 'foo' and
'bar' start at address (B), and so GDB uses the inline frame mechanism
to skip 'bar' and tells the user that the inferior is in 'foo'.
However, as we were in 'bar' before the step then it makes sense that
we should be in 'bar' after the step, and this is what the patch does.
There are two tests using the DWARF assembler, the first checks the
above situation and ensures that GDB reports 'bar' after the step.
The second test is similar, but after the step we enter a new range
where a different inline function starts, something like this:
(A) (B)
bar: |------------|
baz: |---|
foo: |------------------| |--------|
In this case as we step at (A) and land at (B) we leave 'bar' and
expect to stop in 'foo', GDB shouldn't automatically enter 'baz' as
that is a completely different inline function. And this is, indeed,
what we see.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
The entry PC for a DIE, e.g. an inline function, might not be the base
address of the DIE. Currently though, in block::entry_pc(), GDB
always returns the base address (low-pc or the first address of the
first range) as the entry PC.
This commit extends the block class to carry the entry PC as a
separate member variable. Then the DWARF reader is extended to read
and set the entry PC for the block. Now in block::entry_pc(), if the
entry PC has been set, this is the value returned.
If the entry-pc has not been set to a specific value then the old
behaviour of block::entry_pc() remains, GDB will use the block's base
address. Not every DIE will set the entry-pc, but GDB still needs to
have an entry-pc for every block, so the existing logic supplies the
entry-pc for any block where the entry-pc was not set.
The DWARF-5 spec for reading the entry PC is a super-set of the spec
as found in DWARF-4. For example, if there is no DW_AT_entry_pc then
DWARF-4 says to use DW_AT_low_pc while DWARF-5 says to use the base
address, which is DW_AT_low_pc or the first address in the first range
specified by DW_AT_ranges if there is no DW_AT_low_pc.
I have taken the approach of just implementing the DWARF-5 spec for
everyone. There doesn't seem to be any benefit to deliberately
ignoring a ranges based entry PC value for DWARF-4. If some naughty
compiler has emitted that, then lets use it.
Similarly, DWARF-4 says that DW_AT_entry_pc is an address. DWARF-5
allows an address or a constant, where the constant is an offset from
the base address. I allow both approaches for all DWARF versions.
There doesn't seem to be any downsides to this approach.
I ran into an issue when testing this patch where GCC would have the
DW_AT_entry_pc point to an empty range. When GDB parses the ranges
any empty ranges are ignored. As a consequence, the entry-pc appears
to be outside the address range of a block.
The empty range problem is certainly something that we can, and should
address, but that is not the focus of this patch, so for now I'm
ignoring that problem. What I have done is added a check: if the
DW_AT_entry_pc is outside the range of a block then the entry-pc is
ignored, GDB will then fall-back to its default algorithm for
computing the entry-pc.
If/when in the future we address the empty range problem, these
DW_AT_entry_pc attributes will suddenly become valid and GDB will
start using them. Until then, GDB continues to operate as it always
has.
An early version of this patch stored the entry-pc within the block
like this:
std::optional<CORE_ADDR> m_entry_pc;
However, a concern was raised that this, on a 64-bit host, effectively
increases the size of block by 16-bytes (8-bytes for the CORE_ADDR,
and 8-bytes for the std::optional's bool plus padding).
If we remove the std::optional part and just use a CORE_ADDR then we
need to have a "special" address to indicate if m_entry_pc is in use
or not. I don't really like using special addresses; different
targets can access different address ranges, even zero is a valid
address on some targets.
However, Bernd Edlinger suggested storing the entry-pc as an offset,
and I think that will resolve my concerns. So, we store the entry-pc
as a signed offset from the block's base address (the first address of
the first range, or the start() address value if there are now
ranges). Remember, ranges can be out of order, in which case the
first address of the first range might be greater than the entry-pc.
When GDB needs to read the entry-pc we can add the offset onto the
blocks base address to recalculate it.
With this done, on a 64-bit host, block only needs to increase by
8-bytes.
The inline-entry.exp test was originally contributed by Bernd here:
https://inbox.sourceware.org/gdb-patches/AS1PR01MB94659E4D9B3F4A6006CC605FE4922@AS1PR01MB9465.eurprd01.prod.exchangelabs.com
though I have made some edits, making more use of lib/gdb.exp
functions, making the gdb_test output patterns a little tighter, and
updating the test to run with Clang. I also moved the test to
gdb.opt/ as that seemed like a better home for it.
Co-Authored-By: Bernd Edlinger <bernd.edlinger@hotmail.de>
Add .cv_ucomp and .cv_scomp pseudo-directives for object files for
Windows targets, which encode compressed CodeView integers according to
the algorithm in CVCompressData in
https://github.com/Microsoft/microsoft-pdb/blob/master/include/cvinfo.h.
This is essentially Microsoft's answer to the LEB128, though used in far
fewer places.
CodeView uses these to encode the "binary annotations" in the
S_INLINESITE symbol, which express the relationship between code offsets
and line numbers in inlined functions. This has to be done in the
assembler as GCC doesn't know how many bytes each instruction takes up.
There's no equivalent for this for MSVC or LLVM, as in both cases the
assembler and compiler are integrated.
.cv_ucomp represents an unsigned big-endian integer between 0 and 0x1fffffff,
taking up 1, 2, or 4 bytes:
Value between 0 and 0x7f:
0aaaaaaa -> 0aaaaaaa (identity-mapped)
Value between 0x80 and 0x3fff:
00aaaaaa bbbbbbbb -> 10aaaaaa bbbbbbbb
Value between 0x4000 and 0x1fffffff:
000aaaaa bbbbbbbb ccccccccc dddddddd ->
110aaaaa bbbbbbbb ccccccccc dddddddd
.cv_scomp represents a signed big-endian integer between -0xfffffff and
0xfffffff, encoded according to EncodeSignedInt32 in cvinfo.h. The
absolute value of the integer is shifted left one bit, the LSB set
for a negative value, and the result expressed as if it were a
.cv_ucomp: cv_scomp(x) = cv_ucomp((abs(x) << 1) | (x < 0 ? 1 : 0))
doc/chew.c was compiled without any warning flags set. Adding the
common warnings for build showed various issues with non-static
functions missing prototypes and globals with common names (ptr and
idx) that shadowed local arguments or variables.
* doc/local.mk (doc/chew.stamp): Add WARN_CFLAGS_FOR_BUILD.
* Makefile.in: Regenerate.
* doc/chew.c (idx): Rename to pos_idx.
(ptr): Rename to buf_ptr.
(xmalloc): Make static.
(xrealloc): Likewise.
(xstrdup): Likewise.
(nextword): Likewise.
(newentry): Likewise.
(add_to_definition): Likewise.
(add_intrinsic): Likewise.
(compile): Likewise.
(icopy_past_newline): Rename idx to pos_idx, ptr to buf_ptr.
(get_stuff_in_command): Likewise.
(skip_past_newline): Likewise.
(perform): Likewise.
(main): Likewise.
Fedora GDB has, for years, carried an out of tree patch for the
gdb.base/annota{1,3}.exp tests. The patch simply adds a call to 'set
breakpoint pending off' near the start of each test.
Normally GDB tests are written using gdb_test or gdb_test_multiple,
with gdb_test being a wrapper around gdb_test_multiple. Inside
gdb_test_multiple we add a test pattern which detects if GDB offers
the user an interactive yes/no prompt and immediately fails the test.
What this means is that if something goes wrong with a test like:
gdb_test "break main" ".*"
and GDB ends up offering this prompt:
Make breakpoint pending on future shared library load? (y or [n])
then instead of hanging waiting for the test to timeout, DejaGNU will
spot the interactive prompt and immediately fail the test.
In the gdb.base/annota{1,3}.exp tests we turn on GDB's annotation
mode, and many of the tests in these scripts are written using
send_gdb and gdb_expect or gdb_expect_list. This is done because in
the past, gdb_test_multiple and friends were hard-coded to use the
"normal" GDB prompt, and these tests instead expect the annotated
prompt. Specifically, gdb_test_multiple expects '$gdb_prompt $' as
the full prompt, that is the value of $gdb_prompt followed by a single
white space. The annotation tests do update the value of $gdb_prompt,
but with annotations on there is no trailing whitespace, so
gdb_test_multiple's default behaviour doesn't work, which is why the
test scripts originally avoided using gdb_test_multiple.
As a result none of the tests in these files would early exit if we
got an interactive yes/no prompt, and instead we'd be relying on each
test to timeout, which could take a while.
However, gdb_test_multiple now accepts a -prompt argument, so we can
modify the prompt we are looking for, which is neat.
So, I started off by going through these tests an changing all of the
tests that create a breakpoint to use gdb_test, passing the -prompt
option to change the prompt.
While doing that I noticed some other things that I could improve in
these tests, I've cleaned up the use of standard_testfile, switched to
use prepare_for_testing, and removed some redundant clean_restart and
'set height 0' calls (set height 0 is done within clean_restart, which
is called by prepare_for_testing).
I've updated some comments which said "we can't use gdb_test" to say
"we can use gdb_test with -prompt option", and I've removed some
commented out debug code (e.g. setting 'exp_internal').
Finally, I ran these two tests through check-all-boards, and spotted
that annota1.exp failed when using a remote host. This is because one
test checks for a full path to the binary in some output, and with a
remote host the binary ends up being copied and the path is not as
expected.
I'm assuming that checking the full path is important, so I've
disabled this test on remote host boards.
After all these changes I checked using 'make check-all-boards' and
there are no unexpected results on either of these tests.
Tested-By: Tom de Vries <tdevries@suse.de>
Acked-By: Tom de Vries <tdevries@suse.de>
After this commit:
commit 35f09cd5d7
Date: Wed Jul 31 15:04:25 2024 +0200
[gdb/testsuite] Detect trailing-text-in-parentheses duplicates
we are now seeing some duplicate test names in gdb.trace/ tests when
using native-gdbserver or native-extended-gdbserver boards. This is
all due to tests that use some text in trailing parenthesis to make
the test name unique.
I've gone through and edited the test names as best I could to make
them all unique. Hopefully the updated test names should all make
sense.
On my machine I'm no longer seeing any duplicates with either of the
boards listed above.
Acked-By: Tom de Vries <tdevries@suse.de>
It was brought to my attention[1] that if a user makes use of Ctrl+d
to quit from a secondary prompt (e.g. the prompt used to enter lines
for the 'commands' command) then GDB will start displaying some
unexpected blank lines. Here's an example:
Reading symbols from /tmp/hello.x...
(gdb) break main
Breakpoint 1 at 0x401198: file hello.c, line 18.
(gdb) commands
Type commands for breakpoint(s) 1, one per line.
End with a line saying just "end".
>quit # <----------- Use Ctrl+d to quit here.
(gdb) show architecture
# <----------- This blank line is unexpected.
The target architecture is set to "auto" (currently "i386:x86-64").
(gdb)
I've marked up where I press 'Ctrl+d', and also the unexpected blank
line.
This issue will only happen if bracketed-paste-mode is in use. If
this has been disabled (e.g. in ~/.inputrc) then this issue will not
occur.
The blank line is not just emitted for the 'show architecture'
command. The blank line is actually caused by an extra '\n' character
emitted by readline after it has gathered a complete line of input,
and so will occur for any command.
The problem is caused by readline getting "stuck" in a state where it
thinks that an EOF has just been delivered. This state is set when
the 'Ctrl+d' does deliver an EOF, but then this state is never fully
reset. As a result, every time readline completes a line, it thinks
that the line was completed due to an EOF and so adds an extra '\n'
character.
Obviously the real fix for this issue is to patch readline, and I do
have a patch for that[2], however, version 8.2 of readline has been
released, and contains this issue. As such, if GDB is linked against
the system readline, and that system readline is 8.2, then we can
expect to see this issue.
There's a pretty simple, and cheap workaround that we can add to GDB
that will mitigate this issue.
I propose that we add this workaround to GDB. If/when the readline
patch is accepted then I'll back-port this to our local readline copy,
but retaining the workaround will be harmless, and will make GDB play
nicer with system readline libraries (version 8.2).
[1] https://inbox.sourceware.org/gdb-patches/34ef5438-8644-44cd-8537-5068e0e5e434@redhat.com
[2] https://lists.gnu.org/archive/html/bug-readline/2024-10/msg00014.html
Reviewed-By: Keith Seitz <keiths@redhat.com>
When debugging readline issues I'd like an easy way to know (for sure)
what version of readline GDB is using. This could also be useful when
writing readline tests, knowing the precise readline version will
allow us to know if we expect a test to pass or not.
Add the readline library version to the output of the 'show
configuration' command. Also include a suffix indicating if we are
using the system readline, or the statically linked in readline.
The information about static readline vs shared readline can be
figured out from the configure command output, but having it repeated
in the readline version line makes it super easy to grok within tests,
and it's super cheap, so I don't see this as a problem.
Problem Description
-------------------
On a Windows machine I built gdbserver, configured for the target
'x86_64-w64-mingw32', then on a GNU/Linux machine I built GDB with
support for all target (--enable-targets=all).
On the Windows machine I start gdbserver with a small test binary:
$ gdbserver 192.168.129.25:54321 C:\some\directory\executable.exe
On the GNU/Linux machine I start GDB without the test binary, and
connect to gdbserver.
As I have not given GDB the test binary, my expectation is that GDB
would connect to gdbserver and then download the file over the remote
protocol, but instead I was presented with this message:
(gdb) target remote 192.168.129.25:54321
Remote debugging using 192.168.129.25:54321
warning: C:\some\directory\executable.exe: No such file or directory.
0x00007ffa3e1e1741 in ?? ()
(gdb)
What I found is that if I told GDB where to find the binary, like
this:
(gdb) file target:C:/some/directory/executable.exe
A program is being debugged already.
Are you sure you want to change the file? (y or n) y
Reading C:/some/directory/executable.exe from remote target...
warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
Reading C:/some/directory/executable.exe from remote target...
Reading symbols from target:C:/some/directory/executable.exe...
(gdb)
then GDB would download the executable.
The Actual Issue
----------------
I tracked the problem down to exec_file_find (solib.c). The remote
target was passing an absolute Windows filename (beginning with "C:/"
in this case), but in exec_file_find GDB was failing the
IS_TARGET_ABSOLUTE_PATH call, and so was treating the filename as
relative.
The IS_TARGET_ABSOLUTE_PATH call was failing because GDB thought that
the file system kind was "unix", and as the filename didn't start with
a "/" it assumed the filename was not absolute.
But I'm connecting to a Windows target and 'target-file-system-kind'
was set to "auto", so GDB should be figuring out that the target
file-system is "dos-based".
Looking in effective_target_file_system_kind (filesystem.c), we find
that the logic of "auto" is delegated to the current gdbarch. However
in windows-tdep.c we see:
set_gdbarch_has_dos_based_file_system (gdbarch, 1);
So if we are using a Windows gdbarch we should have "dos-based"
filesystems. What this means is that after connecting to the remote
target GDB has selected the wrong gdbarch.
What's happening is that the target description sent back by the
remote target only includes the x86-64 registers. There's no
information about which OS we're on. As a consequence, GDB picks the
first x86-64 gdbarch which can handle the provided register set, which
happens to be a GNU/Linux gdbarch.
And indeed, there doesn't appear to be anywhere in gdbserver that sets
the osabi on the target descriptions. Some target descriptions do have
their osabi set when the description is created, e.g. in:
gdb/arch/amd64.c - Sets GNU/Linux osabi when appropriate.
gdb/arch/i386.c - Likewise.
gdb/arch/tic6x.c - Always set GNU/Linux osabi.
There are also some cases in gdb/features/*.c where the tdesc is set,
but these locations are only called from GDB, not from gdbserver.
This means that many target descriptions are created without an osabi,
gdbserver does nothing to fix this, and the description is returned to
GDB without an osabi included. This leaves GDB having to guess what
the target osabi is, and in some cases, GDB can get this wrong.
Proposed Solution
-----------------
I propose to change init_target_desc so that it requires an gdb_osabi
to be passed in, this will then be used to set the target_desc osabi
field.
I believe that within gdbserver init_target_desc is called for every
target_desc, so this should mean that every target_desc has an
opportunity to set the osabi to something sane.
I did consider passing the osabi into the code which creates the
target_desc objects, but that would require updating far more code, as
each target has its own code for creating target descriptions.
The approach taken here requires minimal changes and forces every
user of init_target_desc to think about what the correct osabi is.
In some cases, e.g. amd64, where the osabi is already set when the
target_desc is created, the init_target_desc call will override the
current value, however, we should always be replacing it with the same
actual value. i.e. if the target_desc is created with the osabi set
to GNU/Linux, then this should only happen when gdbserver is built for
GNU/Linux, in which case the init_target_desc should also be setting
the osabi to GNU/Linux.
The Tricky Bits
---------------
Some targets, like amd64, use a features based approach for creating
target_desc objects, there's a function in arch/amd64.c which creates
a target_desc, adds features too it, and returns the new target_desc.
This target_desc is then passed to an init_target_desc call within
gdbserver. This is the easy case to handle.
Then there are other targets which instead have a fixed set of xml
files, each of which is converted into a .dat file, which is then used
to generate a .cc file, which is compiled into gdbserver. The
generated .cc file creates the target_desc object and calls
init_target_desc on it. In this case though the target description
that is sent to GDB isn't generated from the target_desc object, but
is instead the contents of the fixed xml file. For this case the
osabi which we pass to init_target_desc should match the osabi that
exists in the fixed xml file.
Luckily, in the previous commit I copied the osabi information from
the fixed xml files into the .dat files. So in this commit I have
extended regdat.sh to read the osabi from the .dat file and use it in
the generated init_target_desc call.
The problem with some of these .dat base targets is that their fixed
xml files don't currently contain any osabi information, and the file
names don't indicate that they are Linux only (despite them currently
only being used from gdbserver for Linux targets), so I don't
currently feel confident adding any osabi information to these files.
An example would be features/rs6000/powerpc-64.xml. For now I've just
ignored these cases. The init_target_desc will use GDB_OSABI_UNKNOWN
which is the default. This means that for these targets nothing
changes from the current behaviour. But many other targets do now
pass the osabi back. Targets that do pass the osabi back are
improved with this commit.
Conclusion
----------
Now when I connect to the Windows remote the target description
returned includes the osabi name. With this extra information GDB
selects the correct gdbarch object, which means that GDB understands
the target has a "dos-based" file-system. With that correct GDB
understands that the filename it was given is absolute, and so fetches
the file from the remote as we'd like.
Reviewed-By: Kevin Buettner <kevinb@redhat.com>
Some gdbserver targets generate their target description based on the
gdb/regformats/*.dat files. These .dat files are generated from a
matching xml file in gdb/features/.
Lets consider a concrete example:
Take gdb/features/or1k-linux.xml, this file is processed by
gdb/features/Makefile to create gdb/regformats/or1k-linux.dat.
When gdbserver is built for the or1k target the file
or1k-linux-generated.cc is generated using the
gdb/regformats/regdat.sh script. This .cc file is then compiled and
linked into gdbserver.
The or1k-linux-generated.cc file contains the function
init_registers_or1k_linux which is called from within gdbserver, this
function creates a target_desc object and sets its xmltarget field to
a fixed string. This fixed string is the xml filename that was
originally used to generate the xml file, in this case or1k-linux.xml.
Additionally, as part of the gdbserver build the file or1k-linux.xml
is converted to a string and placed in the file
xml-builtin-generated.cc which is then built into gdbserver.
Now when GDB asks gdbserver for the target description, gdbserver
returns the fixed xmltarget string, which is the name of an xml file.
GDB will then ask gdbserver for that file and gdbserver will return
the contents of that file thanks to the xml-builtin-generated.cc
file's contents.
This is all rather complicated, but it does work. So what's the
problem that I'm fixing?
Well or1k-linux.xml does contain the osabi information, so this will
be returned from gdbserver to GDB. That's good.
However, the target_desc object created in init_registers_or1k_linux
will not have its osabi set correctly.
Now this doesn't really matter too much except
init_registers_or1k_linux includes a call to init_target_desc.
In the next commit I want to extend init_target_desc to require an
osabi to be passed in. The motivation for this will be explained in
the next commit, but if we accept for a moment that this is something
that should be done, then the question is what osabi should we use in
init_registers_or1k_linux?
Ideally we'd use the osabi which is set in or1k-linux.xml. If we do
that then everything will remain consistent, which is a good thing.
And so, to get the osabi from or1k-linux.xml into
init_registers_or1k_linux, we first need to get the osabi information
into or1k-linux.dat file, and this is what this commit does.
I've added a new xsl script print-osabi.xsl and updated
gdb/features/Makefile to make use of this script. Then I regenerated
all of the .dat files. Now every .dat file contains either:
osabi:GNU/Linux
osabi:unknown
The first is for xml files containing <osabi>GNU/Linux</osabi> and the
second is for xml files that don't contain an osabi element.
This commit doesn't attempt to make use of the osabi information in
the .dat files, that will come in the next commit. There should be no
user visible changes after this commit.
Approved-By: Kevin Buettner <kevinb@redhat.com>
Some of the top level (i.e. those that contain the <target> element)
xml files in gdb/features/ are clearly Linux only. I conclude this
based on the files names containing the string "linux".
I think that all of these files should have the <osabi> element
included with the value "GNU/Linux".
This commits adds the <osabi> element where I believe it is
appropriate and regenerates the associated .c files.
The benefit of this change is that gdbserver, which makes use of these
files, will now send the osabi back in more cases. Sending back more
descriptive target descriptions is a good thing as this makes it
easier for GDB to select the correct gdbarch.
Approved-By: Kevin Buettner <kevinb@redhat.com>
The test gdb.reverse/i386-avx-reverse.exp was changed by the recent
commit:
commit 5bf288d5a8
Author: Guinevere Larsen <guinevere@redhat.com>
Date: Fri Jul 26 17:31:14 2024 -0300
gdb/record: support AVX instructions VMOVDQ(U|A) when recording
In that commit I added a few calls to the instruction vmovdqa to and
from memory addresses. Because my local gcc testing always had aligned
pointers, I thought this would always work, but clang (and maybe other
compilers) might not do the same, which will cause vmovdqa to segfault,
and the test to fail spectacularly.
This commit fixes that by using the pre-existing precise-aligned-alloc
to allocate the dynamic buffers, forcing them to be aligned to the
required boundary for vmovdqa instruction to work. The code was then
re-shuffled to keep the current clustering of instructions.
Approved-By: Tom Tromey <tom@tromey.com>
In gdb/aarch64-linux-tdep.c we find:
...
gdb::byte_vector za_zeroed (za_bytes, 0);
regcache->raw_supply (tdep->sme_za_regnum, za_zeroed);
...
We can't use reg_buffer::raw_supply_zeroed here because only part of the
register is written.
Add raw_supply_part_zeroed, and use it instead.
Likewise elsewhere in AArch64 tdep code.
Tested on aarch64-linux.
Approved-By: Luis Machado <luis.machado@arm.com>
sec_merge_hash.size duplicates sec_merge_hash.table.count, albeit using
bfd_size_type rather than unsigned int. The only reason to have the
duplicate field is to catch unsigned int overflows, and that can be
done easily enough when and if required. Overflow isn't possible at
the moment. See the needs_resize comment.
* merge.c (sec_merge_hash): Remove "size" field.
(NEEDS_RESIZE): Delete macro, replacing with..
(needs_resize): ..this inline function.
(sec_merge_resize): Rename from sec_merge_maybe_resize,
removing redundant check.
(sec_merge_hash_insert, sec_merge_hash_lookup): Adjust to suit.
(sec_merge_init, merge_strings): Likewise.
Fix csky-linux-gnu FAIL of ld-elf/pr32341, due to that target having
its own .bss directive.
PR ld/32341
* testsuite/ld-elf/pr32341.s: Don't use .bss directive. Specify
progbits/nobits on all .section directives.
Commit 8b5a212495 supported *ABS* symbols by allowing "section" to be
bfd_abs_section, but bfd_abs_section needs to be treated specially.
In particular, bfd_get_next_section_by_name (.., bfd_abs_section_ptr)
is invalid.
PR 32347
* tekhex.c (first_phase): Guard against modification of
_bfd_std_section[] entries.
Due to a logical bug in gdb/cp-support.c:cp_search_name_hash(), GDB
may not be able to find a symbol when asked by the user. See the
accompanying test for such demonstration.
The cp_search_name_hash() cannot correctly handle a (demangled) symbol
that comprises of type-casting for the first parameter in its template
parameter list, e.g.:
foo<(enum_test)0>(int, int)
In this example, the processing logic in cp_search_name_hash() considers
the "foo<" string for hashing instead of "foo". This is due to a faulty
logic in the processing loop that tries to _keep_ hashing if a '<' char
with the following property is encountered:
---------------------------------------------------------------------
for (const char *string = search_name; *string != '\0'; ++string)
{
...
if (*string == '(')
break;
...
/* Ignore template parameter list. */
if (string[0] == '<'
&& string[1] != '(' && string[1] != '<' && string[1] != '='
&& string[1] != ' ' && string[1] = '\0')
break;
...
hash = SYMBOL_HASH_NEXT (hash, *string);
}
---------------------------------------------------------------------
Ostensibly, this logic strives to bail out of the processing loop as
soon as the beginning of an argument list is encountered, "(int, int)"
in the example, or the beginning of a template parameter list, the
"<(enum_test)0>" in the example. However, when "string" is pointing
at '<', the following incorrect logic takes precedence:
---------------------------------------------------------------------
for (const char *string = search_name; *string != '\0'; ++string)
{
if (*string == '(')
break;
...
if (string[0] == '<' && string[1] != '(' ...)
break;
hash = SYMBOL_HASH_NEXT (hash, *string);
}
---------------------------------------------------------------------
In "foo<(enum_test)0>(int, int)", the '(' char that is positioned after
the '<' char causes the "if" condition at the end of the loop not to
"break". As a result, the '<' is considered for hashing and at the
beginning of the next iteration, the loop is exited because "string"
points to '(' char.
It's obvious that the intention of the "if" condition at the end of the
loop body is to handle cases where the method name is "operator<",
"operator<<", or "operator<=". While fixing the issue, I've re-written
the logic as such to make that more explicit. Still, the complexity of
the function remains O(n). It is worth mentioning that in the same
file the "find_toplevel_char()" follows the same explicit logic.
Reviewed-By: Lancelot SIX <lancelot.six@amd.com>
Reviewed-By: Pedro Alves <pedro@palves.net>
Approved-by: Tom Tromey <tom@tromey.com>
Change-Id: I64cbdbe79671e070cc5da465d1cce7989c58074e
This simplifies things a little bit, removing some `find_if` when
inserting or removing objfiles, and the whole
unwrapping_objfile_iterator thing.
Change-Id: Idd1851d36c7834820c9c1639a6a252de643eafba
Currently, when you're already less than a page from the top in the TUI
source window, and you press Page-Up, nothing happens, while I would
expect that it then scrolls the source up to the first line.
It's happening because scrolling a full page up would result in a
negative starting line number, which is then checked if it's higher than
the (unsigned) number of available lines, and since this will always be
true, the original starting line number is restored.
Afterwards it would check if the line number is too low, but since the
negative value was already gone, it didn't do much.
Fixed by moving the low line number check before the maximum line number
check.
Approved-By: Tom Tromey <tom@tromey.com>
I noticed that in commit:
commit 5cabc8098e
Date: Wed Jul 31 15:55:57 2024 +0100
gdb/python: implement Python find_exec_by_build_id hook
I managed to typo 'unsupported' as 'unsupport'. If you run the test
on a target that doesn't support core file creation then you'll get a
TCL error.
Fixed in this commit.
I ran into an unexpected failure in gdb.base/info_sources.exp. The
test in question runs this command:
(gdb) info sources -d -- -d
That is, list all the source files whose directory name matches the
regexp '-d'. The expectation is that no source files will be listed.
Unfortunately, when I ran the test some source files are listed; the
directory I am running in contains the pattern '-d', and so the test
fails.
As we cannot control where the developer is building and testing GDB,
I propose that instead of just testing with '-d' we should search
through all the letters a-z and find one that isn't present in the
source file directory name. I'm still including the leading '-'
character in the regexp.
So now, unless GDB is being built in a directory that contains '-a',
'-b', '-c', .... '-z', the test will find one letter which isn't
present, and use that for the test.
To avoid test names changing between runs in different directories
I've had to tweak the test name to something more generic, but there
should be no change in which parts of GDB are actually being tested.
Approved-By: Tom Tromey <tom@tromey.com>
With test-case gdb.debuginfod/solib-with-soname.exp on aarch64-linux, I ran
into:
...
(gdb) core-file solib-with-soname.core^M
Downloading 197.86 K file libfoo_1.so...^M
[New LWP 997314]^M
[Thread debugging using libthread_db enabled]^M
Using host libthread_db library "/lib64/libthread_db.so.1".^M
Core was generated by `solib-with-soname'.^M
Program terminated with signal SIGABRT, Aborted.^M
(gdb) FAIL: $exp: load core file, use debuginfod: load core file
...
The test-case doesn't expect the "197.86 K" part.
The same problem was fixed for another test-case in commit a723c56efb
("gdb/testsuite: avoid intermittent failures on a debuginfod test").
Fix this in the same way: by updating the regexp.
Tested on aarch64-linux.
PR testsuite/32354
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32354
This patch changes block::get_using to return an iterator range. This
seemed cleaner to me than the current approach of returning a pointer
to the first using directive; all the callers actually use this to
iterate.
When building a help string, it's possible that the resulting options
will go over 80 columns. This patch changes this code to add line
wrapping where needed.
This can most be seen by looking "help bt" and in particular the
"-frame-info" help text.
The help text for various "internal problem" settings is longer than
80 columns. This patch tightens this up a bit. Note that these
commands are all "maint" commands so, IMO, it is sufficient if they
are clear to a gdb developer.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
The help for remote packet controls includes the "title". However
this is is just the parameter name, and not really useful to see
repeated in the help text.
Approved-By: Eli Zaretskii <eliz@gnu.org>
