From: Simon Marchi <simon.marchi@polymtl.ca>
To: Luis Machado <luis.machado@arm.com>,
Tom de Vries <tdevries@suse.de>,
gdb-patches@sourceware.org
Subject: Re: [PATCH v2 11/13] gdb: make user-created frames reinflatable
Date: Tue, 24 Jan 2023 22:45:00 -0500 [thread overview]
Message-ID: <f1bfbc78-bccc-f0e3-2e13-06857785223d@polymtl.ca> (raw)
In-Reply-To: <0fdfcde0-f51f-6f59-82c3-1089d2d016f0@arm.com>
On 1/24/23 03:22, Luis Machado wrote:
> On 1/24/23 03:55, Simon Marchi wrote:
>>
>>
>> On 1/23/23 09:34, Luis Machado wrote:
>>> On 1/23/23 12:57, Tom de Vries wrote:
>>>> On 12/14/22 04:34, Simon Marchi via Gdb-patches wrote:
>>>>> gdb/testsuite/gdb.base/frame-view.exp | 47 ++++++++++++--
>>>>
>>>> Hi,
>>>>
>>>> on aarch64-linux I get:
>>>> ...
>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
>>>> FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again (GDB internal error)
>>>
>>>
>>> I can reproduce this as well. It also happens on arm-linux Ubuntu 22.04/20.04, with a similar kind of backtrace.
>>
>> I can reproduce too, I have a potential fix. I'll try to send it
>> tomorrow.
>
> Let me know if I can help with testing it on one of the targets.
Ok, so I have a patch that fixes the test, but I'm not sure it's really
good.
First, here's my analysis of what I think is happening.
- When we create the user frame (the "select-frame view" command), we
create a sentinel frame just for our user-created frame, in
create_new_frame. This sentinel frame has the same id as the regular
sentinel frame.
- When printing the frame, after doing the "select-frame view"
command, the argument's pretty printer is invoked, which does an
inferior function call (this is the point of the test). This clears
the frame cache, including the "real" sentinel frame, which sets the
sentinel_frame global to nullptr.
- Later in the frame-printing process (when printing the second
argument), the auto-reinflation mechanism (which would have been the
manual reinflate call before, doesn't matter here) re-creates the
user frame by calling create_new_frame again, creating its own
special sentinel frame again. However, note that the "real" sentinel
frame, the sentinel_frame global, is still nullptr. If the selected
frame had been a regular frame, we would have called
get_current_frame at some point during the reinflation, which would
have re-created the "real" sentinel frame. But it's not the case
here.
- Deep down the stack, something wants to fill in the unwind stop
reason for frame 0, which requires trying to unwind frame 1. This
leads us to trying to unwind the PC of frame 1:
#0 gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955
#1 0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450)
at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390
#2 0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089
#3 0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101
#4 0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281
#5 0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376
#6 0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051
#7 0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356
#8 0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002")
at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110
#9 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
#10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811
#11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
#12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0,
type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078
#13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980,
subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513
#14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true)
at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557
#15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052
- AArch64 defines a special "prev register" function,
aarch64_dwarf2_prev_register, to handle unwinding the PC. This
function does
frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM);
(I must admit that I don't really understand this part, it seems to
me like you would want to unwind LR's value from this_frame->next...
but anyway.)
- frame_unwind_register_unsigned ultimately creates a lazy register
value, saving the frame id of this_frame->next. this_frame is the
user-created frame, to this_frame->next is the special sentinel frame
we created for it.
- When time comes to un-lazify the value, value_fetch_lazy_register
calls frame_find_by_id, to find the frame with the id we saved.
- frame_find_by_id sees it's the sentinel frame id, so returns the
sentinel_frame global, which is, if you remember, nullptr.
- We hit the `gdb_assert (next_frame != NULL)` assertion.
I have a hard time pinpointing a single thing that is wrong. The fact
that the user-created frames have their own sentinel frame, which share
the same id as the real sentinel frame, that sounds like it can't work
with frame_find_by_id. In the case without the pretty printer, where
things appear to work, it means that frame_find_by_id returns the "real"
sentinel frame, part of the "real" stack frame chain, and not the
sentinel that is attached to the user-created frame. Things still kinda
work in the end because those two sentinel frames behave the same, as
far as "prev register" is concerned.
Ultimately, I don't really understand how this "frame view" feature is
useful. Sure, we pretend that the frame's sp and pc is what the user
gave, but what about the other registers? It seems to me like you are
very likely to get bogus values.
The patch I had in mind was to give user-created frames a special
unwinder, rather than letting other unwinders (the arch-specific ones,
the dwarf2 one, etc) kick in. That unwinder would always return the
unwind stop reason UNWIND_OUTERMOST, which effectively makes the
user-created frame an outer frame, and we would never try to unwind any
register value from it. It makes the test pass, because it breaks the
chain of event explained above. But it's probably just papering over
the actual problem.
Simon
next prev parent reply other threads:[~2023-01-25 3:45 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-14 3:34 [PATCH v2 00/13] Make frame_info_ptr automatic Simon Marchi
2022-12-14 3:34 ` [PATCH v2 01/13] gdb: move type_map_instance to compile/compile.c Simon Marchi
2022-12-14 3:34 ` [PATCH v2 02/13] gdb: move compile_instance to compile/compile.h Simon Marchi
2022-12-14 3:34 ` [PATCH v2 03/13] gdb: remove language.h include from frame.h Simon Marchi
2022-12-14 3:34 ` [PATCH v2 04/13] gdb: move sect_offset and cu_offset to dwarf2/types.h Simon Marchi
2022-12-14 3:34 ` [PATCH v2 05/13] gdb: move call site types to call-site.h Simon Marchi
2022-12-14 3:34 ` [PATCH v2 06/13] gdb: move frame_info_ptr to frame.{c,h} Simon Marchi
2022-12-20 17:01 ` Bruno Larsen
2023-01-03 18:59 ` Simon Marchi
2022-12-14 3:34 ` [PATCH v2 07/13] gdb: add frame_id::user_created_p Simon Marchi
2022-12-14 3:34 ` [PATCH v2 08/13] gdb: add user-created frames to stash Simon Marchi
2022-12-14 3:34 ` [PATCH v2 09/13] gdb: add create_new_frame(frame_id) overload Simon Marchi
2022-12-14 3:34 ` [PATCH v2 10/13] gdb: make it possible to restore selected user-created frames Simon Marchi
2022-12-14 3:34 ` [PATCH v2 11/13] gdb: make user-created frames reinflatable Simon Marchi
2023-01-23 12:57 ` Tom de Vries
2023-01-23 14:34 ` Luis Machado
2023-01-24 3:55 ` Simon Marchi
2023-01-24 8:22 ` Luis Machado
2023-01-25 3:45 ` Simon Marchi [this message]
2023-01-30 8:49 ` Luis Machado
2023-01-30 16:20 ` Simon Marchi
2022-12-14 3:34 ` [PATCH v2 12/13] gdb: make frame_info_ptr grab frame level and id on construction Simon Marchi
2022-12-14 3:34 ` [PATCH v2 13/13] gdb: make frame_info_ptr auto-reinflatable Simon Marchi
2022-12-20 16:57 ` [PATCH v2 00/13] Make frame_info_ptr automatic Bruno Larsen
2023-01-03 19:00 ` Simon Marchi
2023-01-03 19:09 ` Simon Marchi
2023-01-18 18:10 ` Tom Tromey
2023-01-19 3:40 ` Simon Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f1bfbc78-bccc-f0e3-2e13-06857785223d@polymtl.ca \
--to=simon.marchi@polymtl.ca \
--cc=gdb-patches@sourceware.org \
--cc=luis.machado@arm.com \
--cc=tdevries@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).