When GDB fails to test the condition of a conditional breakpoint, for
whatever reason, the error message looks like this:
(gdb) break foo if (*(int *) 0) == 1
Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
(gdb) r
Starting program: /tmp/bpcond
Error in testing breakpoint condition:
Cannot access memory at address 0x0
Breakpoint 1, foo () at bpcond.c:11
11 int a = 32;
(gdb)
The line I'm interested in for this commit is this one:
Error in testing breakpoint condition:
In the case above we can figure out that the problematic breakpoint
was #1 because in the final line of the message GDB reports the stop
at breakpoint #1.
However, in the next few patches I plan to change this. In some cases
I don't think it makes sense for GDB to report the stop as being at
breakpoint #1, consider this case:
(gdb) list some_func
1 int
2 some_func ()
3 {
4 int *p = 0;
5 return *p;
6 }
7
8 void
9 foo ()
10 {
(gdb) break foo if (some_func ())
Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
(gdb) r
Starting program: /tmp/bpcond
Program received signal SIGSEGV, Segmentation fault.
0x0000000000401116 in some_func () at bpcond.c:5
5 return *p;
Error in testing breakpoint condition:
The program being debugged was signaled while in a function called from GDB.
GDB remains in the frame where the signal was received.
To change this behavior use "set unwindonsignal on".
Evaluation of the expression containing the function
(some_func) will be abandoned.
When the function is done executing, GDB will silently stop.
Program received signal SIGSEGV, Segmentation fault.
Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5
5 return *p;
(gdb)
Notice that, the final lines of output reports the stop as being at
breakpoint #1, even though the inferior in not located within
some_func, and it's certainly not located at the breakpoint location.
I find this behaviour confusing, and propose that this should be
changed. However, if I make that change then every reference to
breakpoint #1 will be lost from the error message.
So, in this commit, in preparation for the later commits, I propose to
change the 'Error in testing breakpoint condition:' line to this:
Error in testing condition for breakpoint NUMBER:
where NUMBER will be filled in as appropriate. Here's the first
example with the updated error:
(gdb) break foo if (*(int *) 0) == 0
Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
(gdb) r
Starting program: /tmp/bpcond
Error in testing condition for breakpoint 1:
Cannot access memory at address 0x0
Breakpoint 1, foo () at bpcond.c:11
11 int a = 32;
(gdb)
The breakpoint number does now appear twice in the output, but I don't
see that as a negative.
This commit just changes the one line of the error, and updates the
few tests that either included the old error in comments, or actually
checked for the error in the expected output.
As the only test that checked the line I modified is a Python test,
I've added a new test that doesn't rely on Python that checks the
error message in detail.
While working on the new test, I spotted that it would fail when run
with native-gdbserver and native-extended-gdbserver target boards.
This turns out to be due to a gdbserver bug. To avoid cluttering this
commit I've added a work around to the new test script so that the
test passes for the remote boards, in the next few commits I will fix
gdbserver, and update the test script to remove the work around.
I noticed that breakpoint::print_recreate_thread was printing the
global thread-id. This function is used to implement the 'save
breakpoints' command, and should be writing out suitable CLI commands
for recreating the current breakpoints. The CLI does not use global
thread-ids, but instead uses the inferior specific thread-ids,
e.g. "2.1".
After some discussion on the mailing list it was suggested that the
most consistent solution would be for the saved breakpoints file to
always contain the inferior-qualified thread-id, so the file would
include "thread 1.1" instead of just "thread 1", even when there is
only a single inferior.
So, this commit adds print_full_thread_id, which is just like the
existing print_thread_id, only it always prints the inferior-qualified
thread-id.
I then update the existing print_thread_id to make use of this new
function, and finally, I update breakpoint::print_recreate_thread to
also use this new function.
There's a multi-inferior test that confirms the saved breakpoints file
correctly includes the fully-qualified thread-id, and I've also
updated the single inferior test gdb.base/save-bp.exp to have it
validate that the saved breakpoints file includes the
inferior-qualified thread-id, even for this single inferior case.
Since commit cb1e4e32c2d9 ("catch catch/throw/rethrow", breakpoint ->
catchpoint), this simple tracing scenario does not work:
$ gdb/gdb -nx -q --data-directory=gdb/data-directory ./test
Reading symbols from ./test...
(gdb) tar rem :1234
Remote debugging using :1234
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007ffff7fe5730 in ?? () from /lib64/ld-linux-x86-64.so.2
(gdb) trace do_something
Tracepoint 1 at 0x555555555144: file test.c, line 5.
(gdb) tstart
(gdb) continue
Continuing.
Target returns error code '01'.
The root cause is that the bp_location::nserted flag does not transfer
anymore from an old bp_location to the new matching one. When a shared
library gets loaded, GDB computes new breakpoint locations for each
breakpoint in update_breakpoint_locations. The new locations are in the
breakpoint::loc chain, while the old locations are still in the
bp_locations global vector. Later, update_global_location_list is
called. It tries to map old locations to new locations, and if
necessary transfer some properties, like the inserted flag.
Since commit cb1e4e32c2d9, the inserted flag isn't transferred for
locations of tracepoints. This is because bl_address_is_meaningful used
to be implemented like this:
static int
breakpoint_address_is_meaningful (struct breakpoint *bpt)
{
enum bptype type = bpt->type;
return (type != bp_watchpoint && type != bp_catchpoint);
}
and was changed to this:
static bool
bl_address_is_meaningful (bp_location *loc)
{
return loc->loc_type != bp_loc_other;
}
Because locations for tracepoints have the bp_loc_other type,
bl_address_is_meaningful started to return false for them, where it
returned true before. This made update_global_location_list skip the
part where it calls swap_insertion.
I think this can be solved by introduced a new bp_loc_tracepoint
bp_loc_type.
I don't know if it's accurate, but my understanding is that bp_loc_type
describes roughly "how do we ask the target to insert that location".
bp_loc_software_breakpoint are inserted using
target_ops::insert_breakpoint_location. bp_loc_hardware_breakpoint are
inserted using target_ops::insert_hw_breakpoint.
bp_loc_software_watchpoint and bp_loc_hardware_watchpoint are inserted
using target_ops::insert_watchpoint. For all these, the address is
meaningful, as we ask the target to insert the point at a specific
address. bp_loc_other is a catch-all for "the rest", in practice for
catchpoints that don't have a specific address (hence why
bl_address_is_meaningful returns false for them). For instance,
inserting a signal catchpoint is done by asking the target to report
that specific signal. GDB doesn't associate an address to that.
But tracepoints do have a meaningful address to thems, so they can't be
bp_loc_other, with that logic. They also can't be
bp_loc_software_breakpoint, because we don't want GDB to insert
breakpoints for them (even though they might be implemented using
software breakpoints by the remote side). So, the new bp_loc_tracepoint
type describes that the way to insert these locations is with
target_ops::download_tracepoint. It makes bl_address_is_meaningful
return true for them. And they'll be ignored by insert_bp_location and
GDB won't try to insert a memory breakpoint for them.
With this, I see a few instances of 'Target returns error code: 01'
disappearing from gdb.log, and the results of gdb.trace/*.exp improve a
little bit:
-# of expected passes 3765
+# of expected passes 3781
-# of unexpected failures 518
+# of unexpected failures 498
Things remain quite broken in that area though.
Change-Id: Ic40935c450410f4bfaba397c9ebc7faf97320dd3
Background
----------
When a thread-specific breakpoint is deleted as a result of the
specific thread exiting the function remove_threaded_breakpoints is
called which sets the disposition of the breakpoint to
disp_del_at_next_stop and sets the breakpoint number to 0. Setting
the breakpoint number to zero has the effect of hiding the breakpoint
from the user. We also print a message indicating that the breakpoint
has been deleted.
It was brought to my attention during a review of another patch[1]
that setting a breakpoints number to zero will suppress the MI
breakpoint-deleted notification for that breakpoint, and indeed, this
can be seen to be true, in delete_breakpoint, if the breakpoint number
is zero, then GDB will not notify the breakpoint_deleted observer.
It seems wrong that a user created, thread-specific breakpoint, will
have a =breakpoint-created notification, but will not have a
=breakpoint-deleted notification. I suspect that this is a bug.
[1] https://sourceware.org/pipermail/gdb-patches/2023-February/196560.html
The First Problem
-----------------
During my initial testing I wanted to see how GDB handled the
breakpoint after it's number was set to zero. To do this I created
the testcase gdb.threads/thread-bp-deleted.exp. This test creates a
worker thread, which immediately exits. After the worker thread has
exited the main thread spins in a loop.
In GDB I break once the worker thread has been created and place a
thread-specific breakpoint, then use 'continue&' to resume the
inferior in non-stop mode. The worker thread then exits, but the main
thread never stops - instead it sits in the spin. I then tried to use
'maint info breakpoints' to see what GDB thought of the
thread-specific breakpoint.
Unfortunately, GDB crashed like this:
(gdb) continue&
Continuing.
(gdb) [Thread 0x7ffff7c5d700 (LWP 1202458) exited]
Thread-specific breakpoint 3 deleted - thread 2 no longer in the thread list.
maint info breakpoints
... snip some output ...
Fatal signal: Segmentation fault
----- Backtrace -----
0x5ffb62 gdb_internal_backtrace_1
../../src/gdb/bt-utils.c:122
0x5ffc05 _Z22gdb_internal_backtracev
../../src/gdb/bt-utils.c:168
0x89965e handle_fatal_signal
../../src/gdb/event-top.c:964
0x8997ca handle_sigsegv
../../src/gdb/event-top.c:1037
0x7f96f5971b1f ???
/usr/src/debug/glibc-2.30-2-gd74461fa34/nptl/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
0xe602b0 _Z15print_thread_idP11thread_info
../../src/gdb/thread.c:1439
0x5b3d05 print_one_breakpoint_location
../../src/gdb/breakpoint.c:6542
0x5b462e print_one_breakpoint
../../src/gdb/breakpoint.c:6702
0x5b5354 breakpoint_1
../../src/gdb/breakpoint.c:6924
0x5b58b8 maintenance_info_breakpoints
../../src/gdb/breakpoint.c:7009
... etc ...
As the thread-specific breakpoint is set to disp_del_at_next_stop, and
GDB hasn't stopped yet, then the breakpoint still exists in the global
breakpoint list.
The breakpoint will not show in 'info breakpoints' as its number is
zero, but it will show in 'maint info breakpoints'.
As GDB prints the breakpoint, the thread-id for the breakpoint is
printed as part of the 'stop only in thread ...' line. Printing the
thread-id involves calling find_thread_global_id to convert the global
thread-id into a thread_info*. Then calling print_thread_id to
convert the thread_info* into a string.
The problem is that find_thread_global_id returns nullptr as the
thread for the thread-specific breakpoint has exited. The
print_thread_id assumes it will be passed a non-nullptr. As a result
GDB crashes.
In this commit I've added an assert to print_thread_id (gdb/thread.c)
to check that the pointed passed in is not nullptr. This assert would
have triggered in the above case before GDB crashed.
MI Notifications: The Dangers Of Changing A Breakpoint's Number
---------------------------------------------------------------
Currently the delete_breakpoint function doesn't trigger the
breakpoint_deleted observer for any breakpoint with the number zero.
There is a comment explaining why this is the case in the code; it's
something about watchpoints. But I did consider just removing the 'is
the number zero' guard and always triggering the breakpoint_deleted
observer, figuring that I'd then fix the watchpoint issue some other
way.
But I realised this wasn't going to be good enough. When the MI
notification was delivered the number would be zero, so any frontend
parsing the notifications would not be able to match
=breakpoint-deleted notification to the earlier =breakpoint-created
notification.
What this means is that, at the point the breakpoint_deleted observer
is called, the breakpoint's number must be correct.
MI Notifications: The Dangers Of Delaying Deletion
--------------------------------------------------
The test I used to expose the above crash also brought another problem
to my attention. In the above test we used 'continue&' to resume,
after which a thread exited, but the inferior didn't stop. Recreating
the same test in the MI looks like this:
-break-insert -p 2 main
^done,bkpt={number="2",type="breakpoint",disp="keep",...<snip>...}
(gdb)
-exec-continue
^running
*running,thread-id="all"
(gdb)
~"[Thread 0x7ffff7c5d700 (LWP 987038) exited]\n"
=thread-exited,id="2",group-id="i1"
~"Thread-specific breakpoint 2 deleted - thread 2 no longer in the thread list.\n"
At this point the we have a single thread left, which is still
running:
-thread-info
^done,threads=[{id="1",target-id="Thread 0x7ffff7c5eb80 (LWP 987035)",name="thread-bp-delet",state="running",core="4"}],current-thread-id="1"
(gdb)
Notice that we got the =thread-exited notification from GDB as soon as
the thread exited. We also saw the CLI line from GDB, the line
explaining that breakpoint 2 was deleted. But, as expected, we didn't
see the =breakpoint-deleted notification.
I say "as expected" because the number was set to zero. But, even if
the number was not set to zero we still wouldn't see the
notification. The MI notification is driven by the breakpoint_deleted
observer, which is only called when we actually delete the breakpoint,
which is only done the next time GDB stops.
Now, maybe this is fine. The notification is delivered a little
late. But remember, by setting the number to zero the breakpoint will
be hidden from the user, for example, the breakpoint is removed from
the MI's -break-info command output.
This means that GDB is in a position where the breakpoint doesn't show
up in the breakpoint table, but a =breakpoint-deleted notification has
not yet been sent out. This doesn't seem right to me.
What this means is that, when the thread exits, we should immediately
be sending out the =breakpoint-deleted notification. We should not
wait for GDB to next stop before sending the notification.
The Solution
------------
My proposed solution is this; in remove_threaded_breakpoints, instead
of setting the disposition to disp_del_at_next_stop and setting the
number to zero, we now just call delete_breakpoint directly.
The notification will now be sent out immediately; as soon as the
thread exits.
As the number has not changed when delete_breakpoint is called, the
notification will have the correct number.
And as the breakpoint is immediately removed from the breakpoint list,
we no longer need to worry about 'maint info breakpoints' trying to
print the thread-id for an exited thread.
My only concern is that calling delete_breakpoint directly seems so
obvious that I wonder why the original patch (that added
remove_threaded_breakpoints) didn't take this approach. This code was
added in commit 49fa26b0411d, but the commit message offers no clues
to why this approach was taken, and the original email thread offers
no insights either[2]. There are no test regressions after making
this change, so I'm hopeful that this is going to be fine.
[2] https://sourceware.org/pipermail/gdb-patches/2013-September/106493.html
The Complication
----------------
Of course, it couldn't be that simple.
The script gdb.python/py-finish-breakpoint.exp had some regressions
during testing.
The problem was with the FinishBreakpoint.out_of_scope callback
implementation. This callback is supposed to trigger whenever the
FinishBreakpoint goes out of scope; and this includes when the thread
for the breakpoint exits.
The problem I ran into is the Python FinishBreakpoint implementation.
Specifically, after this change I was loosing some of the out_of_scope
calls.
The problem is that the out_of_scope call (of which I'm interested) is
triggered from the inferior_exit observer. Before my change the
observers were called in this order:
thread_exit
inferior_exit
breakpoint_deleted
The inferior_exit would trigger the out_of_scope call.
After my change the breakpoint_deleted notification (for
thread-specific breakpoints) occurs earlier, as soon as the
thread-exits, so now the order is:
thread_exit
breakpoint_deleted
inferior_exit
Currently, after the breakpoint_deleted call the Python object
associated with the breakpoint is released, so, when we get to the
inferior_exit observer, there's no longer a Python object to call the
out_of_scope method on.
My solution is to follow the model for how bpfinishpy_pre_stop_hook
and bpfinishpy_post_stop_hook are called, this is done from
gdbpy_breakpoint_cond_says_stop in py-breakpoint.c.
I've now added a new bpfinishpy_pre_delete_hook
gdbpy_breakpoint_deleted in py-breakpoint.c, and from this new hook
function I check and where needed call the out_of_scope method.
With this fix in place I now see the
gdb.python/py-finish-breakpoint.exp test fully passing again.
Testing
-------
Tested on x86-64/Linux with unix, native-gdbserver, and
native-extended-gdbserver boards.
New tests added to covers all the cases I've discussed above.
Approved-By: Pedro Alves <pedro@palves.net>
When creating a thread-specific breakpoint with a single location, the
'thread' field would be repeated in the MI output. This can be seen
in two existing tests gdb.mi/mi-nsmoribund.exp and
gdb.mi/mi-pending.exp, e.g.:
(gdb)
-break-insert -p 1 bar
^done,bkpt={number="1",type="breakpoint",disp="keep",
enabled="y",
addr="0x000000000040110a",func="bar",
file="/tmp/mi-thread-specific-bp.c",
fullname="/tmp/mi-thread-specific-bp.c",
line="32",thread-groups=["i1"],
thread="1",thread="1", <================ DUPLICATION!
times="0",original-location="bar"}
I know we need to be careful when adjusting MI output, but I'm hopeful
in this case, as the field is duplicated, and the field contents are
always identical, that we might get away with removing one of the
duplicates.
The change in GDB is a fairly trivial condition change.
We did have a couple of tests that contained the duplicate fields in
their expected output, but given there was no comment pointing out
this oddity either in the GDB code, or in the test, I suspect this was
more a case of copying whatever output GDB produced and using that as
the expected results. I've updated these tests to remove the
duplication.
I've update lib/mi-support.exp to provide support for building
breakpoint patterns that contain the thread field, and I've made use
of this in a new test I've added that is just about creating
thread-specific breakpoints and checking the results. The two tests I
mentioned above as being updated could also use the new
lib/mi-support.exp functionality, but I'm going to do that in a later
patch, this way it is clear what changes I'm actually proposing to
make to the expected output.
As I said, I hope that frontends will be able to handle this change,
but I still think its worth adding a NEWS entry, that way, if someone
runs into problems, there's a chance they can figure out what's going
on.
This should not impact CLI output at all.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Pedro Alves <pedro@palves.net>
Delete an out of date comment about disp_del_at_next_stop, the comment
says:
/* NOTE drow/2003-09-08: This state only exists for removing
watchpoints. It's not clear that it's necessary... */
I'm sure this was true when the comment was added, but today the
disp_del_at_next_stop state is not just used for deleting watchpoints,
which leaves us with "It's not clear that it's necessary...", which
doesn't really help at all.
And then this comment is located on one random place where
disp_del_at_next_stop is used, rather than at its definition site.
Lets just delete the comment.
No user visible changes after this commit.
Reviewed-By: Pedro Alves <pedro@palves.net>
As described in the previous commit for this series, I became
concerned that there might be instances in which a QUIT (due to either
a SIGINT or SIGTERM) might not cause execution to return to the top
level. In some (though very few) instances, it is okay to not
propagate the exception for a Ctrl-C / SIGINT, but I don't think that
it is ever okay to swallow the exception caused by a SIGTERM.
Allowing that to happen would definitely be a deviation from the
current behavior in which GDB exits upon receipt of a SIGTERM.
I looked at all cases where an exception handler catches a
gdb_exception. Handlers which did NOT need modification were those
which satisifed one or more of the following conditions:
1) There is no call path to maybe_quit() in the try block. I used a
static analysis tool to help make this determination. In
instances where the tool didn't provide an answer of "yes, this
call path can result in maybe_quit() being called", I reviewed it
by hand.
2) The catch block contains a throw for conditions that it
doesn't want to handle; these "not handled" conditions
must include the quit exception and the new "forced quit" exception.
3) There was (also) a catch for gdb_exception_quit.
Any try/catch blocks not meeting the above conditions could
potentially swallow a QUIT exception.
My first thought was to add catch blocks for gdb_exception_quit and
then rethrow the exception. But Pedro pointed out that this can be
handled without adding additional code by simply catching
gdb_exception_error instead. That's what this patch series does.
There are some oddball cases which needed to be handled differently,
plus the extension languages, but those are handled in later patches.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
Tested-by: Tom de Vries <tdevries@suse.de>
Approved-by: Pedro Alves <pedro@palves.net>
Simon pointed out some spots were doing val.get()->mumble, where val
is a value_ref_ptr. These were introduced by the function-to-method
script, replacing older code that passed the result of .get() to a
function.
Now that value.h is using methods, we can instead rely on operator->.
This patch replaces all the newly-introduced instances of this.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This removes deprecated_lval_hack and the VALUE_LVAL macro, replacing
all uses with a call to value::lval.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This patch turns a grab bag of value functions to methods of value.
These are done together because their implementations are
interrelated.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This turns the remaining value_contents functions -- value_contents,
value_contents_all, value_contents_for_printing, and
value_contents_for_printing_const -- into methods of value. It also
converts the static functions require_not_optimized_out and
require_available to be private methods.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This changes the value_lazy and set_value_lazy functions to be methods
of value. Much of this patch was written by script.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Commit:
commit 2ecee236752932672fb3d6cd63c6976927f747d8
CommitDate: Sun Feb 12 05:46:44 2023 +0000
gdb: use -1 for breakpoint::task default value
Failed to take account of an earlier commit:
commit f1f517e81039f6aa673b7d87a66bfbd25a66e3d3
CommitDate: Sat Feb 11 17:36:24 2023 +0000
gdb: show task number in describe_other_breakpoints
That both of these are my own commits is only more embarrassing.
This small fix updates describe_other_breakpoints to take account of
the default task number now being -1. This fixes regressions in
gdb.base/break.exp, gdb.base/break-always.exp, and many other tests.
Within the breakpoint struct we have two fields ::thread and ::task
which are used for thread or task specific breakpoints. When a
breakpoint doesn't have a specific thread or task then these fields
have the values -1 and 0 respectively.
There's no particular reason (as far as I can tell) why these two
"default" values are different, and I find the difference a little
confusing. Long term I'd like to potentially fold these two fields
into a single field, but that isn't what this commit does.
What this commit does is switch to using -1 as the "default" value for
both fields, this means that the default for breakpoint::task has
changed from 0 to -1. I've updated all the code I can find that
relied on the value of 0, and I see no test regressions, especially in
gdb.ada/tasks.exp, which still fully passes.
There should be no user visible changes after this commit.
Approved-By: Pedro Alves <pedro@palves.net>
After this mailing list posting:
https://sourceware.org/pipermail/gdb-patches/2023-February/196607.html
it seems to me that in practice an Ada task maps 1:1 with a GDB
thread, and so it doesn't really make sense to allow uses to give both
a thread and a task within a single breakpoint or watchpoint
condition.
This commit updates GDB so that the user will get an error if both
are specified.
I've added new tests to cover the CLI as well as the Python and Guile
APIs. For the Python and Guile testing, as far as I can tell, this
was the first testing for this corner of the APIs, so I ended up
adding more than just a single test.
For documentation I've added a NEWS entry, but I've not added anything
to the docs themselves. Currently we document the commands with a
thread-id or task-id as distinct command, e.g.:
'break LOCSPEC task TASKNO'
'break LOCSPEC task TASKNO if ...'
'break LOCSPEC thread THREAD-ID'
'break LOCSPEC thread THREAD-ID if ...'
As such, I don't believe there is any indication that combining 'task'
and 'thread' would be expected to work; it seems clear to me in the
above that those four options are all distinct commands.
I think the NEWS entry is enough that if someone is combining these
keywords (it's not clear what the expected behaviour would be in this
case) then they can figure out that this was a deliberate change in
GDB, but for a new user, the manual doesn't suggest combining them is
OK, and any future attempt to combine them will give an error.
Approved-By: Pedro Alves <pedro@palves.net>
I noticed that describe_other_breakpoints doesn't show the task
number, but does show the thread-id. I can't see any reason why we'd
want to not show the task number in this situation, so this commit
adds this missing information, and extends gdb.ada/tasks.exp to check
this case.
Approved-By: Pedro Alves <pedro@palves.net>
I noticed that describe_other_breakpoints was printing the global
thread-id to the CLI. For CLI output we should be printing the
inferior local thread-id (e.g. "2.1"). This can be seen in the
following GDB session:
(gdb) info threads
Id Target Id Frame
1.1 Thread 4065742.4065742 "bp-thread-speci" main () at /tmp/bp-thread-specific.c:27
* 2.1 Thread 4065743.4065743 "bp-thread-speci" main () at /tmp/bp-thread-specific.c:27
(gdb) break foo thread 2.1
Breakpoint 3 at 0x40110a: foo. (2 locations)
(gdb) break foo thread 1.1
Note: breakpoint 3 (thread 2) also set at pc 0x40110a.
Note: breakpoint 3 (thread 2) also set at pc 0x40110a.
Breakpoint 4 at 0x40110a: foo. (2 locations)
Notice that GDB says:
Note: breakpoint 3 (thread 2) also set at pc 0x40110a.
The 'thread 2' in here is using the global thread-id, we should
instead say 'thread 2.1' which corresponds to how the user specified
the breakpoint.
This commit fixes this issue and adds a test.
Approved-By: Pedro Alves <pedro@palves.net>
Nearly every call to fixup_symbol_section in gdb is incorrect, and if
any such call has an effect, it's purely by happenstance.
fixup_section has a long comment explaining that the call should only
be made before runtime section offsets are applied. And, the loop in
this code (the fallback loop -- the minsym lookup code is "ok") is
careful to remove these offsets before comparing addresses.
However, aside from a single call in dwarf2/read.c, every call in gdb
is actually done after section offsets have been applied. So, these
calls are incorrect.
Now, these calls could be made when the symbol is created. I
considered this approach, but I reasoned that the code has been this
way for many years, seemingly without ill effect. So, instead I chose
to simply remove the offending calls.
This commit addresses the issue in PR gdb/30087.
If a breakpoint with multiple locations has a thread condition, then
the 'info breakpoints' output is a little messed up, here's an example
of the current output:
(gdb) break foo thread 1
Breakpoint 2 at 0x401114: foo. (3 locations)
(gdb) break bar thread 1
Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y <MULTIPLE> thread 1
stop only in thread 1
2.1 y 0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
2.2 y 0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
2.3 y 0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
3 breakpoint keep y 0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32 thread 1
stop only in thread 1
Notice that, at the end of the location for breakpoint 3, the 'thread
1' condition is printed, but this is then repeated on the next line
with 'stop only in thread 1'.
In contrast, for breakpoint 2, the 'thread 1' appears randomly, in the
"What" column, though slightly offset, non of the separate locations
have the 'thread 1' information. Additionally for breakpoint 2 we
also get a 'stop only in thread 1' line.
There's two things going on here. First the randomly placed 'thread
1' for breakpoint 2 is due to a bug in print_one_breakpoint_location,
where we check the variable part_of_multiple instead of
header_of_multiple.
If I fix this oversight, then the output is now:
(gdb) break foo thread 1
Breakpoint 2 at 0x401114: foo. (3 locations)
(gdb) break bar thread 1
Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y <MULTIPLE>
stop only in thread 1
2.1 y 0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
2.2 y 0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
2.3 y 0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
3 breakpoint keep y 0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32 thread 1
stop only in thread 1
The 'thread 1' condition is now displayed at the end of each location,
which makes the output the same for single location breakpoints and
multi-location breakpoints.
However, there's still some duplication here. Both breakpoints 2 and
3 include a 'stop only in thread 1' line, and it feels like the
additional 'thread 1' is redundant. In fact, there's a comment to
this very effect in the code:
/* FIXME: This seems to be redundant and lost here; see the
"stop only in" line a little further down. */
So, lets fix this FIXME. The new plan is to remove all the trailing
'thread 1' markers from the CLI output, we now get this:
(gdb) break foo thread 1
Breakpoint 2 at 0x401114: foo. (3 locations)
(gdb) break bar thread 1
Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
(gdb) info breakpoints
Num Type Disp Enb Address What
2 breakpoint keep y <MULTIPLE>
stop only in thread 1
2.1 y 0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
2.2 y 0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
2.3 y 0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
3 breakpoint keep y 0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32
stop only in thread 1
All of the above points are also true for the Ada 'task' breakpoint
condition, and the changes I've made also update how the task
information is printed, though in the case of the Ada task there was
no 'stop only in task XXX' line printed, so I've added one of those.
Obviously it can't be quite that easy. For MI backwards compatibility
I've retained the existing code (but now only for MI like outputs),
which ensures we should generate backwards compatible output.
I've extended an Ada test to cover the new task related output, and
updated all the tests I could find that checked for the old output.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30087
Approved-By: Pedro Alves <pedro@palves.net>
When creating a breakpoint or watchpoint, the 'thread' and 'task'
keywords can be used to create a thread or task specific breakpoint or
watchpoint.
Currently, a thread or task specific breakpoint can only apply for a
single thread or task, if multiple threads or tasks are specified when
creating the breakpoint (or watchpoint), then the last specified id
will be used.
The exception to the above is that when the 'thread' keyword is used
during the creation of a watchpoint, GDB will give an error if
'thread' is given more than once.
In this commit I propose making this behaviour consistent, if the
'thread' or 'task' keywords are used more than once when creating
either a breakpoint or watchpoint, then GDB will give an error.
I haven't updated the manual, we don't explicitly say that these
keywords can be repeated, and (to me), given the keyword takes a
single id, I don't think it makes much sense to repeat the keyword.
As such, I see this more as adding a missing error to GDB, rather than
making some big change. However, I have added an entry to the NEWS
file as I guess it is possible that some people might hit this new
error with an existing (I claim, badly written) GDB script.
I've added some new tests to check for the new error.
Just one test needed updating, gdb.linespec/keywords.exp, this test
did use the 'thread' keyword twice, and expected the breakpoint to be
created. Looking at what this test was for though, it was checking
the use of '-force-condition', and I don't think that being able to
repeat 'thread' was actually a critical part of this test.
As such, I've updated this test to expect the error when 'thread' is
repeated.
A following patch will want to install a breakpoint and then keep a
non-owning reference to it. Make install_breakpoint return a non-owning
reference, to make that easy.
Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I2e8106a784021ff276ce251e24708cbdccc2d479
Approved-By: Andrew Burgess <aburgess@redhat.com>
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
PR gdb/28947
The address_significant gdbarch setting was introduced as a way to remove
non-address bits from pointers, and it is specified by a constant. This
constant represents the number of address bits in a pointer.
Right now AArch64 is the only architecture that uses it, and 56 was a
correct option so far.
But if we are using Pointer Authentication (PAuth), we might use up to 2 bytes
from the address space to store the required information. We could also have
cases where we're using both PAuth and MTE.
We could adjust the constant to 48 to cover those cases, but this doesn't
cover the case where GDB needs to sign-extend kernel addresses after removal
of the non-address bits.
This has worked so far because bit 55 is used to select between kernel-space
and user-space addresses. But trying to clear a range of bits crossing the
bit 55 boundary requires the hook to be smarter.
The following patch renames the gdbarch hook from significant_addr_bit to
remove_non_address_bits and passes a pointer as opposed to the number of
bits. The hook is now responsible for removing the required non-address bits
and sign-extending the address if needed.
While at it, make GDB and GDBServer share some more code for aarch64 and add a
new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.
Bug-url: https://sourceware.org/bugzilla/show_bug.cgi?id=28947
Approved-By: Simon Marchi <simon.marchi@efficios.com>
[I sent this earlier today, but I don't see it in the archives.
Resending it through a different computer / SMTP.]
The use of the static buffer in command_line_input is becoming
problematic, as explained here [1]. In short, with this patch [2] that
attempt to fix a post-hook bug, when running gdb.base/commands.exp, we
hit a case where we read a "define" command line from a script file
using command_command_line_input. The command line is stored in
command_line_input's static buffer. Inside the define command's
execution, we read the lines inside the define using command_line_input,
which overwrites the define command, in command_line_input's static
buffer. After the execution of the define command, execute_command does
a command look up to see if a post-hook is registered. For that, it
uses a now stale pointer that used to point to the define command, in
the static buffer, causing a use-after-free. Note that the pointer in
execute_command points to the dynamically-allocated buffer help by the
static buffer in command_line_input, not to the static object itself,
hence why we see a use-after-free.
Fix that by removing the static buffer. I initially changed
command_line_input and other related functions to return an std::string,
which is the obvious but naive solution. The thing is that some callees
don't need to return an allocated string, so this this an unnecessary
pessimization. I changed it to passing in a reference to an std::string
buffer, which the callee can use if it needs to return
dynamically-allocated content. It fills the buffer and returns a
pointers to the C string inside. The callees that don't need to return
dynamically-allocated content simply don't use it.
So, it started with modifying command_line_input as described above, all
the other changes derive directly from that.
One slightly shady thing is in handle_line_of_input, where we now pass a
pointer to an std::string's internal buffer to readline's history_value
function, which takes a `char *`. I'm pretty sure that this function
does not modify the input string, because I was able to change it (with
enough massaging) to take a `const char *`.
A subtle change is that we now clear a UI's line buffer using a
SCOPE_EXIT in command_line_handler, after executing the command.
This was previously done by this line in handle_line_of_input:
/* We have a complete command line now. Prepare for the next
command, but leave ownership of memory to the buffer . */
cmd_line_buffer->used_size = 0;
I think the new way is clearer.
[1] https://inbox.sourceware.org/gdb-patches/becb8438-81ef-8ad8-cc42-fcbfaea8cddd@simark.ca/
[2] https://inbox.sourceware.org/gdb-patches/20221213112241.621889-1-jan.vrany@labware.com/
Change-Id: I8fc89b1c69870c7fc7ad9c1705724bd493596300
Reviewed-By: Tom Tromey <tom@tromey.com>
On powerpc64le-linux I ran into this FAIL:
...
(gdb) p exceptions.throw_function()^M
terminate called after throwing an instance of 'int'^M
^M
Program received signal SIGABRT, Aborted.^M
0x00007ffff7979838 in raise () from /lib64/libc.so.6^M
The program being debugged was signaled while in a function called from GDB.^M
GDB remains in the frame where the signal was received.^M
To change this behavior use "set unwindonsignal on".^M
Evaluation of the expression containing the function^M
(SimpleException::throw_function()) will be abandoned.^M
When the function is done executing, GDB will silently stop.^M
(gdb) FAIL: gdb.cp/gdb2495.exp: call a function that raises an exception \
without a handler.
...
The following happens:
- we start an inferior call
- an internal breakpoint is set on the global entry point of std::terminate
- the inferior call uses the local entry point
- the breakpoint is not triggered
- we run into std::terminate
We can fix this by simply adding the missing gdbarch_skip_entrypoint call in
create_std_terminate_master_breakpoint, but we try to do this a bit more
generic, by:
- adding a variant of function create_internal_breakpoint which takes a
minimal symbol instead of an address as argument
- in the new function:
- using both gdbarch_convert_from_func_ptr_addr and gdbarch_skip_entrypoint
- documenting why we don't need to use gdbarch_addr_bits_remove
- adding a note about possibly
needing gdbarch_deprecated_function_start_offset.
- using the new function in:
- create_std_terminate_master_breakpoint
- create_exception_master_breakpoint_hook, which currently uses only
gdbarch_convert_from_func_ptr_addr.
Note: we could use the new function in more locations in breakpoint.c, but
as we're not aware of any related failures, we declare this out of scope for
this patch.
Tested on x86_64-linux, powerpc64le-linux.
Co-Authored-By: Ulrich Weigand <uweigand@de.ibm.com>
Tested-by: Carl Love <cel@us.ibm.com>
PR tdep/29793
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29793
The test py-objfile.exp unloads the current file while debugging the process.
This results in bpstat bs->b->loc to become nullptr.
Handle this case in breakpoint.c:bpstat_locno.
Note: GDB crashes on this problem with an internal error,
but the end of gdb summary shows:
...
=== gdb Summary ===
# of expected passes 36
The output also does not contain a 'FAIL:'.
After the fix, the nr of expected passes increased.
In the gdb.log output, one can see:
...
Fatal signal: Segmentation fault
----- Backtrace -----
0x55698905c5b9 gdb_internal_backtrace_1
../../binutils-gdb/gdb/bt-utils.c:122
0x55698905c5b9 _Z22gdb_internal_backtracev
...
ERROR: Couldn't send python print(objfile.filename) to GDB.
ERROR: : spawn id exp9 not open
while executing
"expect {
-i exp9 -timeout 10
-re ".*A problem internal to GDB has been detected" {
fail "$message (GDB internal error)"
gdb_internal_error..."
("uplevel" body line 1)
invoked from within
....
Wondering if it might be possible to improve gdb_test to have
gdb_test "python print(objfile.filename)" "None" \
"objfile.filename after objfile is unloaded"
reporting a failed result instead of just producing the internal error.
If the commands of the bpstat bs contain commands such as step or next or
continue, the BS and its commands are freed by execute_control_command.
So, we cannot remember the BS that was printed. Instead, remember
the bpnum and locno.
Regtested on debian/amd64 and re-run a few tests under valgrind.
This implements the request given in PR breakpoints/12464.
Before this patch, when a breakpoint that has multiple locations is reached,
GDB printed:
Thread 1 "zeoes" hit Breakpoint 1, some_func () at somefunc1.c:5
This patch changes the message so that bkpt_print_id prints the precise
encountered breakpoint:
Thread 1 "zeoes" hit Breakpoint 1.2, some_func () at somefunc1.c:5
In mi mode, bkpt_print_id also (optionally) prints a new table field "locno":
locno is printed when the breakpoint hit has more than one location.
Note that according to the GDB user manual node 'GDB/MI Development and Front
Ends', it is ok to add new fields without changing the MI version.
Also, when a breakpoint is reached, the convenience variables
$_hit_bpnum and $_hit_locno are set to the encountered breakpoint number
and location number.
$_hit_bpnum and $_hit_locno can a.o. be used in the command list of a
breakpoint, to disable the specific encountered breakpoint, e.g.
disable $_hit_bpnum.$_hit_locno
In case the breakpoint has only one location, $_hit_locno is set to
the value 1, so as to allow a command such as:
disable $_hit_bpnum.$_hit_locno
to disable the breakpoint even when the breakpoint has only one location.
This also fixes a strange behaviour: when a breakpoint X has only
one location,
enable|disable X.1
is accepted but transforms the breakpoint in a multiple locations
breakpoint having only one location.
The changes in RFA v4 handle the comments of Tom Tromey:
- Changed convenience var names from $bkptno/$locno to
$_hit_bpnum/$_hit_locno.
- updated the tests and user manual accordingly.
User manual also explictly describes that $_hit_locno is set to 1
for a breakpoint with a single location.
- The variable values are now set in bpstat_do_actions_1 so that
they are set for silent breakpoints, and when several breakpoints
are hit at the same time, that the variables are set to the printed
breakpoint.
The changes in RFA v3 handle the additional comments of Eli:
GDB/NEW:
- Use max 80-column
- Use 'code location' instead of 'location'.
- Fix typo $bkpno
- Ensure that disable $bkptno and disable $bkptno.$locno have
each their explanation inthe example
- Reworded the 'breakpoint-hit' paragraph.
gdb.texinfo:
- Use 'code location' instead of 'location'.
- Add a note to clarify the distinction between $bkptno and $bpnum.
- Use @kbd instead of examples with only one command.
Compared to RFA v1, the changes in v2 handle the comments given by
Keith Seitz and Eli Zaretskii:
- Use %s for the result of paddress
- Use bkptno_numopt_re instead of 2 different -re cases
- use C@t{++}
- Add index entries for $bkptno and $locno
- Added an example for "locno" in the mi interface
- Added examples in the Break command manual.
Currently the resource checking is done if REPARSE is true. The hardware
watchpoint resource checking in update_watchpoint needs to be redone on
each call to function update_watchpoints as the value chain may have
changed. The number of hardware registers needed for a watchpoint can
change if the variable being watched changes. This situation occurs in
this test when watching variable **global_ptr_ptr. Initially when the
watch command is issued, only two addresses need to be watched as
**global_ptr_ptr has not yet been initialized. Once the value of
**global_ptr_ptr is initialized the locations to be tracked increase to
three addresses. However, update_watchpoints is not called again with
REPARSE set to 1 to force the resource checking to be redone. When the
test is run on Power 10, an internal gdb error occurs when the PowerPC
routine tries to setup the three hardware watchpoint address since the hw
only has two hardware watchpoint registers. The error occurs because the
resource checking was not redone in update_watchpoints after
**global_ptr_ptr changed.
The following descibes the situation in detail that occurs on Power 10 with
gdb running on the binary for gdb.base/watchpoint.c.
1 break func4
2 run
3 watch *global_ptr
4 next execute source code: buf[0] = 3;
5 next execute source code: global_ptr = buf;
6 next execute source code: buf[0] = 7;
7 delete 2 (delete watch *global_ptr)
8 watch **global_ptr_ptr
9 next execute source code: buf[1] = 5;
10 next global_ptr_ptr = &global_ptr;
11 next buf[0] = 9;
In step 8, the the watch **global_ptr_prt command calls update_watchpoint
in breakpoint.c with REPARSE set to 1. The function update_watchpoint
calls can_use_hardware_watchpoint to see if there are enough
resources available to add the watchpoint since REPARSE is set to 1. At
this point, **global_ptr_ptr has not been initialized so only two addresses
are watched. The val_chain contains the address for **global_ptr_ptr and 0
since **global_ptr_ptr has not been initialized. The update_watchpoint
updates the breakpoint list as follows:
breakpoint 0
loc 0: b->address = 0x100009c0
breakpoint 1
loc 1: b->address = 0x7ffff7f838a0
breakpoint 2
loc 2: b->address = 0x7ffff7b7fc54
breakpoint 3
loc 3: b->address = 0x7ffff7a5788c
breakpoint 4
loc 4: b->address = 0x0 <-- location pointed to by global_ptr_ptr
loc 5: b->address = 0x100200b8 <-- global_ptr_ptr watchpoint
breakpoint 5
loc 6: b->address = 0x7ffff7b7fc54
In step 10, the next command executes the source code
global_ptr_ptr = &global_ptr. This changes the set of locations to be
watched for the watchpoint **global_ptr_prt. The list of addresses for the
breakpoint consist of the address for global_ptr_prt, global_ptr and buf.
The breakpoint list gets updated by update_watchpoint as follows:
breakpoint 0
loc 0: b->address = 0x100009c0
breakpoint 1
loc 1: b->address = 0x7ffff7f838a0
breakpoint 2
loc 2: b->address = 0x7ffff7b7fc54
breakpoint 3
loc 3: b->address = 0x7ffff7a5788c
breakpoint 4
loc 4: b->address = 0x10020050 buf
loc 5: b->address = 0x100200b0 watch *global_ptr
loc 6: b->address = 0x100200b8 watch **global_ptr_ptr
breakpoint 5
loc 7: b->address = 0x7ffff7b7fc54
breakpoint 6
However, the hardware resource checking was not redone because
update_breakpoint was called with REPARSE equal to 0.
Step 11, execute the third next command. The function
ppc_linux_nat_target::low_prepare_to_resume() attempts a ptrace
call to setup each of the three address for breakpoint 4. The slot
value returned for the third ptrace call is -1 indicating an error
because there are only two hardware watchpoint registers available on
Power 10.
This patch removes just the statement "if (reparse)" in function
update_watchpoint to force the resources to be rechecked on every call to
the function. This ensures that any changes to the val_chain resulting
in needing more resources then available will be caught.
The patch has been tested on Power 8, Power 10 and X86-64. Note the patch
has no effect on Power 9 since hardware watchpoint support is disabled on
that processor.
Recent commit b2829fcf9b5 ("[gdb] Fix rethrow exception slicing in
insert_bp_location") introduced macro RETHROW_ON_TARGET_CLOSE_ERROR.
I wrote this as a macro in order to have the rethrowing throw be part of the
same function as the catch, but as it turns out that's not necessary.
Rewrite into a function.
Tested on x86_64-linux.
The preferred way of rethrowing an exception is by using throw without
expression, because it avoids object slicing of the exception [1].
Fix this in insert_bp_location.
Tested on x86_64-linux.
[1] https://en.cppreference.com/w/cpp/language/throw
Approved-By: Andrew Burgess <aburgess@redhat.com>
Some int to bool conversion in breakpoint.c. I've only updated the
function signatures of static functions, but I've updated some
function local variables throughout the file.
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
I noticed that we could make use of a scoped_restore in the function
unduplicated_should_be_inserted. I've also converted the function
return type from int to bool.
This change shouldn't make any difference, as I don't think anything
within should_be_inserted could throw an exception, but the change
doesn't hurt, and will help keep us safe if anything ever changes in
the future.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
I was doing some int to bool cleanup in update_watchpoint, and I
noticed a manual version of scoped_restore_selected_frame. As always
when these things are done manually, there is the chance that, in an
error case, we might leave the wrong frame selected.
This commit updates things to use scoped_restore_selected_frame, and
also converts a local variable from int to bool.
The only user visible change after this commit is in the case where
update_watchpoint throws an error - we should now correctly restore
the previously selected frame. Otherwise, this commit should be
invisible to the user.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
I spotted a few places where I could make some 'bp_location *'
arguments constant in breakpoint.c.
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
