From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp00.avonet.cz (smtp00.avonet.cz [217.112.162.55]) by sourceware.org (Postfix) with ESMTP id 4131E3855580 for ; Fri, 17 Feb 2023 08:53:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4131E3855580 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=fbl.cz Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=fbl.cz Received: from ktus.lan (217-115-245-101.cust.avonet.cz [217.115.245.101]) by smtp00.avonet.cz (Postfix) with ESMTP id 4PJ5FW2bG6z1y8K for ; Fri, 17 Feb 2023 09:53:35 +0100 (CET) Received: by ktus.lan (Postfix, from userid 209) id 46389322F0D; Fri, 17 Feb 2023 09:53:35 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-Spam-Level: X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,SPF_HELO_NONE,TXREP,T_SPF_PERMERROR autolearn=ham autolearn_force=no version=3.4.6 Received: from [192.168.33.9] (217-115-245-101.cust.avonet.cz [217.115.245.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: vanekt) by ktus.lan (Postfix) with ESMTPSA id 5526C322F09; Fri, 17 Feb 2023 09:53:29 +0100 (CET) Message-ID: <99106d68-67ce-a493-be5e-4775602b4d1c@fbl.cz> Date: Fri, 17 Feb 2023 09:53:28 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Subject: [PING] [PATCH v3] gdb: Modify until_break_command to act correctly on SIGTRAMP_FRAME Content-Language: cs To: gdb-patches@sourceware.org, Simon Marchi References: <1673460096-20054-1-git-send-email-vanekt@fbl.cz> From: Tomas Vanek In-Reply-To: <1673460096-20054-1-git-send-email-vanekt@fbl.cz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: On 11/01/2023 19:01, Tomas Vanek wrote: > 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: > STM32F4, 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 > > The commit 7eb895307f53 Skip unwritable frames in command "finish" > added the gdbarch based mechanism for distinguishing the frames not > suitable for setting a breakpoint. It unfortunately limited the changes > to the 'finish' command and left other commands potentially setting > a breakpoint at magic address. > > This patch introduces get_prev_frame_for_breakpoint () for skipping > over frames that are not suitable for guarding with a breakpoint. > If no suitable frame is found, a momentary breakpoint is not set. > Instead of several independent accesses to frame_unwind_caller_id (), > frame_unwind_caller_pc () and frame_unwind_caller_arch () referencing > the current frame, frame_info_ptr of the real caller is resolved > first and then used to get_frame_id (), get_frame_pc () and > get_frame_arch (). > > v2: Comment fixes, bug reference. > > v3: get_prev_frame_for_breakpoint () introduced. > Fixed skipping inline frames. > Extended to a local variable scope guard breakpoint of the 'watch' > command and to insert_step_resume_breakpoint_at_caller (). > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28683 > Signed-off-by: Tomas Vanek > --- > gdb/breakpoint.c | 26 +++++++++++++++----------- > gdb/frame.c | 37 +++++++++++++++++++++++++++++++++++++ > gdb/frame.h | 10 +++++++++- > gdb/infrun.c | 15 ++++++++++----- > 4 files changed, 71 insertions(+), 17 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 00cc2ab..e1f04b6 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -10212,12 +10212,13 @@ enum print_stop_action > that we will encounter it first in bpstat_stop_status. */ > if (exp_valid_block != NULL && wp_frame != NULL) > { > - frame_id caller_frame_id = frame_unwind_caller_id (wp_frame); > + frame_info_ptr caller_frame; > + caller_frame = get_prev_frame_for_breakpoint (wp_frame); > > - if (frame_id_p (caller_frame_id)) > + if (caller_frame) > { > - gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame); > - CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame); > + gdbarch *caller_arch = get_frame_arch (caller_frame); > + CORE_ADDR caller_pc = get_frame_pc (caller_frame); > > scope_breakpoint > = create_internal_breakpoint (caller_arch, caller_pc, > @@ -10232,7 +10233,7 @@ enum print_stop_action > scope_breakpoint->disposition = disp_del; > > /* Only break in the proper frame (help with recursion). */ > - scope_breakpoint->frame_id = caller_frame_id; > + scope_breakpoint->frame_id = get_frame_id (caller_frame); > > /* Set the address at which we will stop. */ > scope_breakpoint->loc->gdbarch = caller_arch; > @@ -10592,9 +10593,9 @@ 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; > int thread; > struct thread_info *tp; > > @@ -10630,7 +10631,7 @@ 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_for_breakpoint (frame); > > /* Keep within the current frame, or in frames called by the current > one. */ > @@ -10639,14 +10640,17 @@ enum async_reply_reason > > gdb::optional lj_deleter; > > - if (frame_id_p (caller_frame_id)) > + if (caller_frame) > { > struct symtab_and_line sal2; > struct gdbarch *caller_gdbarch; > + struct frame_id caller_frame_id; > + CORE_ADDR pc = get_frame_pc (caller_frame); > > - sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0); > - sal2.pc = frame_unwind_caller_pc (frame); > - caller_gdbarch = frame_unwind_caller_arch (frame); > + sal2 = find_pc_line (pc, 0); > + sal2.pc = pc; > + caller_gdbarch = get_frame_arch (caller_frame); > + caller_frame_id = get_frame_id (caller_frame); > > breakpoint_up caller_breakpoint > = set_momentary_breakpoint (caller_gdbarch, sal2, > diff --git a/gdb/frame.c b/gdb/frame.c > index 2f9622a..c547037 100644 > --- a/gdb/frame.c > +++ b/gdb/frame.c > @@ -537,6 +537,43 @@ struct frame_info > return frame; > } > > +/* Similar to skip_artificial_frames () but skips also unwritable frames. */ > + > +static frame_info_ptr > +skip_artificial_unwritable_frames (frame_info_ptr frame) > +{ > + while (get_frame_type (frame) == INLINE_FRAME > + || get_frame_type (frame) == TAILCALL_FRAME > + || gdbarch_code_of_frame_writable (get_frame_arch (frame), frame) == 0) > + { > + frame = get_prev_frame_always (frame); > + if (frame == NULL) > + break; > + } > + > + return frame; > +} > + > +/* Similar to get_prev_frame_always () but skips also unwritable frames. > + Returns nullptr if no suitable frame found. > + Use for setting a guard breakpoint after return to the caller. */ > + > +frame_info_ptr > +get_prev_frame_for_breakpoint (frame_info_ptr next_frame) > +{ > + frame_info_ptr this_frame; > + > + next_frame = skip_artificial_unwritable_frames (next_frame); > + if (next_frame == NULL) > + return nullptr; > + > + this_frame = get_prev_frame_always (next_frame); > + if (this_frame == NULL) > + return nullptr; > + > + return skip_artificial_unwritable_frames (this_frame); > +} > + > /* See frame.h. */ > > frame_info_ptr > diff --git a/gdb/frame.h b/gdb/frame.h > index 5935465..9198061 100644 > --- a/gdb/frame.h > +++ b/gdb/frame.h > @@ -279,6 +279,12 @@ extern void restore_selected_frame (frame_id frame_id, int frame_level) > frame. */ > extern frame_info_ptr get_prev_frame_always (frame_info_ptr); > > +/* Return a "struct frame_info" corresponding to the frame that called > + THIS_FRAME. Returns NULL if there is no such frame. > + > + Unlike get_prev_frame_always, this function skips unwritable frames. */ > +extern frame_info_ptr get_prev_frame_for_breakpoint (frame_info_ptr); > + > /* Given a frame's ID, relocate the frame. Returns NULL if the frame > is not found. */ > extern frame_info_ptr frame_find_by_id (frame_id id); > @@ -533,7 +539,9 @@ extern void put_frame_register_bytes (frame_info_ptr frame, int regnum, > > /* Unwind the PC. Strictly speaking return the resume address of the > calling frame. For GDB, `pc' is the resume address and not a > - specific register. */ > + specific register. > + Does not skip unwritable frames. Do not use for setting breakpoint > + after returnt to the caller. */ > > extern CORE_ADDR frame_unwind_caller_pc (frame_info_ptr frame); > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 181d961..0822b0d 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -7930,20 +7930,25 @@ static bool restart_stepped_thread (process_stratum_target *resume_target, > static void > insert_step_resume_breakpoint_at_caller (frame_info_ptr next_frame) > { > + frame_info_ptr caller_frame; > + struct frame_id caller_frame_id; > + > + caller_frame = get_prev_frame_for_breakpoint (next_frame); > + caller_frame_id = get_frame_id (caller_frame); > + > /* We shouldn't have gotten here if we don't know where the call site > is. */ > - gdb_assert (frame_id_p (frame_unwind_caller_id (next_frame))); > + gdb_assert (frame_id_p (caller_frame_id)); > > - struct gdbarch *gdbarch = frame_unwind_caller_arch (next_frame); > + struct gdbarch *gdbarch = get_frame_arch (caller_frame); > > symtab_and_line sr_sal; > sr_sal.pc = gdbarch_addr_bits_remove (gdbarch, > - frame_unwind_caller_pc (next_frame)); > + get_frame_pc (caller_frame)); > sr_sal.section = find_pc_overlay (sr_sal.pc); > sr_sal.pspace = frame_unwind_program_space (next_frame); > > - insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal, > - frame_unwind_caller_id (next_frame)); > + insert_step_resume_breakpoint_at_sal (gdbarch, sr_sal, caller_frame_id); > } > > /* Insert a "longjmp-resume" breakpoint at PC. This is used to set a