From: Tomas Vanek <vanekt@fbl.cz>
To: Simon Marchi <simark@simark.ca>, Tomas Vanek <vanekt@fbl.cz>,
gdb-patches@sourceware.org, Luis Machado <luis.machado@arm.com>
Subject: Re: [PATCH v2] gdb: Modify until_break_command to act correctly on SIGTRAMP_FRAME
Date: Wed, 11 Jan 2023 00:22:50 +0100 [thread overview]
Message-ID: <7a8c358c-7de3-8390-2566-cee03278184e@fbl.cz> (raw)
In-Reply-To: <dd7a85c5-a1ff-f687-f44e-640c8191d3c4@simark.ca>
On 10/01/2023 18:48, Simon Marchi wrote:
>
> On 10/21/22 07:58, Tomas Vanek wrote:
>> This patch partially depends on
>> gdb/arm: Terminate frame unwinding in M-profile lockup state
>> (without it lockup state is unwound as if it were a normal
>> stack frame).
>>
>> The commands 'advance' and 'until' try to set a breakpoint
>> on the bogus return address derived from Arm M-profile magic
>> address (actually EXC_RETURN or a PC value indicating lockup).
>>
>> The offending breakpoint should be set at the return address in
>> the caller. The magic value 0xffffffff in LR indicates
>> there is no caller (return to this address would lock up the CPU).
>>
>> Similar behaviour of 'advance' and 'until' is observed in
>> an exception handler routine. In this case LR contains e.g.
>> 0xfffffff1 (EXC_RETURN) and GDB tries to set a breakpoint at
>> 0xfffffff0. It should use a return value stacked by the exception
>> instead.
>>
>> Testbench setup:
>> STM32G474, a Cortex-M4 device. Any Cortex-M device can be used.
>> A test application (an ordinary blink) with a standard startup
>> is loaded to the device flash.
>>
>> Steps to reproduce the problem:
>>
>> start GDB server
>> $ openocd -f interface/cmsis-dap.cfg -f target/stm32g4x.cfg
>>
>> start GDB in second terminal
>> $ arm-none-eabi-gdb blink.elf
>>
>> (gdb) target extended-remote localhost:3333
>>
>> Reset the device and halt it:
>> (gdb) monitor reset halt
>> target halted due to debug-request, current mode: Thread
>> xPSR: 0x01000000 pc: 0x08000e14 msp: 0x20020000
>>
>> Step by one instruction to re-read GDB register cache:
>> (gdb) stepi
>>
>> Check registers, LR should be 0xffffffff after reset:
>> (gdb) info registers
>> ...
>> sp 0x20020000 0x20020000
>> lr 0xffffffff -1
>> pc 0x8000e16 0x8000e16
>> xPSR 0x1000000 16777216
>> ...
>>
>> (gdb) set debug remote
>>
>> Issue 'advance' command:
>> (gdb) advance main
>> [remote] Sending packet: $mfffffffe,2#fa
>> [remote] Packet received: 0000
>> [remote] Sending packet: $mfffffffe,2#fa
>> [remote] Packet received: 0000
>> [remote] Sending packet: $m8000526,2#30
>> [remote] Packet received: 2046
>> [remote] Sending packet: $Z1,8000526,2#7a
>> [remote] Packet received: OK
>> [remote] packet_ok: Packet Z1 (hardware-breakpoint) is supported
>> [remote] Sending packet: $Z0,fffffffe,2#43
>> [remote] Packet received: E0E
>> [remote] packet_ok: Packet Z0 (software-breakpoint) is supported
>> Warning:
>> Cannot insert breakpoint 0.
>> Cannot access memory at address 0xfffffffe
>>
>> Command aborted.
>> (gdb)
>>
>> Relevant messages from OpenOCD:
>> Error: Failed to read memory at 0xfffff000
>> Error: can't add breakpoint: unknown reason
>>
>> This patch adds skipping over frames that are not suitable for
>> guarding with a breakpoint inspired by 'finish' command.
>> If no suitable frame is found, a momentary breakpoint is not set.
>>
>> v2: Comment fixes, bug reference.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28683
>> Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
>> ---
>> gdb/breakpoint.c | 22 +++++++++++++++++-----
>> 1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> index f6591d4..bb85342 100644
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -10467,6 +10467,7 @@ enum async_reply_reason
>> until_break_command (const char *arg, int from_tty, int anywhere)
>> {
>> frame_info_ptr frame;
>> + frame_info_ptr caller_frame;
>> struct gdbarch *frame_gdbarch;
>> struct frame_id stack_frame_id;
>> struct frame_id caller_frame_id;
>> @@ -10505,7 +10506,17 @@ enum async_reply_reason
>> frame = get_selected_frame (NULL);
>> frame_gdbarch = get_frame_arch (frame);
>> stack_frame_id = get_stack_frame_id (frame);
>> - caller_frame_id = frame_unwind_caller_id (frame);
>> +
>> + caller_frame = get_prev_frame_always (frame);
>> +
>> + while (caller_frame)
>> + {
>> + if (get_frame_type (caller_frame) != TAILCALL_FRAME
>> + && gdbarch_code_of_frame_writable (get_frame_arch (caller_frame), caller_frame))
>> + break;
>> +
>> + caller_frame = get_prev_frame_always (caller_frame);
>> + }
> frame_unwind_caller_id does skip inline frames (through
> skip_artificial_frames), whereas your version does not, is that correct?
TBH not sure.
I just reused the code in 'finish' command
https://gitlab.com/gnutools/binutils-gdb/-/blob/8ec0b0b5df0ebe28c32900afc7ae8ff22b21f381/gdb/infcmd.c#L1790-1808
because it does not suffer from breakpoints at magic addresses and
changed it to prevent stopping at main()
as explained in
https://sourceware.org/pipermail/gdb-patches/2022-October/193223.html
>
> I'm wondering if we should make frame_unwind_caller_id /
> skip_artificial_frames handle it instead. In other words, would other
> callers of frame_unwind_caller_id / skip_artificial_frames need to skip
> such frames, and benefit from it.
>
> If we do that, frame_unwind_caller_id would return null_frame_id in your
> case, and I think that until_break_command would not need to be
> modified.
>
> Looking at watch_command_1, for instance, there seems to be the same
> situation, where we create a temporary breakpoint to detect when we get
> out of scope:
>
> https://gitlab.com/gnutools/binutils-gdb/-/blob/8ec0b0b5df0ebe28c32900afc7ae8ff22b21f381/gdb/breakpoint.c#L10215
>
> I guess if you try to create a watchpoint in that special frame, you'll
> hit the same problem?
Indeed.
I uploaded a slightly modified example
https://github.com/libopencm3/libopencm3-examples/blob/master/examples/stm32/f4/stm32f4-discovery/tick_blink/tick_blink.c
to the original bug ticket
https://sourceware.org/bugzilla/show_bug.cgi?id=28683
It runs in QEMU and has a local variable in the ISR routine.
Setting 'watch loc' while stopped at tick_blink.c:39 and 'continue'
results in
[remote] Sending packet: $Z0,fffffff9,2#17
as you expected.
>
> From what I understand, in the ARM case, the sigtramp frame does not
> represent any code, and has no real PC. When the normal frame #0
> returns, control goes back directly to the frame above the sigtramp
> frame. It does not go execute instructions in the sigtramp frame. Is
> that right?
Yes, exactly.
>
> With Linux on amd64, after returning from the normal frame, control goes
> to some instructions that will call the sigreturn syscall, so the
> sigtramp frame does have a real pc.
Didn't know that.
At least the current 'finish' command implementation skips that part.
Isn't it intentional?
>
> If so, I think that's the distinction we are dealing with here. We want
> to know what's the first instruction that is going to be executed after
> leaving the current frame. And it may sometimes be the sigtramp frame,
> or it may sometimes be the frame above that.
No idea how to handle that.
Thanks for your comments.
Tomas
next prev parent reply other threads:[~2023-01-10 23:22 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-21 11:58 Tomas Vanek
2022-10-21 11:58 ` Tomas Vanek
2022-10-27 10:31 ` Luis Machado
2022-10-27 17:46 ` Tomas Vanek
2022-11-22 6:48 ` Tomas Vanek
2022-11-22 7:27 ` Luis Machado
2022-11-28 11:48 ` [PING] " Tomas Vanek
2022-12-08 1:15 ` Luis Machado
2022-12-21 8:52 ` [PING 2] " Tomas Vanek
2023-01-10 13:19 ` [PING 3] " Tomas Vanek
2023-01-10 15:31 ` Simon Marchi
2023-01-10 16:33 ` Tomas Vanek
2023-01-10 17:48 ` Simon Marchi
2023-01-10 23:22 ` Tomas Vanek [this message]
2023-01-11 1:38 ` Simon Marchi
2023-02-02 6:38 ` Tomas Vanek
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=7a8c358c-7de3-8390-2566-cee03278184e@fbl.cz \
--to=vanekt@fbl.cz \
--cc=gdb-patches@sourceware.org \
--cc=luis.machado@arm.com \
--cc=simark@simark.ca \
/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).