| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Use the available `switch_to_target_no_thread` function to switch the
target. This is a refactoring.
gdb/ChangeLog:
2020-09-07 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* infrun.c (fetch_inferior_event): Use
`switch_to_target_no_thread` to switch the target.
|
|
dwarf2_free_objfile no longer exists, so this patch removes its
declaration from symfile.h.
gdb/ChangeLog
2020-09-06 Tom Tromey <tom@tromey.com>
* symfile.h (dwarf2_free_objfile): Don't declare.
|
|
In the test cases complex.exp,pointer-to-pointer.exp,vla-ptr-info.exp
fortran.exp routines are not used, which are to determine the type/kind
string. Due to this these test incorrectly fail for Flang.
Now test cases are modified to use fortran.exp routines. fortran.exp
file is modified to add absent routines fortran_complex8 and
fortran_complex16.
gdb/testsuite/ChangeLog
* lib/fortran.exp (fortran_complex8): New proc.
(fortran_complex16): New proc.
* gdb.fortran/complex.exp: Use routines from fortran.exp
* gdb.fortran/pointer-to-pointer.exp: Likewise.
* gdb.fortran/vla-ptr-info.exp: Likewise.
|
|
Currently GDB is not able to print correct value for real/complex type
from binary generated from Flang compiler. This is due to GDB not able
to recognise and determine correct format floatformats_ia64_quad and
instead falling back to default_floatformat_for_type. This leads
incorrect output.
Now function i386_floatformat_for_type is fixed to correctly identify
Flang generated 16 byte real/complex type.
gdb/ChangeLog
* gdb/i386-tdep.c (i386_floatformat_for_type): Added conditions
to match 16 byte real/complex type generated by Flang compiler.
|
|
When adding:
...
if ![runto_main] then {
fail "can't run to main"
return 0
}
...
to test-case gdb.base/label-without-address.exp and running it with target
board unix/-fPIE/-pie, we run into:
...
(gdb) break main:L1^M
Breakpoint 2 at 0x555555554000: file label-without-address.c, line 22.^M
...
That is, for a label with optimized-out address, we set a breakpoint at the
relocation base.
The root cause is that the dwarf reader, despite finding that attribute
DW_AT_low_pc is missing, still tags the L1 symbol as having LOC_LABEL, which
means it has a valid address, which defaults to 0.
Fix this by instead tagging the L1 symbol with LOC_OPTIMIZED_OUT.
Tested on x86_64-linux.
gdb/ChangeLog:
2020-09-03 Tom de Vries <tdevries@suse.de>
PR breakpoint/26546
* dwarf2/read.c (new_symbol): Tag label symbol without DW_AT_low_pc as
LOC_OPTIMIZED_OUT instead of LOC_LABEL.
gdb/testsuite/ChangeLog:
2020-09-03 Tom de Vries <tdevries@suse.de>
PR breakpoint/26546
* gdb.base/label-without-address.exp: Runto main first.
|
|
Since the "maintenance info sections" helper functions are not used
through a callback with a void* parameter anymore, the
maint_print_section_data is not needed anymore. Remove it, replace it
with regular parameters.
Break out the index digits computation in its own function.
gdb/ChangeLog:
* maint.c (index_digits): New function.
(struct maint_print_section_data): Remove.
(print_bfd_section_info): Remove print_data parameter, add arg
and index_digits.
(print_objfile_section_info): Likewise.
(print_bfd_section_info_maybe_relocated): Likewise (plus
objfile).
(maintenance_info_sections): Adjust calls.
Change-Id: Idfeca5e7e0a95e72fade15cb1488058865c0258e
|
|
The Ada varobj code automatically dereferences access types. This is
often handy, but it also does so for null pointers -- showing children
with empty values.
These children are weird, but even weirder when a variant type is
involved, because only the non-varying parts of the type are
displayed. This behavior conflicts a bit with my ongoing quest to
move the Ada code to use DWARF rather than gnat encodings, in that
reproducing this behavior with the DWARF code seems rather hacky.
So, this patch instead changes the Ada varobj code so that it does not
automatically dereference null pointers.
As this patch only affects Ada, and it was already reviewed internally
by Joel, I am checking it in.
2020-09-02 Tom Tromey <tromey@adacore.com>
* ada-varobj.c (ada_varobj_get_ptr_number_of_children): Return 0
for null pointers.
(ada_varobj_adjust_for_child_access): Special-case null pointers.
gdb/testsuite/ChangeLog
2020-09-02 Tom Tromey <tromey@adacore.com>
* gdb.ada/mi_var_access.exp: Test children of access variable.
* gdb.ada/mi_var_access/mi_access.adb: Add new stop markers.
* gdb.ada/mi_var_array.exp: Update.
|
|
It is currently an int, but it is used as a bool.
gdb/ChangeLog:
* bcache.h (struct bcache) <insert>: Change type of `added` to
pointer to bool.
* bcache.c (bcache::insert): Likewise.
* gdbtypes.c (check_types_worklist): Adjust.
* psymtab.c (add_psymbol_to_bcache): Adjust.
Change-Id: I06b1041636c656782a89cb6106c9ae2593f61616
|
|
When testing on docker using the AUFS storage driver, loading a core
file will often print a number of warnings. Here's an example (with
the pathname shortened somewhat):
warning: Can't open file /var/lib/docker/aufs/diff/d07..e21/lib/x86_64-linux-gnu/libc-2.27.so during file-backed mapping note processing
The "warning-free" test in gdb.base/corefile.exp will fail if any
warnings are printed, but this particular warning is unavoidable when
running in the docker environment. Fortunately, the path mentions
both "docker" and "aufs", making it easy to XFAIL this case.
gdb/testsuite/ChangeLog:
* gdb.base/corefile.exp (warning-free): XFAIL test when running
on docker w/ AUFS storage driver.
|
|
Luis Machado reported some regressions after I pushed recent core file
related patches fixing BZ 25631:
FAIL: gdb.base/corefile.exp: backtrace in corefile.exp
FAIL: gdb.base/corefile.exp: core-file warning-free
FAIL: gdb.base/corefile.exp: print func2::coremaker_local
FAIL: gdb.base/corefile.exp: up in corefile.exp
FAIL: gdb.base/corefile.exp: up in corefile.exp (reinit)
This commit fixes these regressions. Thanks to Luis for testing
an earlier version of the patch. (I was unable to reproduce these
regressions in various test environments that I created.)
Luis is testing in a docker container which is using the AUFS storage
driver. It turns out that the kernel is placing docker host paths in
the NT_FILE note instead of paths within the container.
I've made a similar docker environment (though apparently not similar
enough to reproduce the regressions). This is one of the paths that
I see mentioned in the warning messages printed while loading the
core file during NT_FILE note processing - note that I've shortened
the path component starting with "d07c4":
/var/lib/docker/aufs/diff/d07c4...21/lib/x86_64-linux-gnu/ld-2.27.so
This is a path on the docker host; it does not exist in the
container. In the docker container, this is the path:
/lib/x86_64-linux-gnu/ld-2.27.so
My first thought was to disable all NT_FILE mappings when any path was
found to be bad. This would have caused GDB to fall back to accessing
memory using the file stratum as it did before I added the NT_FILE
note loading code. After further consideration, I realized that we
could do better than this. For file-backed memory access, we can
still use the NT_FILE mappings when available, and then attempt to
access memory using the file stratum constrained to those address
ranges corresponding to the "broken" mappings.
In order to test it, I made some additions to corefile2.exp in which
the test case's executable is renamed. The core file is then loaded;
due to the fact that the executable has been renamed, those mappings
will be unavailable. After loading the core file, the executable is
renamed back to its original name at which point it is loaded using
GDB's "file" command. The "interesting" tests are then run. These
tests will print out values in file-backed memory regions along with
mmap'd regions placed within/over the file-backed regions. Despite
the fact that the executable could not be found during the NT_FILE
note processing, these tests still work correctly due to the fact that
memory is available from the file stratum combined with the fact that
the broken NT_FILE mappings are used to prevent file-backed access
outside of the "broken" mappings.
gdb/ChangeLog:
* corelow.c (unordered_set): Include.
(class core_target): Add field 'm_core_unavailable_mappings'.
(core_target::build_file_mappings): Print only one warning
per inaccessible file. Add unavailable/broken mappings
to m_core_unavailable_mappings.
(core_target::xfer_partial): Call...
(core_target::xfer_memory_via_mappings): New method.
gdb/testsuite/ChangeLog:
* gdb.base/corefile2.exp (renamed binfile): New tests.
|
|
gdb/ChangeLog:
* dwarf2/read.c (struct field_info) <non_public_fields>: Change
type to bool.
(dwarf2_add_field): Use true instead of 1.
Change-Id: I7e9c86429402c28d4f15861d17976b9c50049f94
|
|
The indentation is off, fix it before doing other changes.
gdb/ChangeLog:
* dwarf2/read.c (struct field_info): Fix indentation.
Change-Id: Ife6a3d017abcf0a33e49e47e51429e95d504343c
|
|
I forgot to fix some nits pointed out in review before merging the
"frame inlined in outer frame series", this patch fixes them.
gdb/ChangeLog:
* frame-unwind.h (frame_prev_register_ftype): Fix adjective
ordering in comment.
* frame.c (frame_id_eq): Fix indentation.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/dw2-reg-undefined.exp: Remove spurious #.
Change-Id: Iaddde9677fc3f68382558d1a16f5a0b4beb78bac
|
|
Remove the restriction (gdb_assert) that prevents creating frames
inlined in the outer frame. Like for frames inlined in a standard frame
(FID_STACK_VALID), a frame inlined into the outer frame will have:
- artificial_depth greater than 0
- code_addr equal to the first executed instruction in the block
corresponding to the inlined function
It will however have its stack_status set to FID_STACK_OUTER, like the
outer frame.
This is not typically seen on your everyday system (e.g. a Linux /
x86-64 process), because the outer frame would be for instance the
_start function, probably written in assembly and very unlikely to have
anything inlined in it. However this could happen in more "bare-metal"
scenarios. In particular, this was seen in ROCm GDB [1], where the
compiler does inline functions in the top-level kernel functions (kernel
in the sense of compute kernel, not userspace vs kernel).
I however wrote a test that replicates the issue on x86-64 and a few
other arches I had access to. Since we need to control precisely the
emitted DWARF CFI, I didn't find another way than to write it in
assembly. The DWARF is generated using the testsuite's DWARF assembler,
except the unwind information, which is written using CFI directives
(and therefore generated by the actual assembler). I think the test is
adequately commented, but if anything is unclear, just ask and I'll add
more info.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb/
gdb/ChangeLog:
YYYY-MM-DD Scott Linder <scott@scottlinder.com>
YYYY-MM-DD Simon Marchi <simon.marchi@efficios.com>
* inline-frame.c (inline_frame_this_id): Remove assert that prevents
inline frame ids in outer frame.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/frame-inlined-in-outer-frame.exp: New file.
* gdb.dwarf2/frame-inlined-in-outer-frame.S: New file.
Change-Id: I8aa129c667dccc31590ffdf426586418493a6ebe
|
|
In the following patch, we'll need to easily differentiate the frame_id
of the outer frame (or the frame id of a frame inlined into the outer
frame) from a simply invalid frame id.
Currently, the frame id of the outer frame has `stack_status` set to
FID_STACK_INVALID plus special_addr_p set. A frame inlined into the
outer frame would also have `artificial_depth` set to greater than one.
That makes the job of differntiating the frame id of the outer frame (or a
frame inlined into the outer frame) cumbersome.
To make it easier, give the outer frame id its own frame_id_stack_status
enum value. outer_frame_id then becomes very similar to
sentinel_frame_id, another "special" frame id value.
In frame_id_p, we don't need a special case for the outer frame id, as
it's no long a special case of FID_STACK_INVALID. Same goes for
frame_id_eq.
So in the end, FID_STACK_OUTER isn't even used (except in
fprint_frame_id). But that's expected: all the times we wanted to
identify an outer frame was to differentiate it from an otherwise
invalid frame. Since their frame_id_stack_status value is different
now, that is done naturally.
gdb/ChangeLog:
* frame.h (enum frame_id_stack_status) <FID_STACK_OUTER>: New.
* frame.c (fprint_frame_id): Handle FID_STACK_OUTER.
(outer_frame_id): Use FID_STACK_OUTER instead of
FID_STACK_INVALID.
(frame_id_p): Don't check for outer_frame_id.
Change-Id: I654e7f936349debc4f04f7f684b15e71a0c37619
|
|
TLDR: frame_unwind_got_optimized uses wrong frame id value, trying to
fix it makes GDB sad, return not_lval value and don't use frame id value
instead.
Longer version:
The `prev_register` method of the `frame_unwind` interface corresponds
to asking the question: "where did this frame - passed as a parameter -
save the value this register had in its caller frame?". When "this
frame" did not save that register value (DW_CFA_undefined in DWARF), the
implementation can use the `frame_unwind_got_optimized` function to
create a struct value that represents the optimized out / not saved
register.
`frame_unwind_got_optimized` marks the value as fully optimized out,
sets the lval field to lval_register and assigns the required data for
lval_register: the next frame id and the register number. The problem
is that it uses the frame id from the wrong frame (see below for in
depth explanation). In practice, this is not problematic because the
frame id is never used: the value is already not lazy (and is marked as
optimized out), so the value is never fetched from the target.
When trying to change it to put the right next frame id in the value, we
bump into problems: computing the frame id for some frame requires
unwinding some register, if that register is not saved / optimized out,
we try to get the frame id that we are currently computing.
This patch addresses the problem by changing
`frame_unwind_got_optimized` to return a not_lval value instead. Doing
so, we don't need to put a frame id, so we don't hit that problem. It
may seem like an unnecessary change today, because it looks like we're
fixing something that is not broken (from the user point of view).
However, the bug becomes user visible with the following patches, where
inline frames are involved. I put this change in its own patch to keep
it logically separate.
Let's now illustrate how we are putting the wrong frame id in the value
returned by `frame_unwind_got_optimized`. Let's assume this stack:
frame #0
frame #1
frame #2
frame #3
Let's suppose that we are calling `frame_unwind_register_value` with
frame #2 as the "next_frame" parameter and some register number X as the
regnum parameter. That is like asking the question "where did frame #2
save frame #3's value for register X".
`frame_unwind_register_value` calls the frame unwinder's `prev_register`
method, which in our case is `dwarf2_frame_prev_register`. Note that in
`dwarf2_frame_prev_register`, the parameter is now called `this_frame`,
but its value is still frame #2, and we are still looking for where
frame #2 saved frame #3's value of register X.
Let's now suppose that frame #2's CFI explicitly indicates that the
register X is was not saved (DW_CFA_undefined). We go into
`frame_unwind_got_optimized`.
In `frame_unwind_got_optimized`, the intent is to create a value that
represents register X in frame #3. An lval_register value requires that
we specify the id of the _next_ frame, that is the frame from which we
would need to unwind in order to get the value. Therefore, we would
want to put the id of frame #2 in there.
However, `frame_unwind_got_optimized` does:
VALUE_NEXT_FRAME_ID (val)
= get_frame_id (get_next_frame_sentinel_okay (frame));
where `frame` is frame #2. The get_next_frame_sentinel_okay call
returns frame #1, so we end up putting frame #1's id in the value.
Let's now pretend that we try to "fix" it by placing the right frame id,
in other words doing this change:
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -260,8 +260,7 @@ frame_unwind_got_optimized (struct frame_info *frame, int regnum)
mark_value_bytes_optimized_out (val, 0, TYPE_LENGTH (type));
VALUE_LVAL (val) = lval_register;
VALUE_REGNUM (val) = regnum;
- VALUE_NEXT_FRAME_ID (val)
- = get_frame_id (get_next_frame_sentinel_okay (frame));
+ VALUE_NEXT_FRAME_ID (val) = get_frame_id (frame);
return val;
}
This makes some tests fails, such as gdb.dwarf2/dw2-undefined-ret-addr.exp,
like so:
...
#9 0x0000557a8ab15a5d in internal_error (file=0x557a8b31ef80 "/home/simark/src/binutils-gdb/gdb/frame.c", line=623, fmt=0x557a8b31efe0 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:55
#10 0x0000557a87f816d6 in get_frame_id (fi=0x62100034bde0) at /home/simark/src/binutils-gdb/gdb/frame.c:623
#11 0x0000557a87f7cac7 in frame_unwind_got_optimized (frame=0x62100034bde0, regnum=16) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:264
#12 0x0000557a87a71a76 in dwarf2_frame_prev_register (this_frame=0x62100034bde0, this_cache=0x62100034bdf8, regnum=16) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1267
#13 0x0000557a87f86621 in frame_unwind_register_value (next_frame=0x62100034bde0, regnum=16) at /home/simark/src/binutils-gdb/gdb/frame.c:1288
#14 0x0000557a87f855d5 in frame_register_unwind (next_frame=0x62100034bde0, regnum=16, optimizedp=0x7fff5f459070, unavailablep=0x7fff5f459080, lvalp=0x7fff5f4590a0, addrp=0x7fff5f4590b0, realnump=0x7fff5f459090, bufferp=0x7fff5f459150 "") at /home/simark/src/binutils-gdb/gdb/frame.c:1191
#15 0x0000557a87f860ef in frame_unwind_register (next_frame=0x62100034bde0, regnum=16, buf=0x7fff5f459150 "") at /home/simark/src/binutils-gdb/gdb/frame.c:1247
#16 0x0000557a881875f9 in i386_unwind_pc (gdbarch=0x621000190110, next_frame=0x62100034bde0) at /home/simark/src/binutils-gdb/gdb/i386-tdep.c:1971
#17 0x0000557a87fe58a5 in gdbarch_unwind_pc (gdbarch=0x621000190110, next_frame=0x62100034bde0) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:3062
#18 0x0000557a87a6267b in dwarf2_tailcall_sniffer_first (this_frame=0x62100034bde0, tailcall_cachep=0x62100034bee0, entry_cfa_sp_offsetp=0x7fff5f4593f0) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:387
#19 0x0000557a87a70cdf in dwarf2_frame_cache (this_frame=0x62100034bde0, this_cache=0x62100034bdf8) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1198
#20 0x0000557a87a711c2 in dwarf2_frame_this_id (this_frame=0x62100034bde0, this_cache=0x62100034bdf8, this_id=0x62100034be40) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1226
#21 0x0000557a87f81167 in compute_frame_id (fi=0x62100034bde0) at /home/simark/src/binutils-gdb/gdb/frame.c:587
#22 0x0000557a87f81803 in get_frame_id (fi=0x62100034bde0) at /home/simark/src/binutils-gdb/gdb/frame.c:635
#23 0x0000557a87f7efef in scoped_restore_selected_frame::scoped_restore_selected_frame (this=0x7fff5f459920) at /home/simark/src/binutils-gdb/gdb/frame.c:320
#24 0x0000557a891488ae in print_frame_args (fp_opts=..., func=0x621000183b90, frame=0x62100034bde0, num=-1, stream=0x6030000caa20) at /home/simark/src/binutils-gdb/gdb/stack.c:750
#25 0x0000557a8914e87a in print_frame (fp_opts=..., frame=0x62100034bde0, print_level=0, print_what=SRC_AND_LOC, print_args=1, sal=...) at /home/simark/src/binutils-gdb/gdb/stack.c:1394
#26 0x0000557a8914c2ae in print_frame_info (fp_opts=..., frame=0x62100034bde0, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1119
...
We end up calling get_frame_id (in the hunk above, frame #10) while we are
computing it (frame #21), and that's not good.
Now, the question is how do we fix this. I suggest making the unwinder
return a not_lval value in this case.
The reason why we return an lval_register here is to make sure that this
is printed as "not saved" and not "optimized out" down the line. See
these two commits:
1. 901461f8eb40 ("Print registers not saved in the frame as "<not saved>"
instead of "<optimized out>".").
2. 6bd273ae450b ("Make "set debug frame 1" output print <not saved> instead of
<optimized out>.")
The current design (introduced by the first commit) is to check the
value's lval to choose which one to print (see val_print_optimized_out).
Making the unwinder return not_lval instead of lval_register doesn't
break "not saved" when doing "print $rax" or "info registers", because
value_fetch_lazy_register only consumes the contents and optimized-out
property from the value the unwinder returned. The value being
un-lazified stays an lval_register.
I believe that this is a correct technical solution (and not just
papering over the problem), because what we expect of unwinders is to
tell us where a given register's value is saved. If the value is saved
in memory, -> lval_memory. If the value is saved in some other register
of the next frame, -> lval_register. If the value is not saved, it
doesn't really make sense to return an lval_register value. not_lval
would be more appropriate. If the code then wants to represent an
optimized out register value (like value_fetch_lazy_register does), then
it's a separate concern which shouldn't involve the unwinder.
This change breaks the output of "set debug frame 1" though (introduced
by the second commit), since that logging statement consumes the return
value of the unwinder directly. To keep the correct behavior, just make
`frame_unwind_register_value` call `val_print_not_saved` directly,
instead of `val_print_optimized_out`. This is fine because we know in
this context that we are always talking about a register value, and that
we want to show "not saved" for those.
I augmented the gdb.dwarf2/dw2-reg-undefined.exp test case to test some
cases I stumbled on while working on this, which I think are not tested
anywhere:
- the "set debug frame 1" debug output mentioned above. It's just debug
output, but if we want to make sure it doesn't change, it should be
tested
- printing not-saved register values from the history (should print not
saved)
- copying a not-saved register value in a convenience variable. In this
case, we expect that printing the convenience variable shows
"optimized out", because we copied the value, not the property of
where the value came from.
gdb/ChangeLog:
* frame-unwind.c (frame_unwind_got_optimized): Don't set
regnum/frame in value. Call allocate_value_lazy.
* frame.c (frame_unwind_register_value): Use
val_print_not_saved.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/dw2-reg-undefined.exp: Test "set debug frame 1"
output, printing a "not saved" value from history and printing a
convenience variable created from a "not saved" value.
Change-Id: If451739a3ef7a5b453b1f50707e21ce16d74807e
|
|
The NULL_TYPE macro is not very useful... remove it and just use
nullptr.
gdb/ChangeLog:
* gdbtypes.h (NULL_TYPE): Remove, change all uses to nullptr.
Change-Id: Ic6215921413dad5649192b012f1a41d0a650a644
|
|
When running test-case gdb.base/eh_return.exp with target board
unix/-fPIE/-pie, we run into:
...
(gdb) break *0x88e^M
Breakpoint 1 at 0x88e: file eh_return.c, line 54.^M
(gdb) PASS: gdb.base/eh_return.exp: setting breakpoint at *0x88e
run ^M
Starting program: eh_return ^M
Warning:^M
Cannot insert breakpoint 1.^M
Cannot access memory at address 0x88e^M
^M
(gdb) FAIL: gdb.base/eh_return.exp: hit breakpoint
...
The problem is that gdb does not support setting breakpoints on unrelocated
addresses.
Fix this by using nopie for the test-case.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-08-31 Tom de Vries <tdevries@suse.de>
* gdb.base/eh_return.exp: Use nopie.
|
|
By inspection, I noticed that print_program_space is calling
target_pid_to_str on the wrong target stack. Most targets print a
process pid the same way, so it isn't actually visible.
gdb/ChangeLog:
* progspace.c (print_program_space): Use all_inferiors. Switch to
the inferior before calling target_pid_to_str.
|
|
The tests in gdb.base/advance-until-multiple-locations.exp that expect
the program to stop at a caller fail on some systems, depending on
compiler. E.g., with Clang 10, I see:
advance ovld_func
0x00000000004011a3 in test () at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.base/advance-until-multiple-locations.cc:51
51 ovld_func ();
(gdb) FAIL: gdb.base/advance-until-multiple-locations.exp: advance_overload: second advance stops at caller
And Tom de Vries saw:
...
(gdb) until ovld_func^M
main () at advance-until-multiple-locations.cc:61^M
61 }^M
(gdb) FAIL: gdb.base/advance-until-multiple-locations.exp:until_overload: until ovld_func
...
Which exact line the program stops is not important. All we care
about here is that the program stopped at the caller function.
So fix it by adjusting the patterns to match the frame header/function
reported by the breakpoint hits instead of the source lines text.
Tested against:
- gcc {4.8, 4.9, 7.3.1, 9.3.0, trunk-20200828}
- clang {5.0.2, 10}
gdb/testsuite/ChangeLog:
* gdb.base/advance-until-multiple-locations.exp
(advance_overload, until_overload): Adjust to match the
frame/function header instead of the source line text.
|
|
While looking at psymtabs again, I noticed a couple of outdated
comments. These days, psymtabs can be destroyed, as they are no
longer obstack-allocated.
gdb/ChangeLog
2020-08-28 Tom Tromey <tom@tromey.com>
* xcoffread.c (xcoff_end_psymtab): Update comment.
* dbxread.c (dbx_end_psymtab): Update comment.
|
|
Consider test-case test.c:
...
$ cat test.c
int main (void) {
return 0;
L1:
(void)0;
}
...
Compiled with debug info:
...
$ gcc test.c -g
...
When attempting to set a breakpoint at L1, which is a label without address:
...
<1><f4>: Abbrev Number: 2 (DW_TAG_subprogram)
<f5> DW_AT_name : main
<2><115>: Abbrev Number: 3 (DW_TAG_label)
<116> DW_AT_name : L1
<119> DW_AT_decl_file : 1
<11a> DW_AT_decl_line : 5
<2><11b>: Abbrev Number: 0
...
we run into an internal-error:
...
$ gdb -batch a.out -ex "b main:L1"
linespec.c:3233: internal-error: void \
decode_line_full(const event_location*, int, program_space*, symtab*, \
int, linespec_result*, const char*, const char*): \
Assertion `result.size () == 1 || canonical->pre_expanded' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
...
Fix this by detecting the error condition in decode_line_full instead, and
throwing an error, such that we have instead:
...
(gdb) b main:L1
Location main:L1 not available
(gdb)
...
Unfortunately, to call event_location_to_string, which is used to get the
location name in the error message, we need to pass a non-const struct
event_location, because the call may cache the string in the struct (See
EL_STRING). So, we change the prototype of decode_line_full accordingly, and
everywhere this propages to.
Tested on x86_64-linux.
gdb/ChangeLog:
2020-08-28 Tom de Vries <tdevries@suse.de>
PR breakpoint/26544
* breakpoint.c (parse_breakpoint_sals): Remove const from struct
event_location.
(create_breakpoint): Same.
(base_breakpoint_decode_location): Same.
(bkpt_create_sals_from_location): Same.
(bkpt_decode_location): Same.
(bkpt_probe_create_sals_from_location): Same.
(bkpt_probe_decode_location): Same.
(tracepoint_create_sals_from_location): Same.
(tracepoint_decode_location): Same.
(tracepoint_probe_decode_location): Same.
(strace_marker_create_sals_from_location): Same.
(strace_marker_decode_location): Same.
(create_sals_from_location_default): Same.
(decode_location_default): Same.
* breakpoint.h (struct breakpoint_ops): Same.
(create_breakpoint): Same.
* linespec.h (decode_line_full): Same.
* linespec.c (decode_line_full): Same. Throw error if
result.size () == 0.
gdb/testsuite/ChangeLog:
2020-08-28 Tom de Vries <tdevries@suse.de>
* gdb.base/label-without-address.c: New test.
* gdb.base/label-without-address.exp: New file.
|
|
If you do "advance LINESPEC", and LINESPEC expands to more than one
location, GDB just errors out:
if (sals.size () != 1)
error (_("Couldn't get information on specified line."));
For example, advancing to a line in an inlined function, inlined three
times:
(gdb) b 21
Breakpoint 1 at 0x55555555516f: advance.cc:21. (3 locations)
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x000055555555516f in inline_func at advance.cc:21
1.2 y 0x000055555555517e in inline_func at advance.cc:21
1.3 y 0x000055555555518d in inline_func at advance.cc:21
(gdb) advance 21
Couldn't get information on specified line.
(gdb)
Similar issue with the "until" command, as it shares the
implementation with "advance".
Since, as the comment in gdb.base/advance.exp says, "advance <location>"
is really just syntactic sugar for "tbreak <location>;continue",
fix this by making GDB insert a breakpoint at all the resolved
locations.
A new testcase is included, which exercises both "advance" and
"until", in two different cases expanding to multiple locations:
- inlined functions
- C++ overloads
This also exercises the inline frames issue fixed by the previous
patch.
gdb/ChangeLog:
PR gdb/26524
* breakpoint.c (until_break_fsm) <location_breakpoint,
caller_breakpoint>: Delete fields.
<breakpoints>: New field.
<until_break_fsm>: Adjust to save a breakpoint vector instead of
two individual breakpoints.
(until_break_fsm::should_stop): Loop over breakpoints in the
breakpoint vector.
(until_break_fsm::clean_up): Adjust to clear the breakpoints
vector.
(until_break_command): Handle location expanding into multiple
sals.
gdb/testsuite/ChangeLog:
PR gdb/26523
PR gdb/26524
* gdb.base/advance-until-multiple-locations.cc: New.
* gdb.base/advance-until-multiple-locations.exp: New.
|
|
If you do "tbreak LINENO; c" to advance to an inlined function, GDB
presents the stop at the inline frame instead of at the non-artificial
stack frame:
(gdb) list 21
18 static inline __attribute__ ((always_inline)) int
19 inline_func (int i)
20 {
21 return i + 1;
22 }
(gdb) tbreak 21
Temporary breakpoint 3 at 0x55555555516f: advance.cc:21.
(gdb) c
Continuing.
Temporary breakpoint 3, inline_func (i=0) at advance.cc:21
21 return i + 1;
The logic for this is in stopped_by_user_bp_inline_frame:
/* Loop over the stop chain and determine if execution stopped in an
inlined frame because of a breakpoint with a user-specified
location set at FRAME_BLOCK. */
static bool
stopped_by_user_bp_inline_frame (const block *frame_block, bpstat stop_chain)
If however, you do "advance LINENO" or "until LINENO" instead, GDB
presents the stop at the non-artificial frame:
(gdb) advance 21
main () at advance.cc:43
43 i = inline_func (i);
(gdb)
"advance" and "until" should really behave like user breakpoints here,
since their location is also user-specified. As the comment in
gdb.base/advance.exp says, "advance <location>" is really just
syntactic sugar for "tbreak <location>; continue".
Fix this by making stopped_by_user_bp_inline_frame also consider
advance/until breakpoints.
A testcase covering this will be included in the next patch.
gdb/ChangeLog:
PR gdb/26523
* inline-frame.c (stopped_by_user_bp_inline_frame): Also consider
bp_until breakpoints user-specified locations. Update intro
comment.
|
|
Use multi_line to make the expected pattern more readable.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/dw2-reg-undefined.exp: Use multi_line.
Change-Id: Ia8e42d156c0c30265121eb890e1db17a692dbaf0
|
|
Change-Id: I423867477d4342673e629dac71a80592fd879ea1
|
|
Make the test names unique in gdb.arch/*.exp by either modifying the
test names or using with_test_prefix.
I have also fixed a typo 'forth' -> 'fourth' throughout gdb.arch/*.
Finally, I replaced code like this:
gdb_test "break [gdb_get_line_number "first breakpoint here"]" \
"Breakpoint .* at .*${srcfile}.*" \
"set first breakpoint in main"
With this:
gdb_breakpoint [gdb_get_line_number "first breakpoint here"]
In those files that I was already modifying for the other reasons
given above.
gdb/testsuite/ChangeLog:
* gdb.arch/amd64-byte.exp: Make test names unique, use
gdb_breakpoint, and fix typo 'forth' -> 'fourth'.
* gdb.arch/amd64-dword.exp: Likewise.
* gdb.arch/amd64-pseudo.c: Fix typo 'forth' -> 'fourth'.
* gdb.arch/amd64-stap-special-operands.exp: Make test names
unique.
* gdb.arch/amd64-tailcall-ret.exp: Likewise.
* gdb.arch/amd64-word.exp: Make test names unique, use
gdb_breakpoint, and fix typo 'forth' -> 'fourth'.
* gdb.arch/i386-byte.exp: Make test names unique, use
gdb_breakpoint.
* gdb.arch/i386-word.exp: Likewise.
|
|
I wanted to make a nicer / type-safe interface for
bfd_map_over_sections, avoiding the `void *` data parameter.
My first shot was to make a wrapper for bfd_map_over_sections,
gdb_bfd_map_over_sections that took a gdb::function_view.
However, I think that a range adapter gives nicer and simpler code, as a
simple for loop is easier to read than a callback / lambda function. So
here it is, it uses next_iterator and next_adapter, so it's not much
code.
As an example, I ported maintenance_info_sections and friends to use it.
The maint_print_section_data type could probably be removed now, but I
didn't want to do too much in one patch.
gdb/ChangeLog:
* gdb_bfd.h (gdb_bfd_section_iterator, gdb_bfd_section_range,
gdb_bfd_sections): New.
* maint.c (print_bfd_section_info): Change param type to
maint_print_section_data.
(print_objfile_section_info): Likewise.
(print_bfd_section_info_maybe_relocated): Likewise.
(maintenance_info_sections): Use gdb_bfd_sections.
Change-Id: Ib496f6b0a0eb7aadb10da1dd381304014d934ea0
|
|
This patch updates gdb/MAINTAINERS with ARC as a target and
myself as the maintainer. There is no mention of "-Werror"
because that is enabled by default for gdb/ targets now.
gdb/ChangeLog:
* MAINTAINERS: Add ARC target and maintainer.
|
|
ARC Linux targets differences from baremetal:
- No support for hardware single instruction stepping.
- Different access rules to registers.
- Use of another instruction for breakpoints.
v2: Changes after Tom's remarks [1]
arc-linux-tdep.c
- Use true/false instead of TRUE/FALSE.
- arc_linux_sw_breakpoint_from_kind (): Break long lines into two.
- arc_linux_sw_breakpoint_from_kind (): Remove starting blank line.
- Use explicit number evaluation, e.g: if (a & b) -> if ((a & b) != 0)
arc-tdep.c
- Use explicit number evaluation, e.g: if (a & b) -> if ((a & b) != 0)
gdb/configure.tgt
- arc*-*-linux*): Remove "build_gdbserver=yes".
v3: Changes after Simon's remarks [2]
arc-linux-tdep.c
- Use "return trap_size" instead of cryptic "return 2".
- Removed unnecessary curly braces.
- Removed "void" from "_initialize_arc_linux_tdep (void)".
v5: Changes after Simon's remarks [3]
- Remove unnecessary empty lines.
- Replace "breakpoint uses" with "breakpoints use" in a comment.
- "return condition;" i.s.o. "if (condition) return true; else return false;"
[1] Tom's remarks
https://sourceware.org/pipermail/gdb-patches/2020-April/167887.html
[2] Simon's remarks on v2
https://sourceware.org/pipermail/gdb-patches/2020-May/168513.html
[3] Simon's remarks on v4
https://sourceware.org/pipermail/gdb-patches/2020-August/170994.html
gdb/ChangeLog:
2020-08-25 Anton Kolesov <anton.kolesov@synopsys.com>
* configure.tgt: ARC support for GNU/Linux.
* Makefile.in (ALL_TARGET_OBJS): Likewise.
* arc-linux-tdep.c: New file.
* arc-tdep.h (ARC_STATUS32_L_MASK, ARC_STATUS32_DE_MASK): Declare.
* arc-tdep.c (arc_write_pc): Use it.
|
|
For ARC there are registers that are not part of a required set in XML
target descriptions by default, but are almost always present on ARC
targets and are universally exposed by the ptrace interface. Hardware
loop registers being one of them.
LP_START and LP_END auxiliary registers are hardware loop start and end.
Formally, they are optional, but it is hard to find an ARC configuration
that doesn't have them. They are always present in processors that can
run GNU/Linux. GDB needs to know about those registers to implement
proper software single stepping, since they affect what instruction
will be next.
This commit adds the code to check for the existance of "lp_start" and
"lp_end" in XML target descriptions. If they exist, then the function
reports that the target supports hardware loops.
gdb/ChangeLog:
* arc-tdep.c (arc_check_for_hardware_loop): New.
* arc-tdep.h (gdbarch_tdep): New field has_hw_loops.
gdb/doc/ChangeLog:
* gdb.texinfo (Synopsys ARC): Document LP_START, LP_END and BTA.
|
|
The "arc-tdep.h" makes use of definitions in "gdbarch.h", but it
does not include it explicitly. I have piggy backed this fix
in another commit [1], but I was asked to do it separately [2].
[1] arc: Add hardware loop detection
https://sourceware.org/pipermail/gdb-patches/2020-July/170800.html
[2] Simon's remarks to "arc: Add hardware loop detection"
https://sourceware.org/pipermail/gdb-patches/2020-August/170993.html
gdb/ChangeLog:
* arc-tdep.h: Include "gdbarch.h".
|
|
A few changes have been made to make the register support simpler,
more flexible and extendible. The trigger for most of these changes
are the remarks [1] made earlier for v2 of this patch. The noticeable
improvements are:
- The arc XML target features are placed under gdb/features/arc
- There are two cores (based on ISA) and one auxiliary feature:
v1-core: ARC600, ARC601, ARC700
v2-core: ARC EM, ARC HS
aux: common in both
- The XML target features represent a minimalistic sane set of
registers irrespective of application (baremetal or linux).
- A concept of "feature" class has been introduced in the code.
The "feature" object is constructed from BFD and GDBARCH data.
It contains necessary information (ISA and register size) to
determine which XML target feature to use.
- A new structure (ARC_REGISTER_FEATURE) is added that allows
providing index, names, and the necessity of registers. This
simplifies the sanity checks and future extendibility.
- Documnetation has been updated to reflect ARC features better.
- Although the feature names has changed, there still exists
backward compatibility with older names through
find_obsolete_[core,aux]_names() functions.
The last two points were inspired from RiscV port.
[1]
https://sourceware.org/pipermail/gdb-patches/2020-May/168511.html
gdb/ChangeLog:
* arch/arc.h
(arc_gdbarch_features): New class to stir the selection of target XML.
(arc_create_target_description): Use FEATURES to choose XML target.
(arc_lookup_target_description): Use arc_create_target_description
to create _new_ target descriptions or return the already created
ones if the FEATURES is the same.
* arch/arc.c: Implementation of prototypes described above.
* gdb/arc-tdep.h (arc_regnum enum): Add more registers.
(arc_gdbarch_features_init): Initialize the FEATURES struct.
* arc-tdep.c (*_feature_name): Make feature names consistent.
(arc_register_feature): A new struct to hold information about
registers of a particular target/feature.
(arc_check_tdesc_feature): Check if XML provides registers in
compliance with ARC_REGISTER_FEATURE structs.
(arc_update_acc_reg_names): Add aliases for r58 and r59.
(determine_*_reg_feature_set): Which feature name to look for.
(arc_gdbarch_features_init): Given MACH and ABFD, initialize FEATURES.
(mach_type_to_arc_isa): Convert from a set of binutils machine types
to expected ISA enums to be used in arc_gdbarch_features structs.
* features/Makefile (FEATURE_XMLFILES): Add new files.
* gdb/features/arc/v1-aux.c: New file.
* gdb/features/arc/v1-aux.xml: Likewise.
* gdb/features/arc/v1-core.c: Likewise.
* gdb/features/arc/v1-core.xml: Likewise.
* gdb/features/arc/v2-aux.c: Likewise.
* gdb/features/arc/v2-aux.xml: Likewise.
* gdb/features/arc/v2-core.c: Likewise.
* gdb/features/arc/v2-core.xml: Likewise.
* NEWS (Changes since GDB 9): Announce obsolence of old feature names.
gdb/doc/ChangeLog:
* gdb.texinfo (Synopsys ARC): Update the documentation for ARC
Features.
gdb/testsuite/ChangeLog:
* gdb.arch/arc-tdesc-cpu.xml: Use new feature names.
|
|
Commit 1eb8556f5a8b ("gdb: add infrun_debug_printf macro") changed the
debug output format for `set debug infrun 1`. The test
gdb.threads/stepi-random-signal.exp uses that debug output, and was
updated, but not correctly. It results in this failure:
FAIL: gdb.threads/stepi-random-signal.exp: stepi (no random signal)
Fix it by adjusting the pattern in the test.
gdb/testsuite/ChangeLog:
PR gdb/26532
* gdb.threads/stepi-random-signal.exp: Update pattern.
Change-Id: If5fa525e9545e32a286effe6a6184358374bd37c
|
|
Commit 1eb8556f5a8b ("gdb: add infrun_debug_printf macro") changed the
debug output format for `set debug infrun 1`. It broke test
gdb.base/ui-redirect.exp, which I missed:
FAIL: gdb.base/ui-redirect.exp: debugging: continue
Fix it by adjusting the pattern in the test to the new reality.
gdb/testsuite/ChangeLog:
PR gdb/26532
* gdb.base/ui-redirect.exp: Update pattern.
Change-Id: Ie8a8f6675e35a0cab55109b1534b44eb51baec9d
|
|
|
|
gdb.dwarf2/dw2-dir-file-name.exp fails to build using Clang because
the generated assembly language contains .ascii directives with more
than one string literal. gdb.dwarf2/dw2-restore.exp fails to build
using Clang because it contains .func and .endfunc directives.
This commit causes Clang to invoke the system assembler to assemble
the relevant files.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/dw2-dir-file-name.exp: Use system assembler
when compiling with clang.
* gdb.dwarf2/dw2-restore.exp: Likewise
|
|
gdb.cp/ambiguous.exp failed to build using clang with the following
error:
gdb compile failed, /gdbtest/src/gdb/testsuite/gdb.cp/ambiguous.cc:70:36:
warning: direct base 'A1' is inaccessible due to ambiguity:
class JVA1 -> class KV -> class A1
class JVA1 -> class A1 [-Winaccessible-base]
class JVA1 : public KV, public LV, public A1 {
^~~~~~~~~
This commit builds this testcase with -Wno-inaccessible-base when
using clang, to avoid this failure.
Furthermore, gdb.cp/ambiguous.exp has been disabled when using GCC
since 1998. This commit enables this testcase, building with
-Wno-inaccessible-base when using GCC >= 10.1, and -w otherwise.
gdb/testsuite/ChangeLog:
* gdb.cp/ambiguous.exp: Enable test when compiling with GCC.
Add additional_flags=-Wno-inaccessible-base when compiling
with GCC >= 10.1 or clang. Add additional_flags=-w when
compiling with GCC < 10.
|
|
Fix bug PR m2/26372, GDB's inability to parse multi-dimensional
modula-2 arrays.
We previously had two rules for handling the parsing of array
sub-scripts. I have reproduced them here with the actual handler
blocks removed to make the bug clearer:
exp : exp '[' non_empty_arglist ']'
;
exp : exp '[' exp ']'
;
non_empty_arglist
: exp
;
non_empty_arglist
: non_empty_arglist ',' exp
;
This is ambiguous as the pattern "exp '[' exp" could match either of
the 'exp' rules. Currently it just so happens that the parser picks
the second 'exp' rule which means we can only handle a single array
index.
As the handler code for the first 'exp' pattern will correctly handle
and number of array indexes then lets just remove the second pattern.
gdb/ChangeLog:
PR m2/26372
* m2-exp.y (exp): Improve comment for non_empty_arglist case, add
an assert. Remove single element array indexing pattern as the
MULTI_SUBSCRIPT support will handle this case too.
gdb/testsuite/ChangeLog:
PR m2/26372
* gdb.modula2/multidim.c: New file.
* gdb.modula2/multidim.exp: New file.
|
|
This reverts commit 07758bdfa9e5a762f2ec0deeb51b11d6ad5fe376.
|
|
Here is a bugfix for Pr 26372 [Modula-2] Parsing of multi-subscript arrays.
Also included is a dejagnu testcase. No extra regressions are caused on
Debian GNU/Linux Buster amd64.
gdb/ChangeLog:
2020-08-25 Gaius Mulley <gaiusmod2@gmail.com>
PR m2/26372
* m2-exp.y: Rewrite array subscript rules to support multidimension
array access. (ArgumentList) replaces non_empty_arglist.
gdb/testsuite/ChangeLog:
2020-08-25 Gaius Mulley <gaiusmod2@gmail.com>
PR m2/26372
* testsuite/gdb.modula2/multidim.exp: New file.
* testsuite/gdb.modula2/multidim.c: New file.
|
|
The implementation is in valprint.c, so the declaration belongs in
valprint.h.
gdb/ChangeLog:
* value.h (valprint_check_validity): Move declaration from
here...
* valprint.h (valprint_check_validity): ... to here.
Change-Id: Ibe577d3696720099e6d79888d4ee8e3c1bf05a26
|
|
I noticed that when a test uses `runto_main` and a GDB internal error
happens while running to main, no error or fail is emitted. This is
because `runto_main` uses the `no-message` option of `runto`.
As a result, if a test fails to run to main and exits, no sign that
something went wrong is emitted. For example, add this always-false
assertion to compute_frame_id:
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -545,6 +545,7 @@ static void
compute_frame_id (struct frame_info *fi)
{
gdb_assert (!fi->this_id.p);
+ gdb_assert (false);
if (frame_debug)
fprintf_unfiltered (gdb_stdlog, "{ compute_frame_id (fi=%d) ",
... and run gdb.dwarf2/dw2-align.exp. No fail or sign that something
went wrong is shown. It just appears as if the test gets skipped.
A developer introducing such a regression in this test today would
likely notice it, because we are used to diff-ing test results. So we
would see some PASSes dispappear for no good reason and look into it.
But I find it worrysome for two reasons:
1. Scripts that analyze regressions (such as the one on the buildbot)
may only look for new FAILs or new ERRORs. It would probably miss
this.
2. Imagine that we one day have a testsuite that runs cleanly (some
people might already run subsets of the testsuite and expect it to
all pass), we would just run the testsuite and check that there are
no fails. It would be easy to miss something like this.
In case of internal error, I suggest making `runto` emit a FAIL even if
`no-message` was passed. This is different from other failure modes
that might be expected (whchi rightfully cause the test to simply be
skipped). An internal error is always bad, so if it happens it should
noisily fail.
gdb/testsuite/ChangeLog:
* lib/gdb.exp (runto): Always emit fail on internal error.
Change-Id: I6e6faed4868ea821541a23042b2d01c30058b0d3
|
|
To help ensure that all debug statements have the same format, introduce
the debug_prefixed_vprintf helper. Implement linux_nat_debug_printf_1
and infrun_debug_printf_1 with it.
I would eventually like to style the module and function name with some
color, to help them stick out, but I don't really know how to do that
yet, it can always be done later.
gdb/ChangeLog:
* debug.h: New file.
* debug.c (debug_prefixed_vprintf): New function.
* infrun.c (infrun_debug_printf_1): Use debug_prefixed_vprintf.
* linux-nat.c (linux_nat_debug_printf_1): Likewise.
Change-Id: Iccc290a2dc6b5fffcbe1c2866ed8d804ad380764
|
|
Introduce this macro to print debug statements in the infrun.c file,
same idea as what was done in 9327494e0eeb ("gdb: add
linux_nat_debug_printf macro").
Although in this case, there are places outside infrun.c that print
debug statements if debug_infrun is set. So the macro has to be
declared in the header file, so that it can be used in these other
files.
Note one special case. In stop_all_threads, I've used an explicit
if (debug_infrun)
infrun_debug_printf_1 ("stop_all_threads", "done");
for the message in the SCOPE_EXIT. Otherwise, the message appears like
this:
[infrun] operator(): done
Until we find a better solution for extracting a meaningful function
name for lambda functions, I think it's fine to handle these special
cases manually, they are quite rare.
Some tests need to be updated, because they rely on some infrun debug
statements.
gdb/ChangeLog:
* infrun.h (infrun_debug_printf_1): New function declaration.
(infrun_debug_printf): New macro.
* infrun.c (infrun_debug_printf_1): Use infrun_debug_printf
throughout.
(infrun_debug_printf): New function.
* breakpoint.c (should_be_inserted): Use infrun_debug_printf.
(handle_jit_event): Likewise.
gdb/testsuite/ChangeLog:
* gdb.base/gdb-sigterm.exp (do_test): Update expected regexp.
* gdb.threads/signal-while-stepping-over-bp-other-thread.exp:
Likewise.
* gdb.threads/stepi-random-signal.exp: Likewise.
Change-Id: I66433c8a9caa64c8525ab57c593022b9d1956d5c
|
|
Building with a really old flex and a really new g++ is probably not
recommended, but it should not cause compile errors.
gdb/ChangeLog:
* ada-lex.l: Extend register warnings diagnostics comment for g++.
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER): Also define
for GCC version 7.0 or higher.
|
|
I'm dealing these days with a class of bugs that involve trying to get a
certain frame's id while we are in the process of computing it. In other
words, compute_frame_id being called for a frame, eventually calling
get_frame_id for that same frame. I don't think this is ever supposed to
happen, as that creates a cyclic dependency.
Usually, these problems cause some failure down the line. I'm proposing with
this patch to catch them as early as possible, as soon as the situation
described above happens. I think that helps because the failed assertion will
be closer to the root of the problem.
To do so, the patch changes the frame_info::this_id::p flag from a boolean (is
the frame id computed or not) to a tri-state:
- the frame id is not computed
- the frame id is being computed
- the frame id is computed
Then, we can properly assert that get_frame_id doesn't get called for a frame
whose id is being computed.
To illustrate how that can help, let's imagine we apply the following change to
frame_unwind_got_optimized:
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -260,8 +260,7 @@ frame_unwind_got_optimized (struct frame_info *frame, int regnum)
mark_value_bytes_optimized_out (val, 0, TYPE_LENGTH (type));
VALUE_LVAL (val) = lval_register;
VALUE_REGNUM (val) = regnum;
- VALUE_NEXT_FRAME_ID (val)
- = get_frame_id (get_next_frame_sentinel_okay (frame));
+ VALUE_NEXT_FRAME_ID (val) = get_frame_id (frame);
return val;
}
... and run the following command, which leads to a failed assertion (you need
to run the corresponding test to generate the binary first):
$ ./gdb -q -nx testsuite/outputs/gdb.dwarf2/dw2-undefined-ret-addr/dw2-undefined-ret-addr -ex "b stop_frame" -ex r
Without this patch applied, we catch the issue indirectly, when the top-level
get_frame_id tries to stash the frame:
/home/smarchi/src/binutils-gdb/gdb/frame.c:593: internal-error: frame_id get_frame_id(frame_info*): Assertion `stashed' failed.
...
#9 0x0000000001af1c3a in internal_error (file=0x1cea060 "/home/smarchi/src/binutils-gdb/gdb/frame.c", line=593, fmt=0x1ce9f80 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55
#10 0x0000000000e9b413 in get_frame_id (fi=0x6210005105e0) at /home/smarchi/src/binutils-gdb/gdb/frame.c:593
#11 0x0000000000e99e35 in scoped_restore_selected_frame::scoped_restore_selected_frame (this=0x7fff1d8b9760) at /home/smarchi/src/binutils-gdb/gdb/frame.c:308
#12 0x000000000149a261 in print_frame_args (fp_opts=..., func=0x6210000dd7d0, frame=0x6210005105e0, num=-1, stream=0x60300008a580) at /home/smarchi/src/binutils-gdb/gdb/stack.c:750
#13 0x000000000149d938 in print_frame (fp_opts=..., frame=0x6210005105e0, print_level=0, print_what=SRC_AND_LOC, print_args=1, sal=...) at /home/smarchi/src/binutils-gdb/gdb/stack.c:1394
#14 0x000000000149c0c8 in print_frame_info (fp_opts=..., frame=0x6210005105e0, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/smarchi/src/binutils-gdb/gdb/stack.c:1119
#15 0x0000000001498100 in print_stack_frame (frame=0x6210005105e0, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1) at /home/smarchi/src/binutils-gdb/gdb/stack.c:366
#16 0x00000000010234b7 in print_stop_location (ws=0x7fff1d8ba1f0) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:8366
#17 0x000000000102362d in print_stop_event (uiout=0x607000018660, displays=true) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:8382
...
It freaks out because the frame is already in the stash: it was added by an
inner call to get_frame_id, called indirectly by compute_frame_id. Debugging
this failure is difficult because we have to backtrack to where this happened.
With the patch applied, we catch the issue earlier, here:
/home/smarchi/src/binutils-gdb/gdb/frame.c:601: internal-error: frame_id get_frame_id(frame_info*): Assertion `fi->this_id.p != frame_id_status::COMPUTING' failed
...
#9 0x0000000001af22bc in internal_error (file=0x1cea6e0 "/home/smarchi/src/binutils-gdb/gdb/frame.c", line=601, fmt=0x1cea600 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55
#10 0x0000000000e9b7e3 in get_frame_id (fi=0x62100050dde0) at /home/smarchi/src/binutils-gdb/gdb/frame.c:601
#11 0x0000000000e989b3 in frame_unwind_got_optimized (frame=0x62100050dde0, regnum=16) at /home/smarchi/src/binutils-gdb/gdb/frame-unwind.c:264
#12 0x0000000000cbe386 in dwarf2_frame_prev_register (this_frame=0x62100050dde0, this_cache=0x62100050ddf8, regnum=16) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/frame.c:1267
#13 0x0000000000e9f569 in frame_unwind_register_value (next_frame=0x62100050dde0, regnum=16) at /home/smarchi/src/binutils-gdb/gdb/frame.c:1266
#14 0x0000000000e9eaab in frame_register_unwind (next_frame=0x62100050dde0, regnum=16, optimizedp=0x7ffca814ade0, unavailablep=0x7ffca814adf0, lvalp=0x7ffca814ae10, addrp=0x7ffca814ae20, realnump=0x7ffca814ae00, bufferp=0x7ffca814aec0 "") at /home/smarchi/src/binutils-gdb/gdb/frame.c:1169
#15 0x0000000000e9f233 in frame_unwind_register (next_frame=0x62100050dde0, regnum=16, buf=0x7ffca814aec0 "") at /home/smarchi/src/binutils-gdb/gdb/frame.c:1225
#16 0x0000000000f84262 in i386_unwind_pc (gdbarch=0x6210000eed10, next_frame=0x62100050dde0) at /home/smarchi/src/binutils-gdb/gdb/i386-tdep.c:1969
#17 0x0000000000ec95dd in gdbarch_unwind_pc (gdbarch=0x6210000eed10, next_frame=0x62100050dde0) at /home/smarchi/src/binutils-gdb/gdb/gdbarch.c:3062
#18 0x0000000000cb5e9d in dwarf2_tailcall_sniffer_first (this_frame=0x62100050dde0, tailcall_cachep=0x62100050dee0, entry_cfa_sp_offsetp=0x7ffca814b160) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:387
#19 0x0000000000cbdd38 in dwarf2_frame_cache (this_frame=0x62100050dde0, this_cache=0x62100050ddf8) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/frame.c:1198
#20 0x0000000000cbe026 in dwarf2_frame_this_id (this_frame=0x62100050dde0, this_cache=0x62100050ddf8, this_id=0x62100050de40) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/frame.c:1226
#21 0x0000000000e9b447 in compute_frame_id (fi=0x62100050dde0) at /home/smarchi/src/binutils-gdb/gdb/frame.c:580
#22 0x0000000000e9b89e in get_frame_id (fi=0x62100050dde0) at /home/smarchi/src/binutils-gdb/gdb/frame.c:613
#23 0x0000000000e99e35 in scoped_restore_selected_frame::scoped_restore_selected_frame (this=0x7ffca814b610) at /home/smarchi/src/binutils-gdb/gdb/frame.c:315
#24 0x000000000149a8e3 in print_frame_args (fp_opts=..., func=0x6210000dd7d0, frame=0x62100050dde0, num=-1, stream=0x60300008a520) at /home/smarchi/src/binutils-gdb/gdb/stack.c:750
#25 0x000000000149dfba in print_frame (fp_opts=..., frame=0x62100050dde0, print_level=0, print_what=SRC_AND_LOC, print_args=1, sal=...) at /home/smarchi/src/binutils-gdb/gdb/stack.c:1394
#26 0x000000000149c74a in print_frame_info (fp_opts=..., frame=0x62100050dde0, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/smarchi/src/binutils-gdb/gdb/stack.c:1119
#27 0x0000000001498782 in print_stack_frame (frame=0x62100050dde0, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1) at /home/smarchi/src/binutils-gdb/gdb/stack.c:366
#28 0x0000000001023b39 in print_stop_location (ws=0x7ffca814c0a0) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:8366
#29 0x0000000001023caf in print_stop_event (uiout=0x607000018660, displays=true) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:8382
...
Now, we can clearly see that get_frame_id for frame `fi=0x62100050dde0` gets
called while compute_frame_id is active for that frame. The backtrace is more
helpful to identify the root of the problem.
gdb/ChangeLog:
* frame.c (enum class frame_id_status): New.
(struct frame_info) <this_id::p>: Change type to frame_id_status.
(fprintf_frame): Update.
(compute_frame_id): Set frame id status to "computing" on entry.
Set it back to "not_computed" on failure and to "computed" on
success.
(get_frame_id): Assert the frame id is not being computed.
(create_sentinel_frame): Use frame_id_status::COMPUTED.
(create_new_frame): Likewise.
(frame_cleanup_after_sniffer): Update assert.
Change-Id: I5f1a25fafe045f756bd75f358892720b30ed20c9
|
|
As reported by Tom here [1], commit 888bdb2b7445 ("gdb: change regcache
list to be a map") overlooked an important case, causing a regression.
When registers_changed_ptid is called with a pid-like ptid, it used to
clear all the regcaches for that pid. That commit accidentally removed
that behavior. We need to handle the `ptid.is_pid ()` case in
registers_changed_ptid.
The most trivial way of fixing it would be to iterate on all ptids of a
target and delete the entries where the ptid match the pid. But the
point of that commit was to avoid having to iterate on ptids to
invalidate regcaches, so that would feel like a step backwards.
The only logical solution I see is to add yet another map level, so that
we now have:
target -> (pid -> (ptid -> regcaches))
This patch implements that and adds a test for the case of calling
registers_changed_ptid with a pid-like ptid.
[1] https://sourceware.org/pipermail/gdb-patches/2020-August/171222.html
gdb/ChangeLog:
* regcache.c (pid_ptid_regcache_map): New type.
(target_ptid_regcache_map): Remove.
(target_pid_ptid_regcache_map): New type.
(regcaches): Change type to target_pid_ptid_regcache_map.
(get_thread_arch_aspace_regcache): Update.
(regcache_thread_ptid_changed): Update, handle pid-like ptid
case.
(regcaches_size): Update.
(regcache_count): Update.
(registers_changed_ptid_target_pid_test): New.
(_initialize_regcache): Register new test.
Change-Id: I4c46e26d8225c177dbac9488b549eff4c68fa0d8
|
|
The selftest `regcaches` selftest is a bit too broad for my taste,
testing the behavior of get_thread_arch_aspace_regcache and various
cases of registers_changed_ptid. Since I'll want to test even more
scenarios of registers_changed_ptid, passing different sets of
parameters, it will be difficult to do in a single test case. It is
difficult to change something at some point in the test case while make
sure it doesn't compromise what comes after, that we still test the
scenarios that we intended to test. So, split the test case in multiple
smaller ones.
- Split the test case in multiple, where each test case starts from
scratch and tests one specific scenario.
- Introduce the populate_regcaches_for_test function, which is used by
the various test cases to start with a regcache container populated
with a few regcaches for two different targets.
- populate_regcaches_for_test returns a regcache_test_data object, which
contains the test targets that were used to create the regcaches. It
also takes care to call registers_changed at the beginning and end of
the test to ensure the test isn't influenced by existing regcaches,
and cleans up after itself.
- Move the regcache_count lambda function out of
regcache_thread_ptid_changed, so it can be used in
other tests.
- For get_thread_arch_aspace_regcache, test that getting a regcache that
already exists does not increase the count of existing regcaches.
- For registers_changed_ptid, test the three cases we handle today:
(nullptr, minus_one_ptid), (target, minus_one_ptid) and (target,
ptid). The (target, minus_one_ptid) case was not tested prior to this
patch.
gdb/ChangeLog:
* regcache.c (regcache_count): New.
(struct regcache_test_data): New.
(regcache_test_data_up): New.
(populate_regcaches_for_test): New.
(regcaches_test): Remove.
(get_thread_arch_aspace_regcache_test): New.
(registers_changed_ptid_all_test): New.
(registers_changed_ptid_target_test): New.
(registers_changed_ptid_target_ptid_test): New.
(regcache_thread_ptid_changed): Remove regcache_count lambda.
(_initialize_regcache): Register new tests.
Change-Id: Id4280879fb40ff3aeae49b01b95359e1359c3d4b
|
|
Do these misc changes to test_get_thread_arch_aspace_regcache:
- Rename to get_thread_arch_aspace_regcache_and_check. The following
patch introduces a selftest for get_thread_arch_aspace_regcache, named
get_thread_arch_aspace_regcache_test. To avoid confusion between the
two functions, rename this one to
get_thread_arch_aspace_regcache_and_check, I think it describes better
what it does.
- Remove gdbarch parameter. We always pass the same gdbarch (the
current inferior's gdbarch), so having a parameter is not useful. It
would be interesting to actually test with multiple gdbarches, to
verify that the regcache container can hold multiple regcaches (with
different architectures) for a same (target, ptid). But it's not the
case as of this patch.
- Verify that the regcache's arch is correctly set.
- Remove the aspace parameter. We always pass NULL here, so it's not
useful to have it as a parameter. Also, instead of passing a NULL
aspace to get_thread_arch_aspace_regcache and verifying that we get a
NULL aspace back, pass the current inferior's aspace (just like we use
the current inferior's gdbarch).
gdb/ChangeLog:
* regcache.c (test_get_thread_arch_aspace_regcache): Rename to...
(get_thread_arch_aspace_regcache_and_check): ... this. Remove
gdbarch and aspace parameter. Use current inferior's aspace.
Validate regcache's arch value.
(regcaches_test): Update.
Change-Id: I8b4c2303b4f91f062269043d1f7abe1650232010
|
