| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
In compare_symbols in gdb/linespec.c:
...
uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();
if (uia < uib)
return true;
if (uia > uib)
return false;
...
we compare pointers to struct program_space, which gives unstable sorting
results.
The assumption is that this doesn't matter, but as PR32202 demonstrates,
sometimes it does.
While PR32202 is fixed elsewhere, it seems like a good idea to stabilize this
comparison, because it comes at a small cost and possibly prevents
hard-to-reproduce user-visible ordering issues.
Fix this by comparing the program space IDs instead of the pointers.
Likewise in compare_msymbols.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
With test-case gdb.multi/pending-bp-del-inferior.exp, occasionally I run into:
...
(gdb) info breakpoints^M
Num Type Disp Enb Address What^M
3 dprintf keep y <MULTIPLE> ^M
printf "in foo"^M
3.1 y 0x004004dc in foo at $c:21 inf 2^M
3.2 y 0x004004dc in foo at $c:21 inf 1^M
(gdb) FAIL: $exp: bp_pending=false: info breakpoints before inferior removal
...
The FAIL happens because the test-case expects:
- breakpoint location 3.1 to be in inferior 1, and
- breakpoint location 3.2 to be in inferior 2
but it's the other way around.
I managed to reproduce this with a trigger patch in
compare_symbols from gdb/linespec.c:
...
uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();
- if (uia < uib)
+ if (uia > uib)
return true;
- if (uia > uib)
+ if (uia < uib)
return false;
...
The order enforced by compare_symbols shows up in the "info breakpoints"
output because breakpoint::add_location doesn't enforce an ordering for equal
addresses:
...
auto ub = std::upper_bound (m_locations.begin (), m_locations.end (),
loc,
[] (const bp_location &left,
const bp_location &right)
{ return left.address < right.address; });
m_locations.insert (ub, loc);
...
Fix this by using new function bp_location_is_less_than
(forwarding to bp_location_ptr_is_less_than) in breakpoint::add_location.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
PR gdb/32202
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32202
|
|
bp_location_ptr_is_less_than
In breakpoint.c, we have:
...
/* A comparison function for bp_location AP and BP being interfaced to
std::sort. Sort elements primarily by their ADDRESS (no matter what
bl_address_is_meaningful says), secondarily by ordering first
permanent elements and tertiarily just ensuring the array is sorted
stable way despite std::sort being an unstable algorithm. */
static int
bp_location_is_less_than (const bp_location *a, const bp_location *b)
...
There are few problems here:
- the return type is int. While std::sort allows this, because int is
convertible to bool, it's clearer to use bool directly,
- it's not abundantly clear from either function name or comment that we can
use this to sort std::vector<bp_location *> but not
std::vector<bp_location>, and
- the comment mentions AP and BP, but there are no such parameters.
Fix this by:
- changing the return type to bool,
- renaming the function to bp_location_ptr_is_less_than and mentioning
std::vector<bp_location *> in the comment, and
- updating the comment to use the correct parameter names.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Let's not introduce support for breakpoint types the target beneath does
not support, even though we could while replaying.
Otherwise, users may set breakpoints during replay that then couldn't be
inserted into the target when switching back to recording.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
tls_maybe_erase_slot
Functions tls_maybe_fill_slot and tls_maybe_erase_slot blindly assume
that the passe solibs come from solib-svr4. This is not always the
case, because they are called even on the systems where the solib
implementation isn't solib-svr4. Add some checks to return early in
that case.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32990
Change-Id: I0a281e1f4826aa1914460c2213f0fae1bdc9af7c
Tested-By: Hannes Domani <ssbssa@yahoo.de>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
There is no need to allocate the addrmap_mutable on the heap.
Change-Id: Ia6ec17101a44ae5eaffbf3382c9639414ce5343e
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Replace the macro with a function. I don't see a need to use a macro
here, a function is easier to read.
Change-Id: I22370040cb546470498d64939b246b03700af398
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
On x86_64-linux, with gcc 7.5.0 I ran into a build breaker:
...
gdb/dwarf2/read.c: In function ‘dwo_unit* lookup_dwo_unit_in_dwp()’:
gdb/dwarf2/read.c:7403:22: error: unused variable ‘inserted’ \
[-Werror=unused-variable]
auto [it, inserted] = dwo_unit_set.emplace (std::move (dwo_unit));
^
...
Fix this by dropping the unused variable.
Tested on x86_64-linux, by completing a build.
|
|
Change-Id: I4335fbfdabe49778fe37b08689eec59be94c424b
|
|
Spotted a small white space mistake in the NEWS file. Fixed.
|
|
The buildbot pointed out this compilation failure on AlmaLinux, with g++
8.5.0, which is not seen on more recent systems:
CXX gdbtypes.o
In file included from ../../binutils-gdb/gdb/gdbtypes.c:39:
../../binutils-gdb/gdb/dwarf2/read.h:639:8: error: ‘mutex’ in namespace ‘std’ does not name a type
std::mutex dwo_files_lock;
^~~~~
../../binutils-gdb/gdb/dwarf2/read.h:639:3: note: ‘std::mutex’ is defined in header ‘<mutex>’; did you forget to ‘#include <mutex>’?
../../binutils-gdb/gdb/dwarf2/read.h:35:1:
+#include <mutex>
../../binutils-gdb/gdb/dwarf2/read.h:639:3:
std::mutex dwo_files_lock;
^~~
Fix it by including <mutex> in dwarf2/read.h.
Change-Id: Iba334a3dad217c86841a5e804d0f386876f5ff2f
|
|
In commit 2711e4754fc ("Ensure cooked_index_entry self-tests are run"), we
rewrite the function definition of _initialize_dwarf2_entry into a normal
form that allows the make-init-c script to detect it:
...
void _initialize_dwarf2_entry ();
-void _initialize_dwarf2_entry ()
+void
+_initialize_dwarf2_entry ()
...
Update make-init-c to also detect the "void _initialize_dwarf2_entry ()"
variant.
Tested on x86_64-linux, by reverting commit 2711e4754fc, rebuilding and
checking that build/gdb/init.c doesn't change.
|
|
Add a dwarf assembly test-case using a DW_FORM_strx in a .dwo file.
Tested on x86_64-linux.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The dwo_lock mutex is used to synchronize access to some dwo/dwp-related
data structures, such as dwarf2_per_bfd::dwo_files and
dwp_file::loaded_{cus,tus}. Right now the scope of the lock is kind of
coarse. It is taken in the top-level lookup_dwo_unit function, and held
while the thread:
- looks up an existing dwo_file in the per-bfd hash table for the given
id/signature
- if there's no existing dwo_file, attempt to find a .dwo file, open
it, build the list of units it contains
- if a new dwo_file was created, insert it in the per-bfd hash table
- look up the desired unit in the dwo_file
And something similar for the dwp code path. This means that two
indexing thread can't read in two dwo files simultaneously. This isn't
ideal in terms of parallelism.
This patch breaks this lock into 3 more fine grained locks:
- one lock to access dwarf2_per_bfd::dwo_files
- one lock to access dwp_file::loaded_{cus,tus}
- one lock in try_open_dwop_file, where we do two operations that
aren't thread safe (bfd_check_format and gdb_bfd_record_inclusion)
Unfortunately I don't see a clear speedup on my computer with 8 threads.
But the change shouldn't hurt, in theory, and hopefully this can be a
piece that helps in making GDB scale better on machines with many cores
(if we ever bump the max number of worker threads).
This patch uses "double-checked locking" to avoid holding the lock(s)
for the whole duration of reading in dwo files. The idea is, when
looking up a dwo with a given name:
- with the lock held, check for an existing dwo_file with that name in
dwarf2_per_bfd::dwo_files, if found return it
- if not found, drop the lock, load the dwo file and create a dwo_file
describing it
- with the lock held, attempt to insert the new dwo_file in
dwarf2_per_bfd::dwo_files. If an entry exists, it means another
thread simultaneously created an equivalent dwo_file, but won the
race. Drop the new dwo_file and use the existing one. The new
dwo_file is automatically deleted, because it is help by a unique_ptr
and the insertion into the unordered_set fails.
Note that it shouldn't normally happen for two threads to look up a dwo
file with the same name, since different units will point to different
dwo files. But it were to happen, we handle it. This way of doing
things allows two threads to read in two different dwo files
simulatenously, which in theory should help get better parallelism. The
same technique is used for dwp_file::loaded_{cus,tus}.
I have some local CI jobs that run the fission and fission-dwp boards,
and I haven't seen regressions. In addition to the regular testing, I
ran a few tests using those boards on a ThreadSanitizer build of GDB.
Change-Id: I625c98b0aa97b47d5ee59fe22a137ad0eafc8c25
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
|
|
For the same reason as explained in the previous patch (allocations on
obstacks aren't thread-safe), change the allocation of
dwarf2_section_info object for dwo files within dwp files to use "new".
The dwo_file::section object is not always owned by the dwo_file, so
introduce a new "dwo_file::section_holder" object that is only set when
the dwo_file owns the dwarf2_section_info.
Change-Id: I74c4608573c7a435bf3dadb83f96a805d21798a2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The following patch reduces the duration where the dwo_lock mutex is
taken. One operation that is not thread safe is the allocation on
dwo_units on the per_bfd obstack:
dwo_unit *dwo_unit = OBSTACK_ZALLOC (&per_bfd->obstack, struct dwo_unit);
We could take the lock around this allocation, but I think it's just
easier to avoid the problem by having the dwo_unit objects allocated
with "new".
Change-Id: Ida04f905cb7941a8826e6078ed25dbcf57674090
Approved-By: Tom Tromey <tom@tromey.com>
|
|
While debugging a new failure in my long-suffering "search-in-psyms"
series, I found that the C++ name canonicalizer did not handle a case
like "some_name::operator new []". This should remove the space,
resulting in "some_name::operator new[]" -- but does not.
This happens because the parser requires an operator to be followed by
argument types. That is, it's expected.
However, it seems to me that we do want to be able to canonicalize a
name like this. It will appear in the DWARF as a DW_AT_name, and
furthermore it could be entered by the user.
This patch fixes this problem by changing the grammar to supply the
"()" itself, then removing the trailing "()" when changing to string
form (in the functions that matter).
This isn't ideal -- it might miss a very obscure case involving the
gdb extension of providing fully-qualified names for function-local
statics -- but it improves the situation at least.
It's possible a better solution might be to rewrite the name
canonicalizer. I was wondering if this could perhaps be done without
reference to the grammar -- just by examining the tokens. However,
that's much more involved.
Let me know what you think.
Regression tested on x86-64 Fedora 40.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32939
Reviewed-By: Keith Seitz <keiths@redhat.com>
|
|
While reviewing another patch I was briefly confused by a call to
target_pid_to_exec_file coming from validate_exec_file while attaching
to a process when I had not previously set an executable.
The current order of actions in validate_exec_file is:
1. Get name of current executable.
2. Get name of executable from the current inferior.
3. If either of (1) or (2) return NULL, then there's nothing to
check, early return.
I think it would be cleaner if we instead did this:
1. Get name of current executable.
3. If (1) returned NULL then there's nothing to check, early return.
3. Get name of executable from the current inferior.
4. If (3) returned NULL then there's nothing to check, early return.
This does mean there's an extra step, but I don't think the code is
any more complex really, and we now avoid trying to extract the name
of the executable from the current inferior unless we really need it.
This avoids the target_pid_to_exec_file call that I was seeing, which
for remote targets does avoid a packet round trip (not that I'm
selling this as an "optimisation", just noting the change).
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
While looking at code coverage for gdb, I noticed that the
cooked_index_entry self-tests were not run. I tracked this down to a
formatting error in cooked-index-entry.c.
I suspect it might be better to use a macro to define these
initialization functions. That would probably remove the possibility
for this kind of error.
|
|
I ran codespell on the gdb directory and fixed a number of minor
problems. In a couple cases I replaced a "gdb spelling" (e.g.,
"readin") with an English one ("reading") where it seemed harmless.
I also added "Synopsis" as an accepted spelling.
gdb is nowhere near codespell-clean.
Approved-By: Tom de Vries <tdevries@suse.de>
|
|
make-check-all.sh
I forgot to run test-case gdb.dwarf2/dw-form-strx-out-of-bounds.exp with
make-check-all.sh, and consequently failed to notice that it fails with for
instance target board fission-dwp.
The test-case does:
...
source $srcdir/$subdir/dw-form-strx.exp.tcl
...
and in that tcl file, prepare_for_testing fails, so a -1 is returned, but
that is ignored by the source command.
Fix this by using require, but rather that testing the result of the source
command, communicate success by setting a global variable
prepare_for_testing_done.
Likewise in gdb.dwarf2/dw-form-strx.exp.
Also, the test-case gdb.dwarf2/dw-form-strx-out-of-bounds.exp fails for target
board readnow, because the DWARF error occurs during a different command than
expected.
Fix this by just skipping the test-case in that case.
Tested on x86_64-linux.
Reported-by: Simon Marchi <simark@simark.ca>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.debuginfod codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.guile codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.mi codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.opt codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.pascal codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.reverse codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.rocm codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.stabs codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.xml codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make gdb.tui codespell-clean and add the dir to the pre-commit
configuration.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I noticed a typo in the testsuite, twice: gdbsever. Fix these.
Codespell doesn't detect it, so add a new file
gdb/contrib/codespell-dictionary.txt that contains a gdbsever->gdbserver
entry, and update gdb/contrib/setup.cfg to use it.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Andrew pointed out that a recent commit neglected to update the
comment for find_field_create_baton. This patch fixes the oversight.
|
|
Kévin discovered that commit ba005d32b0f ("Handle dynamic field
properties") regressed a test in the internal AdaCore test suite.
The problem here is that, when writing that patch, I did not consider
the case where an array type's bounds might come from a member of a
structure -- but where the array is not defined in the structure's
scope.
In this scenario the field-resolution logic would trip this condition:
/* Defensive programming in case we see unusual DWARF. */
if (fi == nullptr)
return nullptr;
This patch reworks this area, partly backing out that commit, and
fixes the problem.
In the new code, I chose to simply duplicate the field's location
information. This isn't totally ideal, in that it might result in
multiple copies of a baton. However, this seemed nicer than tracking
the DIE/field correspondence for every field in every CU -- my
thinking here is that this particular dynamic scenario is relatively
rare overall. Also, if the baton cost does prove onerous, we could
intern the batons somewhere.
Regression tested on x86-64 Fedora 41. I also tested this using the
AdaCore internal test suite.
Tested-By: Simon Marchi <simon.marchi@efficios.com>
|
|
It conflicts with the ldirname function that will be added in the next
libiberty sync.
|
|
GDB is not properly exited via 'remote_close host' when running the
testsuite in a MinGW environment. Use the 'quit' command to properly
exit the GDB debugging session.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Also translate the MinGW PID to the Windows PID when running on a MinGW
target.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The emoji patch broke the create_breakpoint_parse_arg_string self-test
when gdb is running on a suitable terminal. The problem is that the
test case doesn't take the error prefix string into account.
This patch fixes the test by having it compare the exception message
directly, rather than relying on the result of exception_print. I did
try a different approach, of having the test mimic exception_print,
but this one seemed cleaner to me.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This adds initializers to field_of_this_result, so that certain spots
don't have to memset it. This approach seems safer and cleaner.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I noticed that pre-commit has some complaints (flake8 and codespell)
about gdb/__init__.py. This patch fixes these.
Approved-By: Tom de Vries <tdevries@suse.de>
|
|
This commit adds a new gdb.ParameterPrefix class to GDB's Python API.
When creating multiple gdb.Parameters, it is often desirable to group
these together under a sub-command, for example, 'set print' has lots
of parameters nested under it, like 'set print address', and 'set
print symbol'. In the Python API the 'print' part of these commands
are called prefix commands, and are created using gdb.Command objects.
However, as parameters are set via the 'set ....' command list, and
shown through the 'show ....' command list, creating a prefix for a
parameter usually requires two prefix commands to be created, one for
the 'set' command, and one for the 'show' command.
This often leads to some duplication, or at the very least, each user
will end up creating their own helper class to simplify creation of
the two prefix commands.
This commit adds a new gdb.ParameterPrefix class. Creating a single
instance of this class will create both the 'set' and 'show' prefix
commands, which can then be used while creating the gdb.Parameter.
Here is an example of it in use:
gdb.ParameterPrefix('my-prefix', gdb.COMMAND_NONE)
This adds 'set my-prefix' and 'show my-prefix', both of which are
prefix commands. The user can then add gdb.Parameter objects under
these prefixes.
The gdb.ParameterPrefix initialise method also supports documentation
strings, so we can write:
gdb.ParameterPrefix('my-prefix', gdb.COMMAND_NONE,
"Configuration setting relating to my special extension.")
which will set the documentation string for the prefix command.
Also, it is possible to support prefix commands that use the `invoke`
functionality to handle unknown sub-commands. This is done by
sub-classing gdb.ParameterPrefix and overriding either 'invoke_set' or
'invoke_show' to handle the 'set' or 'show' prefix command
respectively.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
|
|
This commit builds on the previous one, and auto-generates a general
description string for parameters defined via the Guile API. This
brings the Guile API closer inline with the Python API. It is worth
reading the previous commit to see some motivating examples.
This commit updates get_doc_string in guile/scm-param.c to allow for
the generation of a general description string. Then in
gdbscm_make_parameter, if '#:doc' was not given, get_doc_string is
used to generate a suitable default.
This does invalidate (and so the commit removes) this comment that was
in gdbscm_make_parameter:
/* If doc is NULL, leave it NULL. See add_setshow_cmd_full. */
First, Python already does exactly what I'm proposing here, and has
done for a while, with no issues reported. And second, I've gone and
read add_setshow_cmd_full, and some of the functions it calls, and can
see no reasoning behind this comment...
... well, there is one reason that I can think of, but I'll discuss
that more below.
With this commit, if I define a parameter like this:
(use-modules (gdb))
(register-parameter! (make-parameter
"print test"
#:command-class COMMAND_NONE
#:parameter-type PARAM_BOOLEAN))
Then, in GDB, I now see this behaviour:
(gdb) help show print test
Show the current value of 'print test'.
This command is not documented.
(gdb) help set print test
Set the current value of 'print test'.
This command is not documented.
(gdb)
The two 'This command is not documented.' lines are new. This output
is what we get from a similarly defined parameter using the Python
API (see the previous commit for an example).
I mentioned above that I can think of one reason for the (now deleted)
comment in gdbscm_make_parameter about leaving the doc field as NULL,
and that is this: consider the following GDB behaviour:
(gdb) help show style filename foreground
Show the foreground color for this property.
(gdb)
Notice there is only a single line of output. If I want to get the
same behaviour from a parameter defined in Guile, I might try skipping
the #:doc argument, but (after this commit), if I do that, GDB will
auto-generate some text for me, giving two lines of output (see
above).
So, next, maybe I try setting #:doc to the empty string, but if I do
that, then I get this:
(use-modules (gdb))
(register-parameter! (make-parameter
"print test"
#:doc ""
#:command-class COMMAND_NONE
#:parameter-type PARAM_BOOLEAN))
(gdb) help show print test
Show the current value of 'print test'.
(gdb)
Notice the blank line, that's not what I wanted. In fact, the only
way to get rid of the second line is to leave the 'doc' variable as
NULL in gdbscm_make_parameter, which, due to the new auto-generation,
is no longer possible.
This issue also existed in the Python API, and was addressed in
commit:
commit 4b68d4ac98aec7cb73a4b276ac7dd38d112786b4
Date: Fri Apr 11 23:45:51 2025 +0100
gdb/python: allow empty gdb.Parameter.__doc__ string
After this commit, an empty __doc__ string for a gdb.Parameter is
translated into a NULL pointer passed to the add_setshow_* command,
which means the second line of output is completely skipped.
And this commit includes the same solution for the Guile API. Now,
with this commit, and the Guile parameter using an empty '#:doc'
string, GDB has the following behaviour:
(gdb) help show print test
Show the current value of 'print test'.
(gdb)
This matches the output for a similarly defined parameter in Python.
|
|
Consider this user defined parameter created in Python:
class test_param(gdb.Parameter):
def __init__(self, name):
super ().__init__(name, gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
self.value = True
test_param('print test')
If this is loaded into GDB, then we observe the following behaviour:
(gdb) show print test
The current value of 'print test' is "on".
(gdb) help show print test
Show the current value of 'print test'.
This command is not documented.
(gdb) help set print test
Set the current value of 'print test'.
This command is not documented.
(gdb)
If we now define the same parameter using Guile:
(use-modules (gdb))
(register-parameter! (make-parameter
"print test"
#:command-class COMMAND_NONE
#:parameter-type PARAM_BOOLEAN))
And load this into a fresh GDB session, we see the following:
(gdb) show print test
Command is not documented is off.
(gdb) help show print test
This command is not documented.
(gdb) help set print test
This command is not documented.
(gdb)
The output of 'show print test' doesn't make much sense, and is
certainly worse than the Python equivalent. For both the 'help'
commands it appears as if the first line is missing, but what is
actually happening is that the first line has become 'This command is
not documented.', and the second line is then missing.
The problems can all be traced back to 'get_doc_string' in
guile/scm-param.c. This is the guile version of this function. There
is a similar function in python/py-param.c, however, the Python
version returns one of three different strings depending on the use
case. In contrast, the Guile version just returns 'This command is
not documented.' in all cases.
The three cases that the Python code handles are, the 'set' string,
the 'show' string, and the general 'description' string.
Right now the Guile get_doc_string only returns the general
'description' string, which is funny, because, in
gdbscm_make_parameter, where get_doc_string is used, the one case that
we currently don't need is the general 'description' string. Instead,
right now, the general 'description' string is used for both the 'set'
and 'show' cases.
In this commit I plan to bring the Guile API a little more inline with
the Python API. I will update get_doc_string (in scm-param.c) to
return either a 'set' or 'show' string, and gdbscm_make_parameter will
make use of these strings.
The changes to the Guile get_doc_string are modelled on the Python
version of this function. It is also worth checking out the next
commit, which is related, and helps motivate how the changes have been
implemented in this commit.
After this commit, the same Guile parameter description shown above,
now gives this behaviour:
(gdb) show print test
The current value of 'print test' is off.
(gdb) help show print test
Show the current value of 'print test'.
(gdb) help set print test
Set the current value of 'print test'.
(gdb)
The 'show print test' output now matches the Python behaviour, and is
much more descriptive. The set and show 'help' output are now missing
the second line when compared to the Python output, but the first line
is now correct, and I think this is better than the previous Guile
output.
In the next commit I'll address the problem of the missing second
line.
Existing tests have been updated to expect the new output.
|
|
I was recently attempting to create some parameters via the Python
API. I wanted these parameters to appear similar to how GDB handles
the existing 'style' parameters.
Specifically, I was interested in this behaviour:
(gdb) help show style filename foreground
Show the foreground color for this property.
(gdb) help set style filename foreground
Set the foreground color for this property.
(gdb)
Notice how each 'help' command only gets a single line of output.
I tried to reproduce this behaviour via the Python API and was unable.
The problem is that, in order to get just a single line of output like
this, the style parameters are registered with a call to
add_setshow_color_cmd with the 'help_doc' being passed as nullptr.
On the Python side, when parameters are created, the 'help_doc' is
obtained with a call to get_doc_string (python/py-param.c). This
function either returns the __doc__ string, or a default string: "This
command is not documented.".
To avoid returning the default we could try setting __doc__ to an
empty string, but setting this field to any string means that GDB
prints a line for that string, like this:
class test_param(gdb.Parameter):
__doc__ = ""
def __init__(self, name):
super ().__init__(name, gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
self.value = True
test_param('print test')
Then in GDB:
(gdb) help set print test
Set the current value of 'print test'.
(gdb)
The blank line is the problem I'd like to solve.
This commit makes a couple of changes to how parameter doc strings are
handled.
If the doc string is set to an empty string, then GDB now converts
this to nullptr, which removes the blank line problem, the new
behaviour in GDB (for the above `test_param`) is:
(gdb) help set print test
Set the current value of 'print test'.
(gdb)
Next, I noticed that if the set/show docs are set to empty strings,
then the results are less than ideal:
class test_param(gdb.Parameter):
set_doc = ""
def __init__(self, name):
super ().__init__(name, gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
self.value = True
test_param('print test')
And in GDB:
(gdb) help set print test
This command is not documented.
(gdb)
So, if the set/show docs are the empty string, GDB now forces these to
be the default string instead, the new behaviour in GDB is:
(gdb) help set print test
Set the current value of 'print test'.
This command is not documented.
(gdb)
I've added some additional asserts; the set/show docs should always be
non-empty strings, which I believe is the case after this commit. And
the 'doc' string returned from get_doc_string should never nullptr,
but could be empty.
There are new tests to cover all these changes.
|
|
The manual for gdb.Parameter says:
If NAME consists of multiple words, and no prefix parameter group
can be found, an exception is raised.
This makes sense; we cannot create a parameter within a prefix group,
if the prefix doesn't exist. And this almost works, so:
(gdb) python gdb.Parameter("xxx foo", gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
Python Exception <class 'RuntimeError'>: Could not find command prefix xxx.
Error occurred in Python: Could not find command prefix xxx.
The prefix 'xxx' doesn't exist, and we get an error. But, if we try
multiple levels of prefix:
(gdb) python gdb.Parameter("print xxx foo", gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
This completes without error, however, we didn't get what we were
maybe expecting:
(gdb) show print xxx foo
Undefined show print command: "xxx foo". Try "help show print".
But we did get:
(gdb) show print foo
The current value of 'print foo' is "off".
GDB stopped scanning the prefix string at the unknown 'xxx', and just
created the parameter there. I don't think this makes sense, nor is
it inline with the manual.
An identical problem exists with gdb.Command creation; GDB stops
parsing the prefix at the first unknown prefix, and just creates the
command there. The manual for gdb.Command says:
NAME is the name of the command. If NAME consists of multiple
words, then the initial words are looked for as prefix commands.
In this case, if one of the prefix commands does not exist, an
exception is raised.
So again, the correct action is, I believe, to raise an exception.
The problem is in gdbpy_parse_command_name (python/py-cmd.c), GDB
calls lookup_cmd_1 to look through the prefix string and return the
last prefix group. If the very first prefix word is invalid then
lookup_cmd_1 returns NULL, and this case is handled. However, if
there is a valid prefix, followed by an invalid prefix, then
lookup_cmd_1 will return a pointer to the last valid prefix list, and
will update the input argument to point to the start of the invalid
prefix word. This final case, where the input is left pointing to an
unknown prefix, was previously not handled.
I've fixed gdbpy_parse_command_name, and added tests for command and
parameter creation to cover this case.
The exact same error is present in the guile API too. The guile
documentation for make-parameter and make-command says the same things
about unknown prefixes resulting in an exception, but the same error
is present in gdbscm_parse_command_name (guile/scm-cmd.c), so I've
fixed that too, and added some tests.
|
|
Running gdb.base/errno.exp with gcc <= 13 with split DWARF results in:
$ make check TESTS="gdb.base/errno.exp" RUNTESTFLAGS="CC_FOR_TARGET=gcc-13 --target_board=fission"
(gdb) break -qualified main
/home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:7549: internal-error: locate_dwo_sections: Assertion `!dw_sect->readin' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
...
FAIL: gdb.base/errno.exp: macros: gdb_breakpoint: set breakpoint at main (GDB internal error)
The assert being hit has been added in 28f15782adab ("gdb/dwarf: read
multiple .debug_info.dwo sections"), but it merely exposed an existing
problem.
gcc versions <= 13 are affected by this bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111409
Basically, it produces .dwo files with multiple .debug_macro.dwo
sections, with some unresolved links between them. I think that this
macro debug info is unusable, and all we can do is ignore it.
In locate_dwo_sections, if we detect a second .debug_macro.dwo section,
forget about the previous .debug_macro.dwo and any subsequent one. This
will effectively make it as if the macro debug info wasn't there at all.
The errno test seems happy with it:
# of expected passes 84
# of expected failures 8
Change-Id: I6489b4713954669bf69f6e91865063ddcd1ac2c8
Approved-By: Tom Tromey <tom@tromey.com>
|
|
For a subsequent patch, it would be easier if the loop over sections
inside locate_dwo_sections (I want to maintain some state for the
duration of the loop). Move the for loop in there. And because
locate_dwz_sections is very similar, modify that one too, to keep both
in sync.
Change-Id: I90b3d44184910cc2d86af265bb4b41828a5d2c2e
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The documentation for the Source interface says
* The path of the source to be shown in the UI.
* It is only used to locate and load the content of the source if no
* `sourceReference` is specified (or its value is 0).
but the code used `path` first. I fixed it to use `sourceReference` first.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
breakpoints/13457 discusses issues with syscall catchpoints when
following forks, lamenting that there is no coverage for the
various permutations of `follow-fork-mode' and `detach-on-fork'.
This is an attempt to try and cover some of this ground. Unfortunately
the state of syscall support when detaching after the fork is
very, very inconsistent across various architectures. [I've tested
extensively Fedora/RHEL platforms.]
Right now, the only reliable platform to run tests on is x86_64/i?86
for the specific case where we do not detach from the fork. Consequently,
this patch limits testing to those architectures.
I have updated breakpoints/13457 with my findings on failures with the
detaching case.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=13457
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Update linux_find_memory_region_ftype to take 'const std::string &'
instead of 'const char *', update the two functions which are passed
as callbacks to linux_find_memory_regions_full.
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
