| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This adds the DAP readMemory and writeMemory requests. A small change
to the evaluation code is needed in order to test this -- this is one
of the few ways for a client to actually acquire a memory reference.
|
|
I noticed a couple of places in infrun.c where we call
set_momentary_breakpoint_at_pc, and then set the newly created
breakpoint's thread field, these are in:
insert_exception_resume_breakpoint
insert_exception_resume_from_probe
Function set_momentary_breakpoint_at_pc calls
set_momentary_breakpoint, which always creates the breakpoint as
thread-specific for the current inferior_thread().
The two insert_* functions mentioned above take an arbitrary
thread_info* as an argument and set the breakpoint::thread to hold the
thread number of that arbitrary thread.
However, the insert_* functions store the breakpoint pointer within
the current inferior_thread(), so we know that the thread being passed
in must be the currently selected thread.
What this means is that we can:
1. Assert that the thread being passed in is the currently selected
thread, and
2. No longer adjust the breakpoint::thread field, this will already
have been set correctly be calling set_momentary_breakpoint_at_pc.
There should be no user visible changes after this commit.
|
|
Consider the following session:
(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 condition for breakpoint 1:
The program being debugged stopped while in a function called from GDB.
Evaluation of the expression containing the function
(some_func) will be abandoned.
When the function is done executing, GDB will silently stop.
Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5
5 return *p;
(gdb)
What happens here is the breakpoint condition includes a call to an
inferior function, and the inferior function segfaults. We can see
that GDB reports the segfault, and then gives an error message that
indicates that an inferior function call was interrupted.
After this GDB appears to report that it is stopped at Breakpoint 1,
inside some_func.
I find this second stop report a little confusing. While it is true
that GDB stopped as a result of hitting breakpoint 1, I think the
message GDB currently prints might give the impression that GDB is
actually stopped at a location of breakpoint 1, which is not the case.
Also, I find the second stop message draws attention away from
the "Program received signal SIGSEGV, Segmentation fault" stop
message, and this second stop might be thought of as replacing in
someway the earlier message.
In short, I think things would be clearer if the second stop message
were not reported at all, so the output should, I think, look like
this:
(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 condition for breakpoint 1:
The program being debugged stopped while in a function called from GDB.
Evaluation of the expression containing the function
(some_func) will be abandoned.
When the function is done executing, GDB will silently stop.
(gdb)
The user can still find the number of the breakpoint that triggered
the initial stop in this line:
Error in testing condition for breakpoint 1:
But there's now only one stop reason reported, the SIGSEGV, which I
think is much clearer.
To achieve this change I set the bpstat::print field when:
(a) a breakpoint condition evaluation failed, and
(b) the $pc of the thread changed during condition evaluation.
I've updated the existing tests that checked the error message printed
when a breakpoint condition evaluation failed.
|
|
Consider the following 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 this line:
Program received signal SIGSEGV, Segmentation fault.
Appears twice in the output. The first time is followed by the
current location. The second time is a little odd, why do we print
that?
Printing that line is controlled, in part, by a global variable,
stopped_by_random_signal. This variable is reset to zero in
handle_signal_stop, and is set if/when GDB figures out that the
inferior stopped due to some random signal.
The problem is, in our case, GDB first stops at the breakpoint for
foo, and enters handle_signal_stop and the stopped_by_random_signal
global is reset to 0.
Later within handle_signal_stop GDB calls bpstat_stop_status, it is
within this function (via bpstat_check_breakpoint_conditions) that the
breakpoint condition is checked, and, we end up calling the inferior
function (some_func in our example above).
In our case above the thread performing the inferior function call
segfaults in some_func. GDB catches the SIGSEGV and handles the stop,
this causes us to reenter handle_signal_stop. The global variable
stopped_by_random_signal is updated, this time it is set to true
because the thread stopped due to SIGSEGV. As a result of this we
print the first instance of the line (as seen above in the example).
Finally we unwind GDB's call stack, the inferior function call is
complete, and we return to the original handle_signal_stop. However,
the stopped_by_random_signal global is still carrying the value as
computed for the inferior function call's stop, which is why we now
print a second instance of the line, as seen in the example.
To prevent this, I propose adding a scoped_restore before we start an
inferior function call. This will save and restore the global
stopped_by_random_signal value.
With this done, the output from our example is now this:
(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 condition for breakpoint 1:
The program being debugged stopped while in a function called from GDB.
Evaluation of the expression containing the function
(some_func) will be abandoned.
When the function is done executing, GDB will silently stop.
Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5
5 return *p;
(gdb)
We now only see the 'Program received signal SIGSEGV, ...' line once,
which I think makes more sense.
Finally, I'm aware that the last few lines, that report the stop as
being at 'Breakpoint 1', when this is not where the thread is actually
located anymore, is not great. I'll address that in the next commit.
|
|
This commit extends gdbserver to take account of a failed memory
access from agent_mem_read, and to return a new eval_result_type
expr_eval_invalid_memory_access.
I have only updated the agent_mem_read calls related directly to
reading memory, I have not updated any of the calls related to
tracepoint data collection. This is just because I'm not familiar
with that area of gdb/gdbserver, and I don't want to break anything,
so leaving the existing behaviour untouched seems like the safest
approach.
I've then updated gdb.base/bp-cond-failure.exp to test evaluating the
breakpoints on the target, and have also extended the test so that it
checks for different sizes of memory access.
|
|
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.
|
|
This commit builds on the previous one to fix all the remaining
failures in gdb.base/unwind-on-each-insn.exp for RISC-V.
The problem we have in gdb.base/unwind-on-each-insn.exp is that, when
we are in the function epilogue, the previous frame and stack pointer
values are being restored, and so, the values that we calculated
during the function prologue are no longer suitable.
Here's an example from the function 'bar' in the mentioned test. This
was compiled for 64-bit RISC-V with compressed instruction support:
Dump of assembler code for function bar:
0x000000000001018a <+0>: add sp,sp,-32
0x000000000001018c <+2>: sd ra,24(sp)
0x000000000001018e <+4>: sd fp,16(sp)
0x0000000000010190 <+6>: add fp,sp,32
0x0000000000010192 <+8>: sd a0,-24(fp)
0x0000000000010196 <+12>: ld a0,-24(fp)
0x000000000001019a <+16>: jal 0x10178 <foo>
0x000000000001019e <+20>: nop
0x00000000000101a0 <+22>: ld ra,24(sp)
0x00000000000101a2 <+24>: ld fp,16(sp)
0x00000000000101a4 <+26>: add sp,sp,32
0x00000000000101a6 <+28>: ret
End of assembler dump.
When we are at address 0x101a4 the previous instruction has restored
the frame-pointer, as such GDB's (current) preference for using the
frame-pointer as the frame base address is clearly not going to work.
We need to switch to using the stack-pointer instead.
At address 0x101a6 the previous instruction has restored the
stack-pointer value. Currently GDB will not understand this and so
will still assume the stack has been decreased by 32 bytes in this
function.
My proposed solution is to extend GDB such that GDB will scan the
instructions at the current $pc looking for this pattern:
ld fp,16(sp)
add sp,sp,32
ret
Obviously the immediates can change, but the basic pattern indicates
that the function is in the process of restoring state before
returning. If GDB sees this pattern then GDB can use the inferior's
position within this instruction sequence to help calculate the
correct frame-id.
With this implemented then gdb.base/unwind-on-each-insn.exp now fully
passes.
Obviously what I've implemented is just a heuristic. It's not going
to work for every function. If the compiler reorders the
instructions, or merges the epilogue back into the function body then
GDB is once again going to get the frame-id wrong.
I'm OK with that, we're no worse off that we are right now in that
situation (plus we can always improve the heuristic later).
Remember, this is for debugging code without debug information,
and (in our imagined situation) with more aggressive levels of
optimisation being used. Obviously GDB is going to struggle in these
situations.
My thinking is, lets get something in place now. Then, later, if
possible, we might be able to improve the logic to cover more
situations -- if there's an interest in doing so. But I figure we
need something in place as a starting point.
After this commit gdb.base/unwind-on-each-insn.exp passes with no
failures on RV64.
|
|
Add support to the RISC-V prologue scanner for c.ldsp and c.lwsp
instructions.
This fixes some of the failures in gdb.base/unwind-on-each-insn.exp,
though there are further failures that are not fixed by this commit.
This change started as a wider fix that would address all the failures
in gdb.base/unwind-on-each-insn.exp, however, that wider fix needed
support for the two additional compressed instructions.
When I added support for those two compressed instructions I noticed
that some of the failures in gdb.base/unwind-on-each-insn.exp resolved
themselves!
Here's what's going on:
The reason for the failures is that GDB is trying to build the
frame-id during the last few instructions of the function. These are
the instructions that restore the frame and stack pointers just prior
to the return instruction itself.
By the time we reach the function epilogue the stack offset that we
calculated during the prologue scan is no longer valid, and so we
calculate the wrong frame-id.
However, in the particular case of interest here, the test function
'foo', the function is so simple and short (the empty function) that
GDB's prologue scan could, in theory, scan every instruction of the
function.
I say "could, in theory," because currently GDB stops the prologue
scan early when it hits an unknown instruction. The unknown
instruction happens to be one of the compressed instructions that I'm
adding support for in this commit.
Now that GDB understands the compressed instructions the prologue scan
really does go from the start of the function right up to the current
program counter. As such, GDB sees that the stack frame has been
allocated, and then deallocated, and so builds the correct frame-id.
Of course, most real functions are not as simple as the test function
'foo'. As such, we can't usually rely on scanning right up to the end
of the function -- there are some instructions we always need to stop
at because GDB can't reason about how they change the inferior
state (e.g. a function call). The test function 'bar' is just such an
example.
After this commit, we can now build the frame-id correctly for every
instruction in 'foo', but there are some tests still failing in 'bar'.
|
|
Convert the RISC-V specific debug settings to use the new debug
printing scheme. This updates the following settings:
set/show debug riscv breakpoints
set/show debug riscv gdbarch
set/show debug riscv infcall
set/show debug riscv unwinder
All of these settings now take a boolean rather than an integer, and
all print their output using the new debug scheme.
There should be no visible change for anyone not turning on debug.
|
|
This commit follows on from the following two commits:
commit 80dc83fd0e70f4d522a534bc601df5e05b81d564
Date: Fri Jun 11 11:30:47 2021 +0100
gdb/remote: handle target dying just before a stepi
And:
commit 079f190d4cfc6aa9c934b00a9134bc0fcc172d53
Date: Thu Mar 9 10:45:03 2023 +0100
[gdb/testsuite] Fix gdb.server/server-kill.exp for remote target
The first of these commits fixed an issue in GDB and tried to extend
the gdb.server/server-kill.exp test to cover the GDB fix.
Unfortunately, the changes to gdb.server/server-kill.exp were not
correct, and were causing problems when trying to run with the
remote-gdbserver-on-localhost board file.
The second commit reverts some of the gdb.server/server-kill.exp
changes introduced in the first commit so that the test will now work
correctly with the remote-gdbserver-on-localhost board file.
The second commit is just about GDB's testing infrastructure -- it's
not about the original fix to GDB from the first commit, the actual
GDB change was fine.
While reviewing the second commit I wanted to check that the problem
fixed in the first commit is still being tested by the
gdb.server/server-kill.exp script, so I reverted the change to
breakpoint.c that is the core of the first commit and ran the test
script ..... and saw no failures.
The first commit is about GDB discovering that gdbserver has died
while trying to insert a breakpoint. As soon as GDB spots that
gdbserver is gone we mourn the remote inferior, which ends up deleting
all the breakpoints associated with the remote inferiors. We then
throw an exception which is caught in the insert breakpoints code, and
we try to display an error that includes the breakpoint number
.... but the breakpoint has already been deleted ... and so GDB
crashes.
After digging a little, what I found is that today, when the test does
'stepi' the first thing we end up doing is calculating the frame-id as
part of the stepi logic, it is during this frame-id calculation that
we mourn the remote inferior, delete the breakpoints, and throw an
exception. The exception is caught by the top level interpreter loop,
and so we never try to print the breakpoint number which is what
caused the original crash.
If I add an 'info frame' command to the test script, prior to killing
gdbserver, then now when we 'stepi' GDB already has the frame-id
calculated, and the first thing we do is try to insert the
breakpoints, this will trigger the original bug.
In order to reproduce this experiment you'll need to change a function
in breakpoint.c, like this:
static void
rethrow_on_target_close_error (const gdb_exception &e)
{
return;
}
Then run gdb.server/server-kill.exp with and without this patch. You
should find that without this patch there are zero test failures,
while with this patch there will be one failure like this:
(gdb) PASS: gdb.server/server-kill.exp: test_stepi: info frame
Executing on target: kill -9 4513 (timeout = 300)
builtin_spawn -ignore SIGHUP kill -9 4513
stepi
../../src/gdb/breakpoint.c:2863: internal-error: insert_bp_location: Assertion `bl->owner != nullptr' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
...
|
|
A potential test failure was introduced with commit:
commit 6bf5f25bb150c0fbcb125e3ee466ba8f9680310b
Date: Wed Mar 8 16:11:30 2023 +0000
gdb/python: make the gdb.unwinder.Unwinder class more robust
In this commit a new test was added, however the expected output
pattern varies depending on which Python version GDB is linked
against.
Older versions of Python result in output like this:
(gdb) python global_test_unwinder.name = "foo"
Traceback (most recent call last):
File "<string>", line 1, in <module>
AttributeError: can't set attribute
Error while executing Python code.
(gdb)
While more recent versions of Python give a similar, but slightly more
verbose error message, like this:
(gdb) python global_test_unwinder.name = "foo"
Traceback (most recent call last):
File "<string>", line 1, in <module>
AttributeError: can't set attribute 'name'
Error while executing Python code.
(gdb)
The test was only accepting the first version of the output. This
commit extends the test pattern so that either version will be
accepted.
|
|
The detection logic for TPIDR2 was implemented incorrectly. Originally
the detection was supposed to be through a ptrace error code, but in reality,
for backwards compatibility, the detection should be based on the size of
the returned iovec.
For instance, if a target supports both TPIDR and TPIDR2, ptrace will return a
iovec size of 16. If a target only supports TPIDR and not TPIDR2, it will
return a iovec size of 8, even if we asked for 16 bytes.
This patch fixes this issue in code that is shared between gdb and gdbserver,
therefore both gdb and gdbserver are fixed.
Tested on AArch64/Linux Ubuntu 20.04.
|
|
I noticed the prefix parameter was unused in print_doc_of_command. And
when removing it, it becomes unused in apropos_cmd.
Change-Id: Id72980b03fe091b22931e6b85945f412b274ed5e
|
|
GDB expected PC should point right after the SVC instruction when the
syscall is active. But some active syscalls keep PC pointing to the SVC
instruction itself.
This leads to a broken backtrace like:
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
#0 0xb6f8681c in pthread_cond_timedwait@@GLIBC_2.4 () from /lib/arm-linux-gnueabihf/libpthread.so.0
#1 0xb6e21f80 in ?? ()
The reason is that .ARM.exidx unwinder gives up if PC does not point
right after the SVC (syscall) instruction. I did not investigate why but
some syscalls will point PC to the SVC instruction itself. This happens
for the "futex" syscall used by pthread_cond_timedwait.
That normally does not matter as ARM prologue unwinder gets called
instead of the .ARM.exidx one. Unfortunately some glibc calls have more
complicated prologue where the GDB unwinder fails to properly determine
the return address (that is in fact an orthogonal GDB bug). I expect it
is due to the "vpush" there in this case but I did not investigate it more:
Dump of assembler code for function pthread_cond_timedwait@@GLIBC_2.4:
0xb6f8757c <+0>: push {r4, r5, r6, r7, r8, r9, r10, r11, lr}
0xb6f87580 <+4>: mov r10, r2
0xb6f87584 <+8>: vpush {d8}
Regression tested on armv7l kernel 5.15.32-v7l+ (Raspbian 11).
Approved-By: Luis Machado <luis.machado@arm.com>
|
|
A recent patch caused my system gcc (Fedora 36, so gcc 12.2.1) to warn
about sym_addr being possibly uninitialized in frame.c. It isn't, but
the compiler can't tell. So, this patch initializes the variable. I
also fixed a formatting buglet that I missed in review.
|
|
With test-case gdb.base/trace-commands.exp and editing off, I run into fails
because multi-line commands are issued using gdb_test_sequence, which
doesn't handle them correctly.
Fix this by using gdb_test instead.
Tested on x86_64-linux.
PR testsuite/30288
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30288
|
|
With test-case gdb.threads/threadapply.exp and editing set to on, we have:
...
(gdb) define remove^M
Type commands for definition of "remove".^M
End with a line saying just "end".^M
>remove-inferiors 3^M
>end^M
(gdb)
...
but with editing set to off, we run into:
...
(gdb) define remove^M
Type commands for definition of "remove".^M
End with a line saying just "end".^M
>remove-inferiors 3^M
end^M
>(gdb) FAIL: gdb.threads/threadapply.exp: thread_set=all: try remove: \
define remove (timeout)
...
The commands are issued by this test:
...
gdb_define_cmd "remove" {
"remove-inferiors 3"
}
...
which does:
- gdb_test_multiple "define remove", followed by
- gdb_test_multiple "remove-inferiors 3\nend".
Proc gdb_test_multiple has special handling for multi-line commands, which
splits it up into subcommands, and for each subcommand issues it and then
waits for the resulting prompt (the secondary prompt ">" for all but the last
subcommand).
However, that doesn't work as expected in this case because the initial
gdb_test_multiple "define remove" fails to match all resulting output, and
consequently the secondary prompt resulting from "define remove" is counted as
if it was the one resulting from "remove-inferiors 3".
Fix this by matching the entire output of "define remove", including the
secondary prompt.
Tested on x86_64-linux.
PR testsuite/30288
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30288
|
|
I noticed that language_demangle shadows the global
"current_language". When I went to fix this, though, I then saw that
language_demangle is only called in two places, and has a comment
saying it should be removed. This patch removes it. Note that the
NULL check in language_demangle is not needed by either of the
existing callers.
Regression tested on x86-64 Fedora 36.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Tom de Vries pointed out a bug in the index-cache background writer --
sometimes it will fail. He also noted that it fails when the number
of worker threads is set to zero. These turn out to be the same
problem -- the cache can't be written to until the per-BFD's
"index_table" member is set.
This patch avoids the race by rearranging the code slightly, to ensure
the cache cannot possibly be written before the member is set.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30261
|
|
Allow consumers of GDB to extract the name of the main method. This is
most useful for Fortran programs which have a variable main method.
Used by both MAP and DDT e.g. it is used to detect the presence of debug
information.
Co-Authored-By: Maciej W. Rozycki <macro@embecosm.com>
|
|
Fix a functional regression and restore the handling of DW_CC_program
code of DW_AT_calling_convention attribute for determining the name of
the starting function of the program where the DW_AT_main_subprogram
attribute has not been provided, such as with Fortran code compiled with
GCC versions 4.5.4 and below, or where DWARF version 3 or below has been
requested. Without it "main" is considered the starting function. Cf.
GCC PR fortran/43414.
Original code was removed with commit 6209cde4ddb8 ("Delete DWARF
psymtab code"), and then an update to complement commit 81873cc81eff
("[gdb/symtab] Support DW_AT_main_subprogram with -readnow.") has also
been included here.
|
|
In the case where a Fortran program has a program name of "main" and
there is also a minimal symbol called main, such as with programs built
with GCC version 4.4.7 or below, the backtrace will erroneously stop at
the minimal symbol rather than the user specified main, e.g.:
(gdb) bt
#0 bar () at .../gdb/testsuite/gdb.fortran/backtrace.f90:17
#1 0x0000000000402556 in foo () at .../gdb/testsuite/gdb.fortran/backtrace.f90:21
#2 0x0000000000402575 in main () at .../gdb/testsuite/gdb.fortran/backtrace.f90:31
#3 0x00000000004025aa in main ()
(gdb)
This patch fixes this issue by increasing the precedence of the full
symbol when the language of the current frame is Fortran.
Newer versions of GCC transform the program name to "MAIN__" in this
case, avoiding the problem.
Co-Authored-By: Maciej W. Rozycki <macro@embecosm.com>
|
|
The removed if statement is already checked in the parent if else
statement.
Co-Authored-By: Christina Schimpe <christina.schimpe@intel.com>
|
|
PR 30219 shows an internal error due to a "Bad switch" in
print_exception() in gdb/exceptions.c. The switch in question
contains cases for RETURN_QUIT and RETURN_ERROR, but is missing a case
for the recently added RETURN_FORCED_QUIT. This commit adds that case.
Making the above change allows the errant test case to pass, but does
not fix the underlying problem, which I'll describe shortly. Even
though the addition of a case for RETURN_FORCED_QUIT isn't the actual
fix, I still think it's important to add this case so that other
situations which lead to print_exeption() being called won't generate
that "Bad switch" internal error.
In order to understand the underlying problem, please examine
this portion of the backtrace from the bug report:
0x5576e4ff5780 print_exception
/home/smarchi/src/binutils-gdb/gdb/exceptions.c:100
0x5576e4ff5930 exception_print(ui_file*, gdb_exception const&)
/home/smarchi/src/binutils-gdb/gdb/exceptions.c:110
0x5576e6a896dd quit_force(int*, int)
/home/smarchi/src/binutils-gdb/gdb/top.c:1849
The real problem is in quit_force; here's the try/catch which
eventually leads to the internal error:
/* Get out of tfind mode, and kill or detach all inferiors. */
try
{
disconnect_tracing ();
for (inferior *inf : all_inferiors ())
kill_or_detach (inf, from_tty);
}
catch (const gdb_exception &ex)
{
exception_print (gdb_stderr, ex);
}
While running the calls in the try-block, a QUIT check is being
performed. This check finds that sync_quit_force_run is (still) set,
causing a gdb_exception_forced_quit to be thrown. The exception
gdb_exception_forced_quit is derived from gdb_exception, causing
exception_print to be called. As shown by the backtrace,
print_exception is then called, leading to the internal error.
The actual fix, also implemented by this commit, is to clear
sync_quit_force_run along with the quit flag. This will allow the
various cleanup code, called by quit_force, to run without triggering
a gdb_exception_forced_quit. (Though, if another SIGTERM is sent to
the gdb process, these flags will be set again and a QUIT check in the
cleanup code will detect it and throw the exception.)
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30219
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
When writing an unwinder it is necessary to create a new class to act
as a frame-id. This new class is almost certainly just going to set a
'sp' and 'pc' attribute within the instance.
This commit adds a little helper class gdb.unwinder.FrameId that does
this job. Users can make use of this to avoid having to write out
standard boilerplate code any time they write an unwinder.
Of course, if the user wants their FrameId class to be more
complicated in some way, then they can still write their own class,
just like they could before.
I've simplified the example code in the documentation to now use the
new helper class, and I've also made use of this helper within the
testsuite.
Any existing user code will continue to work just as it did before
after this change.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
Currently when creating a gdb.UnwindInfo object a user must call
gdb.PendingFrame.create_unwind_info and pass a frame-id object.
The frame-id object should have at least a 'sp' attribute, and
probably a 'pc' attribute too (it can also, in some cases have a
'special' attribute).
Currently all of these frame-id attributes need to be gdb.Value
objects, but the only reason for that requirement is that we have some
code in py-unwind.c that only handles gdb.Value objects.
If instead we switch to using get_addr_from_python in py-utils.c then
we will support both gdb.Value objects and also raw numbers, which
might make things simpler in some cases.
So, I started rewriting pyuw_object_attribute_to_pointer (in
py-unwind.c) to use get_addr_from_python. However, while looking at
the code I noticed a problem.
The pyuw_object_attribute_to_pointer function returns a boolean flag,
if everything goes OK we return true, but we return false in two
cases, (1) when the attribute is not present, which might be
acceptable, or might be an error, and (2) when we get an error trying
to extract the attribute value, in which case a Python error will have
been set.
Now in pending_framepy_create_unwind_info we have this code:
if (!pyuw_object_attribute_to_pointer (pyo_frame_id, "sp", &sp))
{
PyErr_SetString (PyExc_ValueError,
_("frame_id should have 'sp' attribute."));
return NULL;
}
Notice how we always set an error. This will override any error that
is already set.
So, if you create a frame-id object that has an 'sp' attribute, but
the attribute is not a gdb.Value, then currently we fail to extract
the attribute value (it's not a gdb.Value) and set this error in
pyuw_object_attribute_to_pointer:
rc = pyuw_value_obj_to_pointer (pyo_value.get (), addr);
if (!rc)
PyErr_Format (
PyExc_ValueError,
_("The value of the '%s' attribute is not a pointer."),
attr_name);
Then we return to pending_framepy_create_unwind_info and immediately
override this error with the error about 'sp' being missing.
This all feels very confused.
Here's my proposed solution: pyuw_object_attribute_to_pointer will now
return a tri-state enum, with states OK, MISSING, or ERROR. The
meanings of these states are:
OK - Attribute exists and was extracted fine,
MISSING - Attribute doesn't exist, no Python error was set.
ERROR - Attribute does exist, but there was an error while
extracting it, a Python error was set.
We need to update pending_framepy_create_unwind_info, the only user of
pyuw_object_attribute_to_pointer, but now I think things are much
clearer. Errors from lower levels are not blindly overridden with the
generic meaningless error message, but we still get the "missing 'sp'
attribute" error when appropriate.
This change also includes the switch to get_addr_from_python which was
what started this whole journey.
For well behaving user code there should be no visible changes after
this commit.
For user code that hits an error, hopefully the new errors should be
more helpful in figuring out what's gone wrong.
Additionally, users can now use integers for the 'sp' and 'pc'
attributes in their frame-id objects if that is useful.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
While refactoring some other code in gdb/python/* I wanted to merge
two code paths. One path calls value_as_address, while the other
calls unpack_pointer.
I suspect calling value_as_address is the correct choice, but, while
examining the code I noticed that value_as_address calls unpack_long
rather than unpack_pointer.
Under the hood, unpack_pointer does just call unpack_long so there's
no real difference here, but it feels like value_as_address should
call unpack_pointer.
I've updated the code to use unpack_pointer, and changed a related
comment to say that we call unpack_pointer. I've also adjusted the
header comment on value_as_address. The existing header refers to
some code that is now commented out.
Rather than trying to describe the whole algorithm of
value_as_address, which is already well commented within the function,
I've just trimmed the comment on value_as_address to be a brief
summary of what the function does.
There should be no user visible changes after this commit.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
It is not currently possible to directly create gdb.UnwindInfo
instances, they need to be created by calling
gdb.PendingFrame.create_unwind_info so that the newly created
UnwindInfo can be linked to the pending frame.
As such there's no tp_init method defined for UnwindInfo.
A consequence of all this is that it doesn't really make sense to
allow sub-classing of gdb.UnwindInfo. Any sub-class can't call the
parents __init__ method to correctly link up the PendingFrame
object (there is no parent __init__ method). And any instances that
sub-classes UnwindInfo but doesn't call the parent __init__ is going
to be invalid for use in GDB.
This commit removes the Py_TPFLAGS_BASETYPE flag from the UnwindInfo
class, which prevents the class being sub-classed. Then I've added a
test to check that this is indeed prevented.
Any functional user code will not have any issues with this change.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
Having a useful __repr__ method can make debugging Python code that
little bit easier. This commit adds __repr__ for gdb.PendingFrame and
gdb.UnwindInfo classes, along with some tests.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
The gdb.Frame class has far more methods than gdb.PendingFrame. Given
that a PendingFrame hasn't yet been claimed by an unwinder, there is a
limit to which methods we can add to it, but many of the methods that
the Frame class has, the PendingFrame class could also support.
In this commit I've added those methods to PendingFrame that I believe
are safe.
In terms of implementation: if I was starting from scratch then I
would implement many of these (or most of these) as attributes rather
than methods. However, given both Frame and PendingFrame are just
different representation of a frame, I think there is value in keeping
the interface for the two classes the same. For this reason
everything here is a method -- that's what the Frame class does.
The new methods I've added are:
- gdb.PendingFrame.is_valid: Return True if the pending frame
object is valid.
- gdb.PendingFrame.name: Return the name for the frame's function,
or None.
- gdb.PendingFrame.pc: Return the $pc register value for this
frame.
- gdb.PendingFrame.language: Return a string containing the
language for this frame, or None.
- gdb.PendingFrame.find_sal: Return a gdb.Symtab_and_line object
for the current location within the pending frame, or None.
- gdb.PendingFrame.block: Return a gdb.Block for the current
pending frame, or None.
- gdb.PendingFrame.function: Return a gdb.Symbol for the current
pending frame, or None.
In every case I've just copied the implementation over from gdb.Frame
and cleaned the code slightly e.g. NULL to nullptr. Additionally each
function required a small update to reflect the PendingFrame type, but
that's pretty minor.
There are tests for all the new methods.
For more extensive testing, I added the following code to the file
gdb/python/lib/command/unwinders.py:
from gdb.unwinder import Unwinder
class TestUnwinder(Unwinder):
def __init__(self):
super().__init__("XXX_TestUnwinder_XXX")
def __call__(self,pending_frame):
lang = pending_frame.language()
try:
block = pending_frame.block()
assert isinstance(block, gdb.Block)
except RuntimeError as rte:
assert str(rte) == "Cannot locate block for frame."
function = pending_frame.function()
arch = pending_frame.architecture()
assert arch is None or isinstance(arch, gdb.Architecture)
name = pending_frame.name()
assert name is None or isinstance(name, str)
valid = pending_frame.is_valid()
pc = pending_frame.pc()
sal = pending_frame.find_sal()
assert sal is None or isinstance(sal, gdb.Symtab_and_line)
return None
gdb.unwinder.register_unwinder(None, TestUnwinder())
This registers a global unwinder that calls each of the new
PendingFrame methods and checks the result is of an acceptable type.
The unwinder never claims any frames though, so shouldn't change how
GDB actually behaves.
I then ran the testsuite. There was only a single regression, a test
that uses 'disable unwinder' and expects a single unwinder to be
disabled -- the extra unwinder is now disabled too, which changes the
test output. So I'm reasonably confident that the new methods are not
going to crash GDB.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
This commit copies the pattern that is present in many other py-*.c
files: having a single macro to check that the Python object is still
valid.
This cleans up the code a little throughout the py-unwind.c file.
Some of the exception messages will change slightly with this commit,
though the type of the exceptions is still ValueError in all cases.
I started writing some tests for this change and immediately ran into
a problem: GDB would crash. It turns out that the PendingFrame
objects are not being marked as invalid!
In pyuw_sniffer where the pending frames are created, we make use of a
scoped_restore to invalidate the pending frame objects. However, this
only restores the pending_frame_object::frame_info field to its
previous value -- and it turns out we never actually give this field
an initial value, it's left undefined.
So, when the scoped_restore (called invalidate_frame) performs its
cleanup, it actually restores the frame_info field to an undefined
value. If this undefined value is not nullptr then any future
accesses to the PendingFrame object result in undefined behaviour and
most likely, a crash.
As part of this commit I now initialize the frame_info field, which
ensures all the new tests now pass.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
Spotted a redundant nullptr check in python/py-frame.c in the function
frapy_block. This was introduced in commit 57126e4a45e3000e when we
expanded an earlier check in return early if the pointer in question
is nullptr.
There should be no user visible changes after this commit.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
This commit makes a few related changes to the gdb.unwinder.Unwinder
class attributes:
1. The 'name' attribute is now a read-only attribute. This prevents
user code from changing the name after registering the unwinder. It
seems very unlikely that any user is actually trying to do this in
the wild, so I'm not very worried that this will upset anyone,
2. We now validate that the name is a string in the
Unwinder.__init__ method, and throw an error if this is not the
case. Hopefully nobody was doing this in the wild. This should
make it easier to ensure the 'info unwinder' command shows sane
output (how to display a non-string name for an unwinder?),
3. The 'enabled' attribute is now implemented with a getter and
setter. In the setter we ensure that the new value is a boolean,
but the real important change is that we call
'gdb.invalidate_cached_frames()'. This means that the backtrace
will be updated if a user manually disables an unwinder (rather than
calling the 'disable unwinder' command). It is not unreasonable to
think that a user might register multiple unwinders (relating to
some project) and have one command that disables/enables all the
related unwinders. This command might operate by poking the enabled
attribute of each unwinder object directly, after this commit, this
would now work correctly.
There's tests for all the changes, and lots of documentation updates
that both cover the new changes, but also further improve (I think)
the general documentation for GDB's Unwinder API.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
The test case in this patch shows an unusual situation: an Ada array
has a dynamic bound, but the bound comes from a frame that's referred
to by the static link. This frame is correctly found when evaluating
the array variable itself, but is lost when evaluating the array's
bounds.
This patch fixes the problem by passing this frame through to
value_at_lazy in the DWARF expression evaluator.
|
|
This patch adds a 'frame' parameter to value_at_lazy and ensures that
it is passed down to the call to resolve_dynamic_type. This required
also adding a frame parameter to value_from_contents_and_address.
Nothing passes this parameter to value_at_lazy yet, so this patch
should have no visible effect.
|
|
This adds a frame parameter to resolve_dynamic_type and arranges for
it to be passed through the call tree and, in particular, to all calls
to dwarf2_evaluate_property.
Nothing passes this parameter yet, so this patch should have no
visible effect.
A 'const frame_info_ptr *' is used here to avoid including frame.h
from gdbtypes.h.
|
|
version_at_least is a less capable variant of version_compare, so this
patch removes it.
|
|
This rewrites version_compare to allow the input lists to have
different lengths, then rewrites rust_at_least to use version_compare.
|
|
This adds a 'rust_at_least' helper proc, for checking the version of
the Rust compiler in use. It then changes various tests to use this
with 'require'.
|
|
With test-case gdb.ada/verylong.exp and gnatmake 7.5.0 I run into:
...
compilation failed: gcc ... $src/gdb/testsuite/gdb.ada/verylong/prog.adb
prog.adb:16:11: warning: file name does not match unit name, should be "main.adb"
prog.adb:17:08: "Long_Long_Long_Integer" is undefined (more references follow)
gnatmake: "prog.adb" compilation error
FAIL: gdb.ada/verylong.exp: compilation prog.adb
...
AFAICT, support for Long_Long_Long_Integer was added in gcc 11.
Fix this by requiring gnatmake version 11 or higher in the test-case.
Tested on x86_64-linux.
|
|
Co-Authored-By: Christina Schimpe <christina.schimpe@intel.com>
|
|
The ERROR_NO_INFERIOR macro is already called at the beginning of the
function continue_command. Since target/inferior are not switched in-between,
the second call to it is redundant.
Co-Authored-By: Christina Schimpe <christina.schimpe@intel.com>
|
|
It was pointed out during review of another patch that the function
displaced_step_dump_bytes really isn't specific to displaced stepping,
and should really get a more generic name and move into gdbsupport/.
This commit does just that. The function is renamed to
bytes_to_string and is moved into gdbsupport/common-utils.{cc,h}. The
function implementation doesn't really change. Much...
... I have updated the function to take an array view, which makes it
slightly easier to call in a couple of places where we already have a
gdb::bytes_vector. I've then added an inline wrapper to convert a raw
pointer and length into an array view, which is used in places where
we don't easily have a gdb::bytes_vector (or similar).
Updated all users of displaced_step_dump_bytes.
There should be no user visible changes after this commit.
Finally, I ended up having to add an include of gdb_assert.h into
array-view.h. When I include array-view.h into common-utils.h I ran
into build problems because array-view.h calls gdb_assert.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
While investigating a displaced stepping issue I wanted an easy way to
see what GDB thought the original instruction was, and what
instruction GDB replaced that with when performing the displaced step.
We do print out the address that is being stepped, so I can track down
the original instruction, I just need to go find the information
myself.
And we do print out the bytes of the new instruction, so I can figure
out what the replacement instruction was, but it's not really easy.
Also, the code that prints the bytes of the replacement instruction
only prints 4 bytes, which clearly isn't always going to be correct.
In this commit I remove the existing code that prints the bytes of the
replacement instruction, and add two new blocks of code to
displaced_step_prepare_throw. This new code prints the original
instruction, and the replacement instruction. In each case we print
both the bytes that make up the instruction and the completely
disassembled instruction.
Here's an example of what the output looks like on x86-64 (this is
with 'set debug displaced on'). The two interesting lines contain the
strings 'original insn' and 'replacement insn':
(gdb) step
[displaced] displaced_step_prepare_throw: displaced-stepping 2892655.2892655.0 now
[displaced] displaced_step_prepare_throw: original insn 0x401030: ff 25 e2 2f 00 00 jmp *0x2fe2(%rip) # 0x404018 <puts@got.plt>
[displaced] prepare: selected buffer at 0x401052
[displaced] prepare: saved 0x401052: 1e fa 31 ed 49 89 d1 5e 48 89 e2 48 83 e4 f0 50
[displaced] fixup_riprel: %rip-relative addressing used.
[displaced] fixup_riprel: using temp reg 2, old value 0x7ffff7f8a578, new value 0x401036
[displaced] amd64_displaced_step_copy_insn: copy 0x401030->0x401052: ff a1 e2 2f 00 00 68 00 00 00 00 e9 e0 ff ff ff
[displaced] displaced_step_prepare_throw: prepared successfully thread=2892655.2892655.0, original_pc=0x401030, displaced_pc=0x401052
[displaced] displaced_step_prepare_throw: replacement insn 0x401052: ff a1 e2 2f 00 00 jmp *0x2fe2(%rcx)
[displaced] finish: restored 2892655.2892655.0 0x401052
[displaced] amd64_displaced_step_fixup: fixup (0x401030, 0x401052), insn = 0xff 0xa1 ...
[displaced] amd64_displaced_step_fixup: restoring reg 2 to 0x7ffff7f8a578
0x00007ffff7e402c0 in puts () from /lib64/libc.so.6
(gdb)
One final note. For many targets that support displaced stepping (in
fact all targets except ARM) the replacement instruction is always a
single instruction. But on ARM the replacement could actually be a
series of instructions.
The debug code tries to handle this by disassembling the entire
displaced stepping buffer. Obviously this might actually print more
than is necessary, but there's (currently) no easy way to know how
many instructions to disassemble; that knowledge is all locked in the
architecture specific code. Still I don't think it really hurts, if
someone is looking at this debug then hopefully they known what to
expect.
Obviously we can imagine schemes where the architecture specific
displaced stepping code could communicate back how many bytes its
replacement sequence was, and then our debug print code could use this
to limit the disassembly. But this seems like a lot of effort just to
save printing a few additional instructions in some debug output.
I'm not proposing to do anything about this issue for now.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Fix test-case gdb.guile/scm-symbol.exp for remote host by making a regexp less
strict.
Likewise in gdb.guile/scm-symtab.exp.
Tested on x86_64-linux.
|
|
Fix test-case gdb.guile/scm-parameter.exp for remote host by taking into
account that gdb_reinitialize_dir has no effect for remote host.
Tested on x86_64-linux.
|
|
Fix test-case gdb.guile/scm-objfile-script.exp using gdb_remote_download.
Tested on x86_64-linux.
|
|
Fix test-case gdb.guile/scm-objfile-script.exp using host_standard_output_file.
Tested on x86_64-linux.
|
|
Fix test-case gdb.guile/scm-cmd.exp using readline_is_used.
Tested on x86_64-linux.
|
|
Fix test-case gdb.guile/guile.exp for remote host using gdb_remote_download.
Tested on x86_64-linux.
|
