Simon Marchi [Tue, 14 Feb 2023 19:23:27 +0000 (14:23 -0500)]
gdb: store internalvars in an std::map
In a test downstream in ROCgdb, we had a test case failing when
GDB_REVERSE_INIT_FUNCTIONS was set. The test was assuming a particular
order in the output of "show convenience". And the order changes when
running with GDB_REVERSE_INIT_FUNCTIONS.
I think that a nice way to fix it is to make the output of "show
convenience" sorted, and therefore stable. Ideally, I think that the
the user-visible behavior of GDB should not change when using
GDB_REVERSE_INIT_FUNCTIONS. Plus, it makes the output of "show
convenience" look nice, not that it's really important.
Implement this by storing the internal vars in an std::map, which is a
sorted container.
Change-Id: I1fca7e7877cc984a3a3432c7639d45e68d437241 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Tue, 14 Feb 2023 19:23:25 +0000 (14:23 -0500)]
gdb: use std::string for internalvar::name
Change internalvar::name to std::string, automating memory management.
It becomes necessary to allocate internalvar with new instead of XNEW.
I didn't find how to trigger the code in complete_internalvar. It is
called from condition_completer, so it should be by using the
"condition" command, but I never managed to get in the right code path.
Change-Id: I814d61361663e7becb8f3fb5f58c0180cdc414bc Approved-By: Tom Tromey <tom@tromey.com>
If you then kill the inferior and change the exec-file, this will
happen:
(gdb) file bar
Architecture of file not recognized.
After this, debugging doesn't work very well.
What happens here is that, despite the warning,
target_find_description records the downloaded description in the
target_desc_info. Then the "file" command ends up calling
set_gdbarch_from_file, which uses that description.
It seems to me that, because the architecture rejected the
description, it should not be used. That is what this patch
implements.
As suggested by Eli, move the @findex entries before @item so that the
index records the position of @item, and the Info reader places you
there when you use index-search.
I went over all @findex calls in the manual, and most are like the
above. Most either appear before @item, or before @subheading, like:
@subheading The @code{-break-after} Command
@findex -break-after
I fixed all of them.
There are findex entries in annotate.texinfo,python.texi, and
stabs.texinfo as well, though those all look right to me already.
Tested by typing "i _isvoid" (@item case) and "i -complete"
(@subheading case) in an Info reader, and checking where those took
me.
Change-Id: Idb6903b0bb39ff03f93524628dcef86b5585c97e Suggested-By: Eli Zaretskii <eliz@gnu.org>
Alan Modra [Wed, 15 Feb 2023 10:50:47 +0000 (21:20 +1030)]
objdump read_section_stabs
This function is used to read sections other than stabs, and there is
now another version of it that extracts different info from the bfd
section. Rename it and return the bfd section instead of assorted
fields of the bfd section.
* objcopy.c (read_section): Renamed from read_section_stabs.
Delete size_ptr and entsize_ptr params, add contents param.
Return asection pointer. Don't unnecessarily free contents on
failure from bfd_malloc_and_get_section.
(find_stabs_section): Use read_section.
(dump_ctf, dump_section_sframe): Likewise.
(read_section_sframe): Delete.
Nick Clifton [Wed, 15 Feb 2023 09:26:10 +0000 (09:26 +0000)]
Fix the linker's merge4 test for the HPPA architecture.
PR 30078 * testsuite/ld-elf/merge4b.s: Use .asciz instead of .string in order to avoid the special behaviour of the .string directive on HPPA architectures.
Jan Beulich [Wed, 15 Feb 2023 07:46:02 +0000 (08:46 +0100)]
gas: buffer_and_nest() needs to pass nul-terminated string to temp_ilp()
In 7545aa2dd2eb ("gas: improve interaction between read_a_source_file()
and s_linefile()") I didn't pay attention to the dual purpose of the
nul character previously used. This was to a fair degree because of the
open-coding of certain operations. Insert the earlier found line
terminator instead of a hard-coded newline, and do so early in this
special case (bypassing the later general insertion point). Plus
properly use sb_terminate() to mark the end of the string. (Note that
saved_eol_char was misnamed: Without calling sb_terminate() there's
simply random data at that position in the buffer.)
Alan Modra [Wed, 15 Feb 2023 05:58:07 +0000 (16:28 +1030)]
More ecoff sanity checks
Change FIX so that unused pointers that escape the UPDATE_RAW_END
sanity checks won't result in overflows. Also sanity check the local
sym fdr isymBase and csym values.
* ecoff.c (_bfd_ecoff_slurp_symbolic_info): Define FIX to set
pointers into swapped internal data to NULL if count is zero.
Sanity check local sym fdr_ptr->isymBase and fdr_ptr->csym.
Alan Modra [Tue, 14 Feb 2023 21:21:00 +0000 (07:51 +1030)]
binutils stabs type list
Fuzzers have found that specifying a large stab type number results in
lots of memory being requested, as the list is extended with a 16
element array at a time until we reach the given stab type. It also
takes a long time. Of course normal sane stab types use small
positive integers, but it's not hard to modify the code to handle type
numbers starting anyhere.
* stabs.c (struct stab_types): Add base_index.
(stab_find_slot): Simplify filenum check. Delete type number
check. Don't allocate entire array from 0 to type number,
allocate a sparse array.
Tom Tromey [Sat, 28 Jan 2023 15:23:52 +0000 (08:23 -0700)]
Remove a use of pagination_enabled
I noticed that the TUI temporarily sets pagination_enabled and
gdb_stdout in one spot. However, I don't believe these settings are
necessary here, as a ui_file is passed to
gdbarch_print_registers_info. This patch removes these settings.
Simon Marchi [Mon, 13 Feb 2023 14:55:56 +0000 (09:55 -0500)]
gdb/dwarf2: rename some things, index -> gdb_index
This renaming helps make it clearer that these entites (classes,
functions) are specific to .gdb_index only, they are not shared with the
.debug_names handling.
Change-Id: I1a3cf3dbf450b62d1a0879d9aedd26397abdfd13 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 13 Feb 2023 19:55:13 +0000 (14:55 -0500)]
gdb: cast return value of std::unique_ptr::release to void
My editor shows warnings like:
value.c:2784: warning: The value returned by this function should be used
value.c:2784: note: cast the expression to void to silence this warning [bugprone-unused-return-value]
These warnings come from clangd, so ultimately from one of the clang
static analyzers (probably clang-tidy).
Silence these warnings by casting to void. Add a comment to explain
why this unusual thing is done.
Change-Id: I58323959c0baf9f1b20a8d596e4c58dc77c6809a Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Tue, 14 Feb 2023 16:37:16 +0000 (11:37 -0500)]
gdb: remove unnecessary tui directory check in configure
I suppose this was possible in the CVS days for the tui directory to be
missing, but it's not really possible nowaday. Well, a user could
delete the directory from their source tree but... it doesn't make
sense. Remove the check for that directory in configure.
Change-Id: Iea1412f5e5482ed003015030132ec22150c7d0b3 Approved-By: Tom Tromey <tom@tromey.com>
Tom Tromey [Tue, 14 Feb 2023 14:03:11 +0000 (07:03 -0700)]
Do not cast away const in agent_run_command
While investigating something else, I noticed some weird code in
agent_run_command (use of memcpy rather than strcpy). Then I noticed
that 'cmd' is used as both an in and out parameter, despite being
const.
Casting away const like this is bad. This patch removes the const and
fixes the memcpy. I also added a static assert to assure myself that
the code in gdbserver is correct -- gdbserver is passing its own
buffer directly to agent_run_command.
Tom de Vries [Tue, 14 Feb 2023 12:15:49 +0000 (13:15 +0100)]
[gdb/testsuite] Add xfail in gdb.python/py-record-btrace.exp
There's a HW bug affecting Processor Trace on some Intel processors
(Ice Lake to Raptor Lake microarchitectures).
The bug was exposed by linux kernel commit 670638477aed
("perf/x86/intel/pt: Opportunistically use single range output mode"),
added in version v5.5.0, and was worked around by commit ce0d998be927
("perf/x86/intel/pt: Fix sampling using single range output") in version
6.1.0.
The bug manifests (on a Performance-core of an i7-1250U, an Alder Lake cpu) in
a single test-case:
...
(gdb) python insn = r.instruction_history^M
warning: Decode error (-20) at instruction 33 (offset = 0x3d6a, \
pc = 0x400501): compressed return without call.^M
(gdb) FAIL: gdb.python/py-record-btrace.exp: prepare record: \
python insn = r.instruction_history
...
Add a corresponding XFAIL.
Note that the i7-1250U has both Performance-cores and Efficient-cores, and on
an Efficient-Core the test-case runs without any problems, so if the testsuite
run is not pinned to a specific cpu, the test may either PASS or XFAIL.
Tested on x86_64-linux:
- openSUSE Leap 15.4 with linux kernel version 5.14.21
- openSUSE Tumbleweed with linux kernel version 6.1.8
Jan Beulich [Tue, 14 Feb 2023 07:35:02 +0000 (08:35 +0100)]
gas: correct symbol name comparison in .startof./.sizeof. handling
In 162c6aef1f3a ("gas: fold symbol table entries generated for
.startof.() / .sizeof.()") I screwed up quite badly, inverting the case
sensitive and case insensitive comparison functions.
Jan Beulich [Tue, 14 Feb 2023 07:34:03 +0000 (08:34 +0100)]
gas: improve interaction between read_a_source_file() and s_linefile()
read_a_source_file() would bump line numbers only when seeing a newline,
whereas is_end_of_line[] indicates further end-of-line characters, in
particular the nul character. s_linefile() attempts to compensate for
the bump, but was too aggressive with this so far: It should only adjust
when a newline ends the line. To facilitate such a check, the check for
nothing else on the line needs to move ahead, which luckily is easily
possible: The relevant two conditions match, and the function can
simply return from the body of that earlier instance of the conditional.
The more strict treatment in s_linefile() then requires an adjustment
to buffer_and_nest()'s invocation of the function: The line terminator
now needs to be a newline, not nul.
Tom Tromey [Fri, 10 Feb 2023 17:48:50 +0000 (10:48 -0700)]
Rely on value_ref_ptr::operator->
Simon pointed out some spots were doing val.get()->mumble, where val
is a value_ref_ptr. These were introduced by the function-to-method
script, replacing older code that passed the result of .get() to a
function.
Now that value.h is using methods, we can instead rely on operator->.
This patch replaces all the newly-introduced instances of this.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Thu, 9 Feb 2023 13:35:33 +0000 (06:35 -0700)]
Introduce set_lval method on value
This introduces the set_lval method on value, one step toward removing
deprecated_lval_hack. Ultimately I think the goal should be for some
of these set_* methods to be replaced with constructors; but I haven't
done this, as the series is already too long. Other 'deprecated'
methods can probably be handled the same way.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 1 Feb 2023 15:34:58 +0000 (08:34 -0700)]
Make ~value private
At the end of this series, I belatedly realized that values should
only be destroyed by value_decref. This patch marks the the
destructor private to enforce this.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 1 Feb 2023 15:25:21 +0000 (08:25 -0700)]
Add value::set_modifiable
This introduces a value::set_modifiable and changes a couple of spots
to use it.
I'm not completely sure the comments by deprecated_modifiable are
correct any more. Perhaps they should be removed and the method
renamed. Like so many before me, though, I've deferred investigation
of the issue.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 1 Feb 2023 03:54:26 +0000 (20:54 -0700)]
Change some code to use value methods
A few functions in value.c were accessing the internal fields of
struct value. However, in these cases it seemed simpler to change
them to use the public API rather than convert them to be methods.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 23:13:08 +0000 (16:13 -0700)]
Turn many optimized-out value functions into methods
This turns many functions that are related to optimized-out or
availability-checking to be methods of value. The static function
value_entirely_covered_by_range_vector is also converted to be a
private method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 21:51:24 +0000 (14:51 -0700)]
Fully qualify calls to copy in value.c
A coming patch will add value::copy, so this namespace-qualifies
existing calls to 'copy' in value.c, to ensure it will still compile
after that change is done.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 21:38:30 +0000 (14:38 -0700)]
Turn remaining value_contents functions into methods
This turns the remaining value_contents functions -- value_contents,
value_contents_all, value_contents_for_printing, and
value_contents_for_printing_const -- into methods of value. It also
converts the static functions require_not_optimized_out and
require_available to be private methods.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 21:22:13 +0000 (14:22 -0700)]
Move value_ref_policy methods out-of-line
This moves the value_ref_policy methods to be defined out-of-line.
This is a necessary step to change value_incref and value_decref to be
methods of value.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 20:59:56 +0000 (13:59 -0700)]
Turn allocate_value_contents into a method
This turns the static function allocate_value_contents into a method
on value. It is temporarily public, until some users are converted.
set_limited_array_length is converted as well.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 20:53:55 +0000 (13:53 -0700)]
Turn value_fetch_lazy into a method
This changes value_fetch_lazy to be a method of value. A few helper
functions are converted as well, to avoid problems in later patches
when the data members are all made private.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 20:45:40 +0000 (13:45 -0700)]
Turn some value_contents functions into methods
This turns value_contents_raw, value_contents_writeable, and
value_contents_all_raw into methods on value. The remaining functions
will be changed later in the series; they were a bit trickier and so I
didn't include them in this patch.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Tue, 31 Jan 2023 20:24:00 +0000 (13:24 -0700)]
Turn allocate_value_lazy into a static "constructor"
This changes allocate_value_lazy to be a static "constructor" of
struct value.
I considered trying to change value to use ordinary new/delete, but it
seems to me that due to reference counting, we may someday want to
change these static constructors to return value_ref_ptr instead.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Thu, 12 Jan 2023 16:18:53 +0000 (16:18 +0000)]
gdb/tui: don't leak the known_window_types map
This commit finishes the task that was started in the previous
commit.
Now that all Python TUI window factories are correctly deleted when
the Python interpreter is shut down, we no longer need to dynamically
allocate the known_window_types map in tui-layout.c
This commit changes known_window_types to a statically allocated data
structure, removes the dynamic allocation from
initialize_known_windows, and then replaces lots of '->' with '.'
throughout this file.
There should be no user visible changes after this commit.
Andrew Burgess [Thu, 12 Jan 2023 15:47:17 +0000 (15:47 +0000)]
gdb/python: deallocate tui window factories at Python shut down
The previous commit relied on spotting when a Python defined TUI
window factory was deleted. I spotted that the window factories are
not deleted when GDB shuts down its Python environment, they are only
deleted when one window factory replaces another. Consider this
example Python script:
(gdb) source tui.py
Entering TestWindowFactory.__init__: A
Entering TestWindowFactory.__init__: B
Entering TestWindowFactory.__del__: B
(gdb) quit
Notice that when the 'B' window replaces the 'A' window we see the 'A'
object being deleted. But, when Python is shut down (after the
'quit') the 'B' object is never deleted.
Instead, GDB retains a reference to the window factory object, which
forces the Python object to remain live even after the Python
interpreter itself has been shut down.
The references themselves are held in a dynamically allocated
std::unordered_map (in tui/tui-layout.c) which is never deallocated,
thus the underlying Python references are never decremented to zero,
and so GDB never tries to delete these Python objects.
This commit is the first half of the work to clean up this edge case.
All gdbpy_tui_window_maker objects (the objects that implement the
TUI window factory callback for Python defined TUI windows), are now
linked together into a global list using the intrusive list mechanism.
When GDB shuts down the Python interpreter we can now walk this global
list and release the reference that is held to the underlying Python
object. By releasing this reference the Python object will now be
deleted.
I've added a new assert in gdbpy_tui_window_maker::operator(), this
will catch the case where we somehow end up in here after having
reset the reference to the underlying Python object. I don't think
this should ever happen though as we only clear the references when
shutting down the Python interpreter, and the ::operator() function is
only called when trying to apply a new TUI layout - something that
shouldn't happen while GDB itself is shutting down.
This commit does not update the std::unordered_map in tui-layout.c,
that will be done in the next commit.
Then GDB would have a new TUI window 'my_window', which could be
created by calling FactoryFunction(). If I then, in the same GDB
session imported a script which included:
Then GDB would replace the old 'my_window' factory with my new one,
GDB would now call UpdatedFactoryFunction().
This is pretty useful in practice, as it allows users to iterate on
their window implementation within a single GDB session.
However, right now, this is not how GDB operates. The second call to
register_window_type is basically ignored and the old window factory
is retained.
This is because in tui_register_window (tui/tui-layout.c) we use
std::unordered_map::emplace to insert the new factory function, and
emplace doesn't replace an existing element in an unordered_map.
In this commit, before the emplace call, I now search for an already
existing element, and delete any matching element from the map, the
emplace call will then add the new factory function.
Keith Seitz [Mon, 13 Feb 2023 14:14:40 +0000 (06:14 -0800)]
Fix doc build dependencies for --with-system-readline
PR build/30108 concerns building gdb documentation with
--with-sytem-readline. If the in-tree readline directory is
missing, though, the docs will fail to build:
make[4]: Entering directory '/home/keiths/work/readline-doc-issue/linux/gdb/doc'
make[4]: *** No rule to make target '../../../src/gdb/doc/../../readline/readline/doc/rluser.texi', needed by 'gdb.info'. Stop.
The listed file (and hsuser.texi) are conditionally included by gdb.texinfo.
When system readline is used, gdb/configure.ac will leave
READLINE_TEXI_INCFLAGS empty, causing doc/Makefile.in to output a line to
$BUILD/doc/GDBvn.texi with "@set SYSTEM_READLINE". This surpresses the
inclusion of the missing files. They are not needed or used in this
scenario.
However, GDB_DOC_SOURCE_INCLUDES always lists these two files as dependencies,
thus provoking the build error whenever readline/ is missing.
This patch fixes this by creating (essentially) a conditional setting of the
dependencies to be included from readline.
Michael Matz [Thu, 9 Feb 2023 14:29:00 +0000 (15:29 +0100)]
Fix PR30079: abort on mingw
the early-out in wild_sort is not enough, it might still be
that filenames are equal _and_ the wildcard list doesn't specify
a sort order either. Don't call compare_section then.
opcodes/mips: use .word/.short for undefined instructions
unknown 32-bit microMIPS instructions were disassembled as a raw
32-bit number with no '.word' directive. The above commit changed
this and added a '.word' directive before the 32-bit number.
It was pointed out on the mailing list, that for microMIPS it would be
better to display such 32-bit instructions using a '.short' directive
followed by two 16-bit values.
This commit updates the mips disassembler to do this, and adds a new
test that validates this output.
gdb: only allow one of thread or task on breakpoints or watchpoints
fails for some versions of guile. It turns out that some versions of
guile emit an error like this:
(gdb) guile (set-breakpoint-thread! bp 1)
ERROR: In procedure set-breakpoint-thread!:
In procedure gdbscm_set_breakpoint_thread_x: cannot set both task and thread attributes
Error while executing Scheme code.
while other versions of guile emit the error like this:
(gdb) guile (set-breakpoint-thread! bp 1)
ERROR: In procedure set-breakpoint-thread!:
ERROR: In procedure gdbscm_set_breakpoint_thread_x: cannot set both task and thread attributes
Error while executing Scheme code.
notice the extra 'ERROR: ' on the second line of output. This commit
updates the test regexp to handle this optional 'ERROR: ' string.
Alan Modra [Mon, 13 Feb 2023 09:58:41 +0000 (20:28 +1030)]
stabs.c static state
Move all the function local static state variables to file scope,
in order to tidy memory on exit and to reinit everything for that
annoying oss-fuzz. Also fix a couple memory leaks.
* read.h (read_begin, read_end): Declare.
* read.c (read_begin): Call stabs_begin.
(read_end): Call stabs_end.
* stabs.c (stabs_begin, stabs_end): New functions.
(in_dot_func_p): Delete, use current_function_label instead.
(cached_sec): Move from s_stab_generic.
(last_asm_file, file_label_count): Move from generate_asm_file.
(line_label_count, prev_lineno, prev_line_file): Move from
stabs_generate_asm_lineno.
(void_emitted_p): Move from stabs_generate_asm_func.
(endfunc_label_count): Move from stabs_generate_asm_endfunc.
(stabs_generate_asm_lineno): Simplify setting of
prev_line_file.
(stabs_generate_asm_func): Don't leak current_function_label.
(stabs_generate_asm_endfunc): Likewise.
Lancelot SIX [Tue, 7 Feb 2023 15:13:47 +0000 (15:13 +0000)]
gdb/testsuite: look for hipcc in env(ROCM_PATH)
If the hipcc compiler cannot be found in dejagnu's tool_root_dir, look
for it in $::env(ROCM_PATH) (if set). If hipcc is still not found,
fallback to "hipcc" so the compiler will be searched in the PATH. This
removes the fallback to the hard-coded "/opt/rocm/bin" prefix.
This change is done so ROCM tools are searched in a uniform manner.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Lancelot SIX [Sat, 4 Feb 2023 23:57:07 +0000 (23:57 +0000)]
gdb/testsuite: allow_hipcc_tests tests the hipcc compiler
Update allow_hipcc_tests so all gdb.rocm tests are skipped if we do not
have a working hipcc compiler available.
To achieve this, adjust gdb_simple_compile to ensure that the hip
program is saved in a ".cpp" file before calling hipcc otherwise
compilation will fail.
One thing to note is that it is possible to have a hipcc installed with
a CUDA backend. Compiling with this back-end will successfully result
in an application, but GDB cannot debug it (at least for the offload
part). In the context of the gdb.rocm tests, we want to detect such
situation where gdb_simple_compile would give a false positive.
To achieve this, this patch checks that there is at least one AMDGPU
device available and that hipcc can compile for this or those targets.
Detecting the device is done using the rocm_agent_enumerator tool which
is installed with the all ROCm installations (it is used by hipcc to
detect identify targets if this is not specified on the comand line).
This patch also makes the allow_hipcc_tests proc a cached proc.
Co-Authored-By: Pedro Alves <pedro@palves.net> Approved-By: Simon Marchi <simon.marchi@efficios.com>
Lancelot SIX [Mon, 6 Feb 2023 19:08:13 +0000 (19:08 +0000)]
gdb/testsuite: require amd-dbgapi support to run rocm tests
Update allow_hipcc_tests to check that GDB has the amd-dbgapi support
built-in. Without this support, all tests using hipcc and the rocm
stack will fail.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Alan Modra [Sun, 12 Feb 2023 12:04:27 +0000 (22:34 +1030)]
objcopy memory leaks
This fixes some objcopy memory leaks. commit 450da4bd38ae used
xatexit to tidy most of the hash table memory, but of course that's
ineffective without a call to xexit. The other major memory leak
happens if there is an error of some sort writing the output file, due
to not closing the input file and thus not freeing memory attached to
the bfd.
* objcopy.c (copy_file): Don't return when bfd_close of output
gives an error, always bfd_close input too.
(main): Call xexit.
Tom Tromey [Fri, 27 Jan 2023 04:13:20 +0000 (21:13 -0700)]
Move some code from dwarf2/read.c to die.c
This patch introduces a new file, dwarf2/die.c, and moves some
DIE-related code out of dwarf2/read.c and into this new file. This is
just a small part of the long-term project to split up read.c.
(According to 'wc', dwarf2/read.c is the largest file in gdb by around
8000 LOC.)
gdb: show task number in describe_other_breakpoints
That both of these are my own commits is only more embarrassing.
This small fix updates describe_other_breakpoints to take account of
the default task number now being -1. This fixes regressions in
gdb.base/break.exp, gdb.base/break-always.exp, and many other tests.
Andrew Burgess [Fri, 16 Dec 2022 15:15:42 +0000 (15:15 +0000)]
gdb/c++: fix handling of breakpoints on @plt symbols
This commit should fix PR gdb/20091, PR gdb/17201, and PR gdb/17071.
Additionally, PR gdb/17199 relates to this area of code, but is more
of a request to refactor some parts of GDB, this commit does not
address that request, but it is probably worth reading that PR when
looking at this commit.
When the current language is C++, and the user places a breakpoint on
a function in a shared library, GDB will currently find two locations
for the breakpoint, one location will be within the function itself as
we would expect, but the other location will be within the PLT table
for the call to the named function. Consider this session:
$ gdb -q /tmp/breakpoint-shlib-func
Reading symbols from /tmp/breakpoint-shlib-func...
(gdb) start
Temporary breakpoint 1 at 0x40112e: file /tmp/breakpoint-shlib-func.cc, line 20.
Starting program: /tmp/breakpoint-shlib-func
Temporary breakpoint 1, main () at /tmp/breakpoint-shlib-func.cc:20
20 int answer = foo ();
(gdb) break foo
Breakpoint 2 at 0x401030 (2 locations)
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y <MULTIPLE>
2.1 y 0x0000000000401030 <foo()@plt>
2.2 y 0x00007ffff7fc50fd in foo() at /tmp/breakpoint-shlib-func-lib.cc:20
This is not the expected behaviour. If we compile the same test using
a C compiler then we see this:
(gdb) break foo
Breakpoint 2 at 0x7ffff7fc50fd: file /tmp/breakpoint-shlib-func-c-lib.c, line 20.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y 0x00007ffff7fc50fd in foo at /tmp/breakpoint-shlib-func-c-lib.c:20
Here's what's happening. When GDB parses the symbols in the main
executable and the shared library we see a number of different symbols
for foo, and use these to create entries in GDB's msymbol table:
- In the main executable we see a symbol 'foo@plt' that points at
the plt entry for foo, from this we add two entries into GDB's
msymbol table, one called 'foo@plt' which points at the plt entry
and has type mst_text, then we create a second symbol, this time
called 'foo' with type mst_solib_trampoline which also points at
the plt entry,
- Then, when the shared library is loaded we see another symbol
called 'foo', this one points at the actual implementation in the
shared library. This time GDB creates a msymbol called 'foo' with
type mst_text that points at the implementation.
This means that GDB creates 3 msymbols to represent the 2 symbols
found in the executable and shared library.
When the user creates a breakpoint on 'foo' GDB eventually ends up in
search_minsyms_for_name (linespec.c), this function then calls
iterate_over_minimal_symbols passing in the name we are looking for
wrapped in a lookup_name_info object.
In iterate_over_minimal_symbols we iterate over two hash tables (using
the name we're looking for as the hash key), first we walk the hash
table of symbol linkage names, then we walk the hash table of
demangled symbol names.
When the language is C++ the symbols for 'foo' will all have been
mangled, as a result, in this case, the iteration of the linkage name
hash table will find no matching results.
However, when we walk the demangled hash table we do find some
results. In order to match symbol names, GDB obtains a symbol name
matching function by calling the get_symbol_name_matcher method on the
language_defn class. For C++, in this case, the matching function we
use is cp_fq_symbol_name_matches, which delegates the work to
strncmp_iw_with_mode with mode strncmp_iw_mode::MATCH_PARAMS and
language set to language_cplus.
The strncmp_iw_mode::MATCH_PARAMS mode means that strncmp_iw_mode will
skip any parameters in the demangled symbol name when checking for a
match, e.g. 'foo' will match the demangled name 'foo()'. The way this
is done is that the strings are matched character by character, but,
once the string we are looking for ('foo' here) is exhausted, if we
are looking at '(' then we consider the match a success.
Lets consider the 3 symbols GDB created. If the function declaration
is 'void foo ()' then from the main executable we added symbols
'_Z3foov@plt' and '_Z3foov', while from the shared library we added
another symbol call '_Z3foov'. When these are demangled they become
'foo()@plt', 'foo()', and 'foo()' respectively.
Now, the '_Z3foov' symbol from the main executable has the type
mst_solib_trampoline, and in search_minsyms_for_name, we search for
any symbols of type mst_solib_trampoline and filter these out of the
results.
However, the '_Z3foov@plt' symbol (from the main executable), and the
'_Z3foov' symbol (from the shared library) both have type mst_text.
During the demangled name matching, due to the use of MATCH_PARAMS
mode, we stop the comparison as soon as we hit a '(' in the demangled
name. And so, '_Z3foov@plt', which demangles to 'foo()@plt' matches
'foo', and '_Z3foov', which demangles to 'foo()' also matches 'foo'.
By contrast, for C, there are no demangled hash table entries to be
iterated over (in iterate_over_minimal_symbols), we only consider the
linkage name symbols which are 'foo@plt' and 'foo'. The plain 'foo'
symbol obviously matches when we are looking for 'foo', but in this
case the 'foo@plt' will not match due to the '@plt' suffix.
And so, when the user asks for a breakpoint in 'foo', and the language
is C, search_minsyms_for_name, returns a single msymbol, the mst_text
symbol for foo in the shared library, while, when the language is C++,
we get two results, '_Z3foov' for the shared library function, and
'_Z3foov@plt' for the plt entry in the main executable.
I propose to fix this in strncmp_iw_with_mode. When the mode is
MATCH_PARAMS, instead of stopping at a '(' and assuming the match is a
success, GDB will instead search forward for the matching, closing,
')', effectively skipping the parameter list, and then resume
matching. Thus, when comparing 'foo' to 'foo()@plt' GDB will
effectively compare against 'foo@plt' (skipping the parameter list),
and the match will fail, just as it does when the language is C.
There is one slight complication, which is revealed by the test
gdb.linespec/cpcompletion.exp, when searching for the symbol of a
const member function, the demangled symbol will have 'const' at the
end of its name, e.g.:
Previously, the matching would stop at the '(' character, but after my
change the whole '()' is skipped, and the match resumes. As a result,
the 'const' modifier results in a failure to match, when previously
GDB would have found a match.
To work around this issue, in strncmp_iw_with_mode, when mode is
MATCH_PARAMS, after skipping the parameter list, if the next character
is '@' then we assume we are looking at something like '@plt' and
return a value indicating the match failed, otherwise, we return a
value indicating the match succeeded, this allows things like 'const'
to be skipped.
With these changes in place I now see GDB correctly setting a
breakpoint only at the implementation of 'foo' in the shared library.
Andrew Burgess [Wed, 8 Feb 2023 10:31:14 +0000 (10:31 +0000)]
gdb: use -1 for breakpoint::task default value
Within the breakpoint struct we have two fields ::thread and ::task
which are used for thread or task specific breakpoints. When a
breakpoint doesn't have a specific thread or task then these fields
have the values -1 and 0 respectively.
There's no particular reason (as far as I can tell) why these two
"default" values are different, and I find the difference a little
confusing. Long term I'd like to potentially fold these two fields
into a single field, but that isn't what this commit does.
What this commit does is switch to using -1 as the "default" value for
both fields, this means that the default for breakpoint::task has
changed from 0 to -1. I've updated all the code I can find that
relied on the value of 0, and I see no test regressions, especially in
gdb.ada/tasks.exp, which still fully passes.
There should be no user visible changes after this commit.