| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Clang does not add line information for lines that only contain a
closing } in functions. Many tests in the gdb.reverse folder set a
breakpoint in that line, but don't seem to use information available
after the return statement is executed, so this commit moves the
breakpoint to the previous line, where the return statement is.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Run black on make-target-delegates.py to fix buildbot build breaker.
Tested on x86_64-linux.
|
|
When doing this in target.h:
...
- virtual gdb::byte_vector thread_info_to_thread_handle (struct thread_info *)
+ virtual gdb::byte_vector &thread_info_to_thread_handle (struct thread_info *)
...
make-target-delegates.py drops the function.
By handling '&' in POINTER_PART we can prevent that the function is dropped,
but when recompiling target.o we get:
...
gdb/target-delegates.c: In member function ‘virtual gdb::byte_vector& \
debug_target::thread_info_to_thread_handle(thread_info*)’:
gdb/target-delegates.c:1889:22: error: ‘result’ declared as reference but not \
initialized
gdb::byte_vector & result;
^~~~~~
make: *** [Makefile:1923: target.o] Error 1
...
Fix this by making sure result is initialized.
Regenerate target-delegates.c using this new style.
Tested on x86_64-linux.
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Since b080fe54fb3 "gdb: add inferior-specific breakpoints", the
breakpoint class has an "inferior" member used to handle
inferior-specific breakpoints. This creates a compilation error
in amd_dbgapi_target_breakpoint::check_status which declares a local
variable "inferior *inf".
Fix this by using "struct inferior *inf" instead.
Change-Id: Icc4dc1ba96c7d3ff9d33f9cb384ffcf64eba26fb
Approved-By: Pedro Alves <pedro@palves.net>
|
|
After running a number of programs under Windows gdb and detaching
them, I typed run in gdb, and got a hang, here:
(top-gdb) bt
#0 sharing_input_terminal (pid=4672) at /home/pedro/gdb/src/gdb/mingw-hdep.c:388
#1 0x00007ff71a2d8678 in sharing_input_terminal (inf=0x23bf23dafb0) at /home/pedro/gdb/src/gdb/inflow.c:269
#2 0x00007ff71a2d887b in child_terminal_save_inferior (self=0x23bf23de060) at /home/pedro/gdb/src/gdb/inflow.c:423
#3 0x00007ff71a2c80c0 in inf_child_target::terminal_save_inferior (this=0x23bf23de060) at /home/pedro/gdb/src/gdb/inf-child.c:111
#4 0x00007ff71a429c0f in target_terminal_is_ours_kind (desired_state=target_terminal_state::is_ours_for_output) at /home/pedro/gdb/src/gdb/target.c:1037
#5 0x00007ff71a429e02 in target_terminal::ours_for_output () at /home/pedro/gdb/src/gdb/target.c:1094
#6 0x00007ff71a2ccc8e in post_create_inferior (from_tty=0) at /home/pedro/gdb/src/gdb/infcmd.c:245
#7 0x00007ff71a2cd431 in run_command_1 (args=0x0, from_tty=0, run_how=RUN_NORMAL) at /home/pedro/gdb/src/gdb/infcmd.c:502
#8 0x00007ff71a2cd58b in run_command (args=0x0, from_tty=0) at /home/pedro/gdb/src/gdb/infcmd.c:527
The problem is that the loop around GetConsoleProcessList looped
forever, because there were exactly 10 processes to return.
GetConsoleProcessList's documentation says:
If the buffer is too small to hold all the valid process identifiers,
the return value is the required number of array elements. The
function will have stored no identifiers in the buffer. In this
situation, use the return value to allocate a buffer that is large
enough to store the entire list and call the function again.
In this case, the buffer wasn't too small, it was exactly the right
size, so we should have broken out of the loop. We didn't due to a
"<" check that should have been "<=". That is fixed by this patch.
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Change-Id: I14e4909f2ac2fa83d0d9b6e64418b5831ac4e4e3
|
|
Spotted a few places where a char is being treated as a bool. The GDB
style is to use explicit comparisons, so fix things up.
There should be no user visible changes after this commit.
|
|
This recent commit:
commit b1c0ab20809a502b2d2224fecb0dca3ada2e9b22
Date: Wed Jul 12 21:56:50 2023 +0100
gdb: avoid double stop after failed breakpoint condition check
Addressed a problem where two MI stopped events would be reported if a
breakpoint condition failed due to a signal, this commit was a
replacement for this commit:
commit 2e411b8c68eb2b035b31d5b00d940d4be1a0928b
Date: Fri Oct 14 14:53:15 2022 +0100
gdb: don't always print breakpoint location after failed condition check
which solved the two stop problem, but only for the CLI. Before both
of these commits, if a b/p condition failed due to a signal then the
user would see two stops reported, the first at the location where the
signal occurred, and the second at the location of the breakpoint.
By default GDB remains at the location where the signal occurred, so
the second reported stop can be confusing, this is the problem that
commit 2e411b8c68eb tried to solve (for the CLI) and b1c0ab20809a
extended also address the issue for MI too.
However, while working on another patch I realised that there was a
problem with GDB after the above commits. Neither of the above
commits considered 'set unwindonsignal on'. With this setting on,
when an inferior function call fails with a signal GDB will unwind the
stack back to the location where the inferior function call started.
In the b/p case we're looking at, the stop should be reported at the
location of the breakpoint, not at the location where the signal
occurred, and this isn't what happens.
This commit fixes this by ensuring that when unwindonsignal is 'on',
GDB reports a single stop event at the location of the breakpoint,
this fixes things for both CLI and MI.
The function call_thread_fsm::should_notify_stop is called when the
inferior function call completes and GDB is figuring out if the user
should be notified about this stop event by calling normal_stop from
fetch_inferior_event in infrun.c. If normal_stop is called, then this
notification will be for the location where the inferior call stopped,
which will be the location at which the signal occurred.
Prior to this commit, the only time that normal_stop was not called,
was if the inferior function call completed successfully, this was
controlled by ::should_notify_stop, which only turns false when the
inferior function call has completed successfully.
In this commit I have extended the logic in ::should_notify_stop. Now
there are three cases in which ::should_notify_stop will return false,
and we will not announce the first stop (by calling normal_stop).
These three reasons are:
1. If the inferior function call completes successfully, this is
unchanged behaviour,
2. If the inferior function call stopped due to a signal and 'set
unwindonsignal on' is in effect, and
3. If the inferior function call stopped due to an uncaught C++
exception, and 'set unwind-on-terminating-exception on' is in
effect.
However, if we don't call normal_stop then we need to call
async_enable_stdin in call_thread_fsm::should_stop. Prior to this
commit this was only done for the case where the inferior function
call completed successfully.
In this commit I now call ::should_notify_stop and use this to
determine if we need to call async_enable_stdin. With this done we
now call async_enable_stdin for each of the three cases listed above,
which means that GDB will exit wait_sync_command_done correctly (see
run_inferior_call in infcall.c).
With these two changes the problem is mostly resolved. However, the
solution isn't ideal, we've still lost some information.
Here is how GDB 13.1 behaves, this is before commits b1c0ab20809a and
2e411b8c68eb:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Program received signal SIGSEGV, Segmentation fault.
0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
24 return *p; /* Crash here. */
Error in testing breakpoint condition:
The program being debugged was signaled while in a function called from GDB.
GDB has restored the context to what it was before the call.
To change this behavior use "set unwindonsignal off".
Evaluation of the expression containing the function
(cond_fail) will be abandoned.
Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
30 global_counter += 1; /* Set breakpoint here. */
(gdb)
In this state we see two stop notifications, the first is where the
signal occurred, while the second is where the breakpoint is located.
As GDB has unwound the stack (thanks to unwindonsignal) the second
stop notification reflects where the inferior is actually located.
Then after commits b1c0ab20809a and 2e411b8c68eb the behaviour changed
to this:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Program received signal SIGSEGV, Segmentation fault.
0x0000000000401116 in cond_fail () at /tmp/mi-condbreak-fail.c:24
24 return *p; /* Crash here. */
Error in testing condition for breakpoint 1:
The program being debugged was signaled while in a function called from GDB.
GDB has restored the context to what it was before the call.
To change this behavior use "set unwindonsignal off".
Evaluation of the expression containing the function
(cond_fail) will be abandoned.
(gdb) bt 1
#0 foo () at /tmp/mi-condbreak-fail.c:30
(More stack frames follow...)
(gdb)
This is the broken state. GDB is reports the SIGSEGV location, but
not the unwound breakpoint location. The final 'bt 1' shows that the
inferior is not located in cond_fail, which is the only location GDB
reported, so this is clearly wrong.
After implementing the fixes described above we now get this
behaviour:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Error in testing breakpoint condition for breakpoint 1:
The program being debugged was signaled while in a function called from GDB.
GDB has restored the context to what it was before the call.
To change this behavior use "set unwindonsignal off".
Evaluation of the expression containing the function
(cond_fail) will be abandoned.
Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
30 global_counter += 1; /* Set breakpoint here. */
(gdb)
This is better. GDB now reports a single stop at the location of the
breakpoint, which is where the inferior is actually located. However,
by removing the first stop notification we have lost some potentially
useful information about which signal caused the inferior to stop.
To address this I've reworked the message that is printed to include
the signal information. GDB now reports this:
$ gdb -q /tmp/mi-condbreak-fail \
-ex 'set unwindonsignal on' \
-ex 'break 30 if (cond_fail())' \
-ex 'run'
Reading symbols from /tmp/mi-condbreak-fail...
Breakpoint 1 at 0x40111e: file /tmp/mi-condbreak-fail.c, line 30.
Starting program: /tmp/mi-condbreak-fail
Error in testing condition for breakpoint 1:
The program being debugged received signal SIGSEGV, Segmentation fault
while in a function called from GDB. GDB has restored the context
to what it was before the call. To change this behavior use
"set unwindonsignal off". Evaluation of the expression containing
the function (cond_fail) will be abandoned.
Breakpoint 1, foo () at /tmp/mi-condbreak-fail.c:30
30 global_counter += 1; /* Set breakpoint here. */
(gdb)
This is better, the user now sees a single stop notification at the
correct location, and the error message describes which signal caused
the inferior function call to stop.
However, we have lost the information about where the signal
occurred. I did consider trying to include this information in the
error message, but, in the end, I opted not too. I wasn't sure it was
worth the effort. If the user has selected to unwind on signal, then
surely this implies they maybe aren't interested in debugging failed
inferior calls, so, hopefully, just knowing the signal name will be
enough. I figure we can always add this information in later if
there's a demand for it.
|
|
New helper proc mi_info_frame which takes care of running the MI
-stack-info-frame command and matching its output.
Like the breakpoint helper procs, this new proc takes a name/value
argument list and uses this to build the expected result regexp. This
means that we can now write something like:
mi_info_frame "test name here" \
-level 0 -func name -line 123
Instead of the current equivalent:
mi_gdb_test "235-stack-info-frame" \
"235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"name\",file=\".*\",fullname=\".*\",line=\"123\",arch=\".*\"\}" \
"test name here"
There's also a helper proc mi_make_info_frame_regexp which is
responsible for building the 'frame={...}' part of the pattern.
I've update the two existing tests that use -stack-info-frame and
expect the command to succeed. There is another test that runs
-stack-info-frame and expects the command to fail -- the helper proc
doesn't help with this case, so that test is not changed.
|
|
Currently, each target backend is responsible for printing "[Thread
...exited]" before deleting a thread. This leads to unnecessary
differences between targets, like e.g. with the remote target, we
never print such messages, even though we do print "[New Thread ...]".
E.g., debugging the gdb.threads/attach-many-short-lived-threads.exp
with gdbserver, letting it run for a bit, and then pressing Ctrl-C, we
currently see:
(gdb) c
Continuing.
^C[New Thread 3850398.3887449]
[New Thread 3850398.3887500]
[New Thread 3850398.3887551]
[New Thread 3850398.3887602]
[New Thread 3850398.3887653]
...
Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
78 in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
(gdb)
Above, we only see "New Thread" notifications, even though threads
were deleted.
After this patch, we'll see:
(gdb) c
Continuing.
^C[Thread 3558643.3577053 exited]
[Thread 3558643.3577104 exited]
[Thread 3558643.3577155 exited]
[Thread 3558643.3579603 exited]
...
[New Thread 3558643.3597415]
[New Thread 3558643.3600015]
[New Thread 3558643.3599965]
...
Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
78 in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
(gdb) q
This commit fixes this by moving the thread exit printing to common
code instead, triggered from within delete_thread (or rather,
set_thread_exited).
There's one wrinkle, though. While most targest want to print:
[Thread ... exited]
the Windows target wants to print:
[Thread ... exited with code <exit_code>]
... and sometimes wants to suppress the notification for the main
thread. To address that, this commits adds a delete_thread_with_code
function, only used by that target (so far).
This fix was originally posted as part of a larger series:
https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/
But didn't really need to be part of that series. In order to get
this fix merged sooner, I (Andrew Burgess) have rebased this commit
outside of the original series. Any bugs introduced while splitting
this patch out and rebasing, are entirely my own.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30129
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
|
|
After the previous commit, exit_inferior_1 no longer makes use of the
silent parameter. This commit removes this parameter and cleans up
the callers.
After doing this exit_inferior_1, exit_inferior, and
exit_inferior_silent are all equivalent, so rename exit_inferior_1 to
exit_inferior and delete exit_inferior_silent, update all the callers.
Also I spotted the declaration exit_inferior_num_silent in inferior.h,
but this function is not defined anywhere, so I deleted the
declaration.
There should be no user visible changes after this commit.
|
|
After this commit:
commit a78ef8757418105c35685c5d82b9fdf79459321b
Date: Wed Jun 22 18:10:00 2022 +0100
Always emit =thread-exited notifications, even if silent
The function mi_interp::on_thread_exited (or mi_thread_exit as the
function was called back then) no longer makes use of the "silent"
parameter.
As a result there is no difference between inferior::clear_thread_list
with silent true or false, because:
- None of the interpreter ::on_thread_exited functions rely on the
silent parameter, and
- None of GDB's thread_exit observers rely on the silent parameter
either.
This commit removes the silent parameter from
inferior::clear_thread_list, and makes the function always silent.
This commit was originally part of a larger series:
https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/
But didn't really need to be part of that series. I had an interest
in seeing this patch merged:
https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-31-pedro@palves.net/
Which also didn't really need to be part of the larger series, but
does depend, at least a little, on this commit. In order to get the
fix I'm interested in merged quicker, I (Andrew Burgess) have rebased
this commit outside of the original series. Any bugs introduced while
splitting this patch out and rebasing, are entirely my own.
There should be no user visible changes after this commit.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
|
|
Remove the static mi_parse::make functions, and instead use the
mi_parse constructor.
This is a partial revert of the commit:
commit fde3f93adb50c9937cd2e1c93561aea2fd167156
Date: Mon Mar 20 10:56:55 2023 -0600
Introduce "static constructor" for mi_parse
which introduced the mi_parse::make functions, though after discussion
on the list the reasons for seem to have been lost[1]. Given there
are no test regressions when moving back to using the constructors, I
propose we should do that for now.
There should be no user visible changes after this commit.
[1] https://inbox.sourceware.org/gdb-patches/20230404-dap-loaded-sources-v2-5-93f229095e03@adacore.com/
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Have the mi_out_new function return a std::unique_ptr instead of a raw
pointer. Update the two uses of mi_out_new.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
While GDB is still C++11, lets add a gdb::make_unique template
function that can be used to create std::unique_ptr objects, just like
the C++14 std::make_unique.
If GDB is being compiled with a C++14 compiler then the new
gdb::make_unique function will delegate to the std::make_unique. I
checked with gcc, and at -O1 and above gdb::make_unique will be
optimised away completely in this case.
If C++14 (or later) becomes our minimum, then it will be easy enough
to go through the code and replace gdb::make_unique with
std::make_unique later on.
I've make use of this function in all the places I think this can
easily be used, though I'm sure I've probably missed some.
Should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In this commit:
commit b080fe54fb3414b488b8ef323c6c50def061f918
Date: Tue Nov 8 12:32:51 2022 +0000
gdb: add inferior-specific breakpoints
limited support was added in lib/mi-support.exp to help with testing
of inferior specific breakpoints.
Though the changes that were added were not wrong, while working on a
later patch, I realised that I had added the support in the wrong
place -- I only added support to mi_make_breakpoint_multi, when really
I should have added the support to mi_make_breakpoint_1, which is used
by all of the MI procs that create breakpoints.
This commit moves the support to mi_make_breakpoint_1, and updates all
the procs that use mi_make_breakpoint_1 to accept, and then pass
through, and (optional) inferior argument. This will make it much
easier to write MI tests for inferior specific breakpoints.
There's no change in what is tested after this commit.
|
|
The commit:
commit b080fe54fb3414b488b8ef323c6c50def061f918
Date: Tue Nov 8 12:32:51 2022 +0000
gdb: add inferior-specific breakpoints
introduced a bug in the function breakpoint_set_inferior. The above
commit includes this line:
gdb::observers::breakpoint_modified.notify (b);
when it should have instead used this line:
notify_breakpoint_modified (b);
The change to use notify_breakpoint_modified was introduced to GDB
after commit b080fe54fb34 was written, but before it was merged, and I
failed to update this part of the code during the rebase.
The consequence of this error is that the MI interpreter will not emit
breakpoint-modified notifications when breakpoint_set_inferior is
called.
In this commit I update the code to call notify_breakpoint_modified,
and add a test that checks the MI events are being emitted correctly
in this case.
|
|
Some places matching the first char of a string against
bfd_get_symbol_leading_char, which may be zero, didn't check for "".
This could lead to accesses past the end of the string and potential
buffer overruns. Fix that, and also get rid of a stupid optimisation
in dbxread when looking for "__DYNAMIC" that also might access past
the end of a string.
|
|
When building gdb with -flto -O2, I run into:
...
opcodes/mep-desc.h:250:14: warning: type 'cgen_operand_type' violates the \
C++ One Definition Rule [-Wodr]
typedef enum cgen_operand_type {
^
opcodes/or1k-desc.h:624:14: note: an enum with different value name is \
defined in another translation unit
typedef enum cgen_operand_type {
^
opcodes/mep-desc.h:212:14: warning: type 'cgen_hw_type' violates the C++ One \
Definition Rule [-Wodr]
typedef enum cgen_hw_type {
^
opcodes/or1k-desc.h:433:14: note: an enum with different value name is \
defined in another translation unit
typedef enum cgen_hw_type {
^
...
Fix this by making the conflicting type names unique, adding a target-specific
prefix using a define before the include:
...
#define cgen_operand_type <target-name>_cgen_operand_type
#define cgen_hw_type <target-name>_cgen_hw_type
#include "opcodes/<target-name>-desc.h"
...
and move those defines into a new file cgen-remap.h, similar to how that's
done for yacc in yy-remap.h.
Likewise for targets frv and lm32, the two other targets that include
opcodes/<target-name>-desc.h.
Likewise for more cgen symbols that I got the same warning for when using
-flto-partition=one.
A PR has been filed to take care of this in the opcodes dir instead (PR30758).
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR build/30757
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30757
|
|
I noticed a call to value::copy in gdbpy_get_varobj_pretty_printer,
and I couldn't figure out why it was there. I think maybe it came
from the time when value_to_value_object would release values from the
value chain -- but that was removed in commit f3d3bbbc.
This patch removes this call. Regression tested on x86-64 Fedora 36.
|
|
While looking at a bug, I noticed what I think is an off-by-one
mistake in a call to vector::reserve. This code:
new_args.reserve (args.size ());
new_args.push_back
(value_from_pointer (lookup_pointer_type (values_type), struct_addr));
new_args.insert (new_args.end (), args.begin (), args.end ());
... reserves 'size()' entries, but then proceeds to push one extra
one.
This shouldn't have any really bad effects, as insert will grow the
vector. Still, it seems better to use the correct size if we're going
to bother calling reserve.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30780
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
|
|
psympriv.h was intended for use by code that created partial symbols.
Now that no generic code needs psymtab.h any more, psympriv.h can be
merged into psymtab.h.
|
|
I found that most spots including psymtab.h do not need it. This
patch removes these includes, and also one unnecessary include of
psympriv.h.
|
|
I noticed minidebug.c was still using explicit malloc and free, where
a vector would be more automatic.
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
|
|
This commit extends the breakpoint mechanism to allow for inferior
specific breakpoints (but not watchpoints in this commit).
As GDB gains better support for multiple connections, and so for
running multiple (possibly unrelated) inferiors, then it is not hard
to imagine that a user might wish to create breakpoints that apply to
any thread in a single inferior. To achieve this currently, the user
would need to create a condition possibly making use of the $_inferior
convenience variable, which, though functional, isn't the most user
friendly.
This commit adds a new 'inferior' keyword that allows for the creation
of inferior specific breakpoints.
Inferior specific breakpoints are automatically deleted when the
associated inferior is removed from GDB, this is similar to how
thread-specific breakpoints are deleted when the associated thread is
deleted.
Watchpoints are already per-program-space, which in most cases mean
watchpoints are already inferior specific. There is a small window
where inferior-specific watchpoints might make sense, which is after a
vfork, when two processes are sharing the same address space.
However, I'm leaving that as an exercise for another day. For now,
attempting to use the inferior keyword with a watchpoint will give an
error, like this:
(gdb) watch a8 inferior 1
Cannot use 'inferior' keyword with watchpoints
A final note on the implementation: currently, inferior specific
breakpoints, like thread-specific breakpoints, are inserted into every
inferior, GDB then checks once the inferior stops if we are in the
correct thread or inferior, and resumes automatically if we stopped in
the wrong thread/inferior.
An obvious optimisation here is to only insert breakpoint locations
into the specific program space (which mostly means inferior) that
contains either the inferior or thread we are interested in. This
would reduce the number times GDB has to stop and then resume again in
a multi-inferior setup.
I have a series on the mailing list[1] that implements this
optimisation for thread-specific breakpoints. Once this series has
landed I'll update that series to also handle inferior specific
breakpoints in the same way. For now, inferior specific breakpoints
are just slightly less optimal, but this is no different to
thread-specific breakpoints in a multi-inferior debug session, so I
don't see this as a huge problem.
[1] https://inbox.sourceware.org/gdb-patches/cover.1685479504.git.aburgess@redhat.com/
|
|
When building gdb with -O2 -flto on openSUSE Tumbleweed (using bison 3.8.2) I
run into:
...
ada-exp.c.tmp:653: warning: type 'yysymbol_kind_t' violates the C++ One \
Definition Rule [-Wodr]
c-exp.c.tmp:398: note: an enum with different value name is defined in \
another translation unit
ada-exp.c.tmp:660: note: name 'YYSYMBOL_NULL_PTR' differs from name \
'YYSYMBOL_COMPLEX_INT' defined in another translation unit
c-exp.c.tmp:405: note: mismatching definition
...
Fix this by renaming to ada_exp_yysymbol_kind_t and likewise for other .y
files.
Tested on x86_64-linux.
PR build/22395
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22395
|
|
In target_read_auxv we return a copy of an object:
...
gdb::optional<gdb::byte_vector>
target_read_auxv ()
{
...
return info->data;
}
...
Return a const reference instead, saving a copy.
This is exposed by using std::pmr::polymorphic_allocator instead of
std::allocator in default_init_allocator.
Tested on x86_64-linux.
|
|
When building gdb with -std=c++20, I run into:
...
gdb/dwarf2/read.c:2709:3: error: invalid conversion from ‘const char8_t*’ to \
‘const char*’ [-fpermissive]
2709 | u8"u8função",
| ^~~~~~~~~~~~
| |
| const char8_t*
...
Fix this by making the conversion explicit.
Tested on x86_64-linux.
|
|
When building gdb with -std=c++20 I run into:
...
gdb/ada-lang.c:10713:16: error: implicit capture of ‘this’ via ‘[=]’ is \
deprecated in C++20 [-Werror=deprecated]
10713 | auto do_op = [=] (LONGEST x, LONGEST y)
| ^
gdb/ada-lang.c:10713:16: note: add explicit ‘this’ or ‘*this’ capture
...
Fix this by using "[this]".
Likewise in two more spots.
Tested on x86_64-linux.
|
|
When building gdb with -std=c++20, I run into:
...
include/ansidecl.h:342:9: error: expected unqualified-id before ‘const’
342 | TYPE (const TYPE&) = delete; \
| ^~~~~
gdb/ui-out.h:412:3: note: in expansion of macro ‘DISABLE_COPY_AND_ASSIGN’
412 | DISABLE_COPY_AND_ASSIGN (ui_out_emit_type<Type>);
| ^~~~~~~~~~~~~~~~~~~~~~~
...
Fix this by using "DISABLE_COPY_AND_ASSIGN (ui_out_emit_type)".
Tested on x86_64-linux.
|
|
When building gdb with clang 15 and -std=c++20, I run into:
...
gdbsupport/poison.h:52:11: error: 'is_pod<timeval>' is deprecated: use \
is_standard_layout && is_trivial instead [-Werror,-Wdeprecated-declarations]
std::is_pod<T>>
^
...
Fix this by following the suggestion.
Likewise in gdb/unittests/ptid-selftests.c.
Tested on x86_64-linux.
|
|
When building gdb with clang 15 and -std=c++20, I run into:
...
gdbsupport/common-exceptions.h:203:32: error: arithmetic between different \
enumeration types ('const enum return_reason' and 'const enum errors') is \
deprecated [-Werror,-Wdeprecated-enum-enum-conversion]
size_t result = exc.reason + exc.error;
~~~~~~~~~~ ^ ~~~~~~~~~
...
Fix this by using to_underlying.
Likewise in a few other places.
Tested on x86_64-linux.
|
|
When running test-case gdb.base/vfork-follow-parent.exp, I run into:
...
ERROR: tcl error sourcing gdb/testsuite/gdb.base/vfork-follow-parent.exp.
ERROR: error copying "vforked-prog": no such file or directory
while executing
"file copy -force $fromfile $tofile"
(procedure "gdb_remote_download" line 29)
invoked from within
"gdb_remote_download target $binfile3"
...
Fix this by:
- making the copy-to-remote conditional on is_remote target, and
- allowing gdb_remote_download to find $binfile3 by using
standard_output_file.
Also remove unused variable remote_exec_prog.
Tested on x86_64-linux.
|
|
While working on a dwarf assembly test-case I accidentally created the
following pathological dwarf:
...
<1><be>: Abbrev Number: 3 (DW_TAG_class_type)
<bf> DW_AT_name : c1
<c2> DW_AT_specification: <0xbe>
...
and noticed gdb segfaulting during cooked index creating due to running out of
stack. This is a regression from gdb-12, where gdb just hung.
Fix this by inhibiting the scan_attributes self-recursion for self-references.
The same test-case with -readnow makes gdb hang, so also fix this in
dwarf2_attr and follow_die_ref.
Note that this doesn't fix the same problems for the more complicated case of:
...
<1><be>: Abbrev Number: 3 (DW_TAG_class_type)
<bf> DW_AT_name : c1
<c2> DW_AT_specification: <0xc6>
<1><c6>: Abbrev Number: 4 (DW_TAG_class_type)
<c7> DW_AT_name : c2
<ca> DW_AT_specification: <0xbe>
...
but the approach for deciding whether to fix pathological dwarf cases is as
per PR27981 comment 3:
...
yes if it is cheap/obvious, and no if it is something complicated or expensive.
...
and at this point I'm not sure whether fixing this will fall in the first
category.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
A bug report pointed out a buffer overflow in ada_decode, which Keith
helpfully analyzed. ada_decode had a logic error when the input was
all digits. While this isn't valid -- and would probably only appear
in fuzzer tests -- it still should be handled properly.
This patch adds a missing bounds check. Tested with the self-tests in
an asan build.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30639
Reviewed-by: Keith Seitz <keiths@redhat.com>
|
|
I found an obvious bug in Ada aggregate expression handling:
if (vvo != nullptr)
error (_("Invalid record component association."));
name = vvo->get_symbol ()->natural_name ();
Here the code errors when vvo is not null -- and then proceeds to use
vvo.
This hasn't caused a crash because, I believe, there's currently no
way to reach this code in the null case. However, I'm not really
willing to assert this...
Fixing this shows another bug, which is that due to the way the parser
works, a field name in an aggregate expression might erroneously be
fully qualified if some global variable with the same base name
exists.
The included test case triggers both bugs. Note that the test
includes a confounding case for array aggregates as well, but as these
are harder to fix, I've left it as kfail.
As this is Ada-specific, and has already been tested internally at
AdaCore, I am checking it in.
|
|
DAP specifies an event that should be sent when a module is removed.
This patch implements this.
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
|
|
I ran into a test failure on gdb.python/py-thread-exited.c. The test
creates two threads and then catches the thread exits in Python. The
test expects the threads to exit in a specific order.
As the test is currently written, it is _likely_, but not guaranteed,
that the threads will exit in the same order they are created, which
is what the test expects.
When running on a loaded system I ran into a case where the threads
exited in the reverse creation order, which caused the test to fail.
I could fix this by having the .exp file not care about the thread
order, or by changing the C file to force the order. I chose the
later, and added a pthread_barrier_t to ensure the threads exit in the
correct order.
There should be no change in what is tested after this commit.
|
|
It was pointed out on the mailing list[1] that after this commit:
commit b1e0126ec56e099d753c20e91a9f8623aabd6b46
Date: Wed Jun 21 14:18:54 2023 +0100
gdb: don't resume vfork parent while child is still running
the test gdb.base/vfork-follow-parent.exp now has some failures when
run with the native-gdbserver or native-extended-gdbserver boards:
FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to end of inferior 2 (timeout)
FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: inferior 1 (timeout)
FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: print unblock_parent = 1 (timeout)
FAIL: gdb.base/vfork-follow-parent.exp: resolution_method=schedule-multiple: continue to break_parent (timeout)
The reason that these failures don't show up when run on the standard
unix board is that the test is only run in the default operating mode,
so for Linux this will be all-stop on top of non-stop.
If we adjust the test script so that it runs in the default mode and
with target-non-stop turned off, then we see the same failures on the
unix board. This commit includes this change.
The way that the test is written means that it is not (currently)
possible to turn on non-stop mode and have the test still work, so
this commit does not do that.
I have also updated the test script so that the vfork child performs
an exec as well as the current exit. Exec and exit are the two ways
in which a vfork child can release the vfork parent, so testing both
of these cases is useful I think.
In this test the inferior performs a vfork and the vfork-child
immediately exits. The vfork-parent will wait for the vfork-child and
then blocks waiting for gdb. Once gdb has released the vfork-parent,
the vfork-parent also exits.
In the test that fails, GDB sets 'detach-on-fork off' and then runs to
the vfork. At this point the test tries to just "continue", but this
fails as the vfork-parent is still selected, and the parent can't
continue until the vfork-child completes. As the vfork-child is
stopped by GDB the parent will never stop once resumed, so GDB refuses
to resume it.
The test script then sets 'schedule-multiple on' and once again
continues. This time GDB, in theory, resumes both the parent and the
child, the parent will be held blocked by the kernel, but the child
will run until it exits, and which point GDB stops again, this time
with inferior 2, the newly exited vfork-child, selected.
What happens after this in the test script is irrelevant as far as
this failure is concerned.
To understand why the test started failing we should consider the
behaviour of four different cases:
1. All-stop-on-non-stop before commit b1e0126ec56e,
2. All-stop-on-non-stop after commit b1e0126ec56e,
3. All-stop-on-all-stop before commit b1e0126ec56e, and
4. All-stop-on-all-stop after commit b1e0126ec56e.
Only case #4 is failing after commit b1e0126ec56e, but I think the
other cases are interesting because, (a) they inform how we might fix
the regression, and (b) it turns out the behaviour of #2 changed too
with the commit, but the change was harmless.
For #1 All-stop-on-non-stop before commit b1e0126ec56e, what happens
is:
1. GDB calls proceed with the vfork-parent selected, as schedule
multiple is on user_visible_resume_ptid returns -1 (everything)
as the resume_ptid (see proceed function),
2. As this is all-stop-on-non-stop, every thread is resumed
individually, so GDB tries to resume both the vfork-parent and the
vfork-child, both of which succeed,
3. The vfork-parent is held stopped by the kernel,
4. The vfork-child completes (exits) at which point the GDB sees the
EXITED event for the vfork-child and the VFORK_DONE event for the
vfork-parent,
5. At this point we might take two paths depending on which event
GDB handles first, if GDB handles the VFORK_DONE first then:
(a) As GDB is controlling both parent and child the VFORK_DONE is
ignored (see handle_vfork_done), the vfork-parent will be
resumed,
(b) GDB processes the EXITED event, selects the (now defunct)
vfork-child, and stops, returning control to the user.
Alternatively, if GDB selects the EXITED event first then:
(c) GDB processes the EXITED event, selects the (now defunct)
vfork-child, and stops, returning control to the user.
(d) At some future time the user resumes the vfork-parent, at
which point the VFORK_DONE is reported to GDB, however, GDB
is ignoring the VFORK_DONE (see handle_vfork_done), so the
parent is resumed.
For case #2, all-stop-on-non-stop after commit b1e0126ec56e, the
important difference is in step (2) above, now, instead of resuming
both the vfork-parent and the vfork-child, only the vfork-child is
resumed. As such, when we get to step (5), only a single event, the
EXITED event is reported.
GDB handles the EXITED just as in (5)(c), then, later, when the user
resumes the vfork-parent, the VFORKED_DONE is immediately delivered
from the kernel, but this is ignored just as in (5)(d), and so,
though the pattern of when the vfork-parent is resumed changes, the
overall pattern of which events are reported and when, doesn't
actually change. In fact, by not resuming the vfork-parent, the order
of events (in this test) is now deterministic, which (maybe?) is a
good thing.
If we now consider case #3, all-stop-on-all-stop before commit
b1e0126ec56e, then what happens is:
1. GDB calls proceed with the vfork-parent selected, as schedule
multiple is on user_visible_resume_ptid returns -1 (everything)
as the resume_ptid (see proceed function),
2. As this is all-stop-on-all-stop, the resume is passed down to the
linux-nat target, the vfork-parent is the event thread, while the
vfork-child is a sibling of the event thread,
3. In linux_nat_target::resume, GDB calls linux_nat_resume_callback
for all threads, this causes the vfork-child to be resumed. Then
in linux_nat_target::resume, the event thread, the vfork-parent,
is also resumed.
4. The vfork-parent is held stopped by the kernel,
5. The vfork-child completes (exits) at which point the GDB sees the
EXITED event for the vfork-child and the VFORK_DONE event for the
vfork-parent,
6. We are now in a situation identical to step (5) as for
all-stop-on-non-stop above, GDB selects one of the events to
handle, and whichever we select the user sees the correct
behaviour.
And so, finally, we can consider #4, all-stop-on-all-stop after commit
b1e0126ec56e, this is the case that started failing.
We start out just like above, in proceed, the resume_ptid is
-1 (resume everything), due to schedule multiple being on. And just
like above, due to the target being all-stop, we call
proceed_resume_thread_checked just once, for the current thread,
which, remember, is the vfork-parent thread.
The change in commit b1e0126ec56e was to avoid resuming a vfork-parent
thread, read the commit message for the justification for this change.
However, this means that GDB now rejects resuming the vfork-parent in
this case, which means that nothing gets resumed! Obviously, if
nothing resumes, then nothing will ever stop, and so GDB appears to
hang.
I considered a couple of solutions which, in the end, I didn't go
with, these were:
1. Move the vfork-parent check out of proceed_resume_thread_checked,
and place it in proceed, but only on the all-stop-on-non-stop
path, this should still address the issue seen in b1e0126ec56e,
but would avoid the issue seen here. I rejected this just
because it didn't feel great to split the checks that exist in
proceed_resume_thread_checked like this,
2. Extend the condition in proceed_resume_thread_checked by adding a
target_is_non_stop_p check. This would have the same effect as
idea 1, but leaves all the checks in the same place, which I
think would be better, but this still just didn't feel right to
me, and so,
What I noticed was that for the all-stop-on-non-stop, after commit
b1e0126ec56e, we only resumed the vfork-child, and this seems fine.
The vfork-parent isn't going to run anyway (the kernel will hold it
back), so if feels like we there's no harm in just waiting for the
child to complete, and then resuming the parent.
So then I started looking at follow_fork, which is called from the top
of proceed. This function already has the task of switching between
the parent and child based on which the user wishes to follow. So, I
wondered, could we use this to switch to the vfork-child in the case
that we are attached to both?
Turns out this is pretty simple to do.
Having done that, now the process is for all-stop-on-all-stop after
commit b1e0126ec56e, and with this new fix is:
1. GDB calls proceed with the vfork-parent selected, but,
2. In follow_fork, and follow_fork_inferior, GDB switches the
selected thread to be that of the vfork-child,
3. Back in proceed user_visible_resume_ptid returns -1 (everything)
as the resume_ptid still, but now,
4. When GDB calls proceed_resume_thread_checked, the vfork-child is
the current selected thread, this is not a vfork-parent, and so
GDB allows the proceed to continue to the linux-nat target,
5. In linux_nat_target::resume, GDB calls linux_nat_resume_callback
for all threads, this does not resume the vfork-parent (because
it is a vfork-parent), and then the vfork-child is resumed as
this is the event thread,
At this point we are back in the same situation as for
all-stop-on-non-stop after commit b1e0126ec56e, that is, the
vfork-child is resumed, while the vfork-parent is held stopped by
GDB.
Eventually the vfork-child will exit or exec, at which point the
vfork-parent will be resumed.
[1] https://inbox.sourceware.org/gdb-patches/3e1e1db0-13d9-dd32-b4bb-051149ae6e76@simark.ca/
|
|
Fixes the issue where jump failed if multiple inferiors run the same
source.
See the below example
$ gdb -q ./simple
Reading symbols from ./simple...
(gdb) break 2
Breakpoint 1 at 0x114e: file simple.c, line 2.
(gdb) run
Starting program: /temp/simple
Breakpoint 1, main () at simple.c:2
2 int a = 42;
(gdb) add-inferior
[New inferior 2]
Added inferior 2 on connection 1 (native)
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) info inferiors
Num Description Connection Executable
1 process 6250 1 (native) /temp/simple
* 2 <null> 1 (native)
(gdb) file ./simple
Reading symbols from ./simple...
(gdb) run
Starting program: /temp/simple
Thread 2.1 "simple" hit Breakpoint 1, main () at simple.c:2
2 int a = 42;
(gdb) info inferiors
Num Description Connection Executable
1 process 6250 1 (native) /temp/simple
* 2 process 6705 1 (native) /temp/simple
(gdb) jump 3
Unreasonable jump request
(gdb)
In this example, jump fails because the debugger finds two different
locations, one for each inferior.
Solution is to limit the search to the current program space.
This is done by having the jump_command function use
decode_line_with_current_source rather than
decode_line_with_last_displayed, which makes sense,
the *_current_source function always looks up a location based on the
current thread's location -- if a user is asking the current thread to
jump, then surely their destination should be relative to where the
current thread is located.
Then, inside decode_line_with_current_source, the call to
decode_line_1 is updated to pass through the current program_space,
which will limit the returned locations to those in the current
program space.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
From what I can tell, inferior::priv is reserved for the
process_stratum target. It seems to me that it has to be, because
currenlty only such targets use it, and if a target at another stratum
started using this field, then conflicts could occur. This patch
documents this.
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
|
|
When building gdb with -O2 -flto I run into:
...
ada-exp.c.tmp:576:7: error: type ‘union YYSTYPE’ violates the C++ One \
Definition Rule [-Werror=odr]
...
Fix this by renaming to ada_exp_YYSTYPE and likewise for other .y files.
Likewise for yyalloc.
Tested on x86_64-linux. Also tested with byacc rather than bison on
suggestion of Tom Tromey.
Approved-By: Tom Tromey <tom@tromey.com>
PR build/22395
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22395
|
|
In addition, detach from any child processes implicitly attached to by
the kernel due to fork following that have not yet been processed by
GDB's core.
|
|
FreeBSD's ptrace fails requests with EBUSY against a running process.
Report that the thread is alive instead of dead if ptrace fails with
EBUSY.
This fixes an internal error in the gdb.threads/detach-step-over.exp
test where one process was detached while a thread in a second process
was being stepped. The core incorrectly assumed the stepping thread
had vanished and discarded the pending stepping state. When the
thread later reported a SIGTRAP from completing the step, this
triggered an assertion.
|
|
- Detach from any child processes implicitly attached to by the kernel
due to fork following that have not yet been processed by GDB's
core.
- Delete breakpoints before detaching.
inf-ptrace::detach does not do this (somewhat surprisingly), so add
an override to remove breakpoints from a process before detaching
from it.
This also requires explicitly draining any pending SIGTRAP events
for software breakpoints before detaching. In particular, threads
may need their PC adjusted due to the software breakpoint before
being resumed after detach. On more modern systems using the si_code
from SIGTRAP to identify software breakpoint traps, the PC is adjusted
in ::wait_1 as a side effect of parsing the event. To support older
kernels, ::detach fixes up the PC for any SIGTRAP stop whose potential
new PC matches an existing software breakpoint.
|
|
I did not fully understand the requirements of multiple process
support when I enabled it previously and several parts were broken.
In particular, the resume method was only resuming a single process,
and wait was not stopping other processes when reporting an event.
To support multiple running inferiors, add a new per-inferior
structure which trackes the number of existing and running LWPs for
each process. The structure also stores a ptid_t describing the
set of LWPs currently resumed for each process.
For the resume method, iterate over all non-exited inferiors resuming
each process matching the passed in ptid rather than only resuming the
current inferior's process for a wildcard ptid. If a resumed process
has a pending event, don't actually resume the process, but other
matching processes without a pending event are still resumed in case
the later call to the wait method requests an event from one of the
processes without a pending event.
For the wait method, stop other running processes before returning an
event to the core. When stopping a process, first check to see if an
event is already pending. If it is, queue the event to be reported
later. If not, send a SIGSTOP to the process and wait for it to stop.
If the event reported by the wait is not for the SIGSTOP, queue the
event and remember to ignore a future SIGSTOP event for the process.
Note that, unlike the Linux native target, entire processes are
stopped rather than individual LWPs. In FreeBSD one can only wait on
processes (via pid), not for an event from a specific thread.
Other changes in this commit handle bookkeeping for the per-inferior
data such as migrating the data to the new inferior in the follow_exec
method. The per-inferior data is created in the attach,
create_inferior, and follow_fork methods.
|
|
If wait_1 finds an event for a thread or process that does not match
the set of threads and processes previously resumed, defer the event.
If the event is for a specific thread, suspend the thread and continue
the associated process before waiting for another event.
One specific example of such an event is if a thread is created while
another thread in the same process hits a breakpoint. If the second
thread's event is reported first, the target resume method does not
yet "know" about the new thread and will not suspend it via
PT_SUSPEND. When wait is called, it will probably return the event
from the first thread before the result of the step from second
thread. This is the case reported in PR 21497.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=21497
|
|
The m_pending_events list stores a queue of deferred events that might
be reported by the next call to the target's wait method. The set of
events that are eligible is filtered by the ptid passed to resume.
For now this just replaces the list of vfork_done events. A
subsequent commit will reuse this to store other events.
|
|
I noticed that the call to alloca in osabi.c can be replaced with a
statically-sized buffer, because some code just before the declaration
ensures that the length is bounded.
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
|
|
When building gdb with -O2 -flto I run into:
...
/data/vries/gdb/src/gdb/c-exp.y:2450:8: warning: type 'struct token' \
violates the C++ One Definition Rule [-Wodr]
struct token
^
/data/vries/gdb/src/gdb/d-exp.y:939:8: note: a different type is defined in \
another translation unit
struct token
^
...
Fix this by renaming to c_token and d_token.
Likewise in:
- fortran-exp.y, renaming to f_token,
- go-exp.y, renaming to go_token, and
- p-exp.y, renaming to p_token.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR build/22395
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22395
|
|
When build gdb with -O2 -flto I run into:
...
gdb/c-exp.y:3003:8: warning: type 'struct token_and_value' violates the C++ \
One Definition Rule [-Wodr]
struct token_and_value
^
gdb/d-exp.y:1310:8: note: a different type is defined in another translation \
unit
struct token_and_value
^
...
Fix this by renaming to c_token_and_value and d_token_and_value.
Likewise in gdb/go-exp.y, renaming to go_token_and_value.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR build/22395
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22395
|
