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 2ecee23675
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 f1f517e810
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 b2829fcf9b ("[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>
With the cc-with-tweaks.sh patch submitted here (
https://sourceware.org/pipermail/gdb-patches/2022-October/192586.html ) we run
with:
...
$ export STRIP_ARGS_STRIP_DEBUG=--strip-all
$ make check RUNTESTFLAGS="gdb.base/jit-reader.exp \
--target_board cc-with-gnu-debuglink"
...
into the following assert:
...
(gdb) run ^M
Starting program: jit-reader ^M
gdb/jit.c:1247: internal-error: jit_event_handler: \
Assertion `jiter->jiter_data != nullptr' failed.^M
...
Fix this by handling the
jit_bp_sym.objfile->separate_debug_objfile_backlink != nullptr case in
handle_jit_event.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29277
Currently, every internal_error call must be passed __FILE__/__LINE__
explicitly, like:
internal_error (__FILE__, __LINE__, "foo %d", var);
The need to pass in explicit __FILE__/__LINE__ is there probably
because the function predates widespread and portable variadic macros
availability. We can use variadic macros nowadays, and in fact, we
already use them in several places, including the related
gdb_assert_not_reached.
So this patch renames the internal_error function to something else,
and then reimplements internal_error as a variadic macro that expands
__FILE__/__LINE__ itself.
The result is that we now should call internal_error like so:
internal_error ("foo %d", var);
Likewise for internal_warning.
The patch adjusts all calls sites. 99% of the adjustments were done
with a perl/sed script.
The non-mechanical changes are in gdbsupport/errors.h,
gdbsupport/gdb_assert.h, and gdb/gdbarch.py.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
I looked at all the spots using value_mark, and converted all the
straightforward ones to use scoped_value_mark instead.
Regression tested on x86-64 Fedora 34.
This changes GDB to use frame_info_ptr instead of frame_info *
The substitution was done with multiple sequential `sed` commands:
sed 's/^struct frame_info;/class frame_info_ptr;/'
sed 's/struct frame_info \*/frame_info_ptr /g' - which left some
issues in a few files, that were manually fixed.
sed 's/\<frame_info \*/frame_info_ptr /g'
sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace
problems.
The changed files were then manually checked and some 'sed' changes
undone, some constructors and some gets were added, according to what
made sense, and what Tromey originally did
Co-Authored-By: Bruno Larsen <blarsen@redhat.com>
Approved-by: Tom Tomey <tom@tromey.com>
This replaces frame_id_eq with operator== and operator!=. I wrote
this for a version of this series that I later abandoned; but since it
simplifies the code, I left this patch in.
Approved-by: Tom Tomey <tom@tromey.com>
This commit adds additional infrun debug from the breakpoint.c file.
The new debug output all relates to breakpoint condition evaluation.
There is already some infrun debug emitted from the breakpoint.c file,
so hopefully, adding more will not be contentious. I think the
functions being instrumented make sense as part of the infrun process,
the inferior stops, evaluates the condition, and then either stops or
continues. This new debug gives more insight into that process.
I had to make the bp_location* argument to find_loc_num_by_location
const, and add a declaration for find_loc_num_by_location.
There should be no user visible changes unless they turn on debug
output.
The print_one_detail_ranged_breakpoint has been renamed to
ranged_breakpoint::print_one_detail in this commit:
commit ec45bb676c
Date: Sat Jan 15 16:34:51 2022 -0700
Convert ranged breakpoints to vtable ops
So their comments should be updated as well.
Change from int to target_waitkind, which is really what is is. While
at it, remove some outdated doc. The return value is described by a
relatively self-describing enum, not a numerical value like the doc
says.
Change-Id: Id899c853a857c7891c45e5b1639024067d5b59cd
breakpoint::decode_location_spec just asserts if called. It turned
out to be relatively easy to remove this method from breakpoint and
instead move the base implementation to code_breakpoint.
The "script" field, output whenever information about a breakpoint with
commands is output, uses wrong MI syntax.
$ ./gdb -nx -q --data-directory=data-directory -x script -i mi
=thread-group-added,id="i1"
=breakpoint-created,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",original-location="main"}
=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",script={"aaa","bbb","ccc"},original-location="main"}
(gdb)
-break-info
^done,BreakpointTable={nr_rows="1",nr_cols="6",hdr=[{width="7",alignment="-1",col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},{width="18",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_name="what",colhdr="What"}],body=[bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x000000000000111d",func="main",file="test.c",fullname="/home/simark/build/binutils-gdb-one-target/gdb/test.c",line="3",thread-groups=["i1"],times="0",script={"aaa","bbb","ccc"},original-location="main"}]}
(gdb)
In both the =breakpoint-modified and -break-info output, we have:
script={"aaa","bbb","ccc"}
According to the output syntax [1], curly braces means tuple, and a
tuple contains key=value pairs. This looks like it should be a list,
but uses curly braces by mistake. This would make more sense:
script=["aaa","bbb","ccc"]
Fix it, keeping the backwards compatibility by introducing a new MI
version (MI4), in exactly the same way as was done when fixing
multi-locations breakpoint output in [2].
- Add a fix_breakpoint_script_output uiout flag. MI uiouts will use
this flag if the version is >= 4.
- Add a fix_breakpoint_script_output_globally variable and the
-fix-breakpoint-script-output MI command to set it, if frontends want
to use the fixed output for this without using the newer MI version.
- When emitting the script field, use list instead of tuple, if we want
the fixed output (depending on the two criteria above)
-
[1] https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax
[2] b4be1b0648
Change-Id: I7113c6892832c8d6805badb06ce42496677e2242
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24285
