| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
With
target-async off
scheduler-locking off
schedule-multiple on
and
record btrace
the record-btrace target splits resume and stop requests when there are
multiple inferiors and some are replaying while others are recording.
Since wait would be blocking in this configuration, we cannot afford to
split wait requests, as well, or risk a hang.
This leads to scenarios where the target beneath record-btrace received
resume requests for inferiors other than the current inferior, since
that would be handled by the record target above.
stop requests, but has not gotten the chance to wait for the
corresponding event before receiving another resume request.
Handle those cases for the linux native target.
|
|
While recording is already per inferior, run-control isn't. As soon as
any thread in any inferior is replaying, no other inferior can be resumed.
This is controlled by calls to record_is_replaying(minus_one_ptid).
Instead of minus_one_ptid, pass the ptid of the inferior to be checked,
and split requests for minus_one_ptid by inferior for resume and stop.
Since it is not safe to split a wait request for blocking targets, we
forward the minus_one_ptid request if there are no events to report for
replaying threads.
|
|
In do_target_wait(), we iterate over inferiors and call
do_target_wait_1(), which eventually calls target_wait() per inferior.
The surrounding code selects a random inferior and then iterates over all
inferiors starting from the selected inferior. In each iteration, it
waits for one inferior. We forgot to restrict the wait, however, so we
may end up with an event for a different inferior.
In some cases, e.g. gdb.threads/detach-step-over.exp, we ask to wait for
one inferior, and get an event from a different inferior back without
noticing the inferior switch.
Wait for a single inferior, instead. Since we iterate over all inferiors,
we still cover everything.
This exposes another bug with STOP_QUIETLY_NO_SIGSTOP handling.
After attaching, we interrupt all threads in the new inferior, then call
do_target_wait() to receive the stopped events. This randomly selects an
inferior to start waiting for and iterates over all inferiors starting
from there.
The initial stop event for the main thread is already queued up, so we
wouldn't actually wait() if we had started with the new inferior. Or if
we had waited for minus_one_ptid, which would then have silently switched
inferiors.
Since we no longer allow that, we may actually wait() for the new inferior
and find other events to report, out of which we randomly select one.
If we selected an event for another thread, e.g. one that had been
interrupted as part of non-stop attach, STOP_QUIETLY_NO_SIGSTOP would be
applied to that thread (unnecessarily), leaving the main thread with a
SIGSTOP event but last_resume_kind = resume_continue.
When the main thread is later selected, SIGSTOP is reported to the user.
Normally, linux-nat's wait() turns the SIGSTOP it uses for interrupting
threads into GDB_SIGNAL_0. This is based on last_resume_kind, which is
set to resume_stop when sending SIGSTOP to interrupt a thread.
We do this for all threads of the new inferior when interrupting them as
part of non-stop attach. Except for the main thread, which we expect to
be reported before the first wait().
Set last_resume_kind to resume_stop for the main thread after attaching.
|
|
In stopped_by_sw_breakpoint() and stopped_by_hw_breakpoint(), we check
whether any thread is replaying. This is unnecessary as it only matters
if inferior_ptid is replaying.
Narrow the check to inferior_ptid.
|
|
The record btrace target does not create or destroy threads. There is no
reason to override the update_thread_list() and thread_alive() target
methods.
|
|
Similar to the parent commit, simplify schedlock_applies() by only
checking the argument thread.
When resuming that thread, GDB will automatically stop replaying its
inferior. The replay state of other inferiors is not considered by
user_visible_resume_ptid(), so let's not consider them in
schedlock_applies(), either.
|
|
When scheduler-locking is set to replay and we're resuming a thread at the
end of its execution history, we check whether anything is replaying in
user_visible_resume_ptid() only to check again in clear_proceed_status()
before we stop replaying the current process.
What really matters is whether the selected thread is replaying or will
start replaying.
Simplify this by removing redundant checks.
Also avoid a redundant pass over all threads to check whether anything is
replaying before stopping replaying. Make record_stop_replaying() handle
the case when we're not replaying gracefully.
|
|
remote::wait () may get called rather frequently, polluting the logging
output with tons of
[remote] wait: enter
[remote] wait: exit
messages.
Similarly, remote_target::remote_notif_remove_queued_reply () will print
the debug message even if nothing was actually removed. Change that to
only print a debug message if a stop reply was removed.
|
|
When record_btrace::wait() is called and no threads are moving, we set the
wait status to no_resumed. Change that to ignore.
This helps with enabling per-inferior run-control for the record btrace
target as it avoids breaking out of do_target_wait() too early with
no_resumed when there would have been an event on another thread.
|
|
We don't really need three inferiors to test multi-inferior recording.
We don't really need to start recording on the second inferior first.
We don't really need to check info record before starting recording.
If we were recording, there would be output, causing a fail.
This just complicates the test when there is something to debug.
|
|
As a side effect of the 'record stop' command, the selected thread may
move to the end of the execution history if it had been replaying.
Call notify_normal_stop () in this case to tell users about the change in
location.
|
|
GDB fails with an assertion when stopping recording on a replaying thread
and then resuming that thread. Stopping recording left the thread
replaying but the record target is gone.
Stop replaying all threads in the selected inferior before stopping recording.
I had to change the stepping test slightly to account for different
compilers generating slightly different debug information, so when
stepping the 'return 0' after 'record stop' I would end up in a different
location depending on which compiler I used. The test still covers all
stepping commands.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19340
|
|
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>
|
