From patchwork Wed Jan 11 18:01:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomas Vanek X-Patchwork-Id: 62955 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B59EE3857C4F for ; Wed, 11 Jan 2023 18:02:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B59EE3857C4F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673460148; bh=jypQnXfkQKLXuCGAWzSMISaUDdqHtbvnjTN0L6BgNUQ=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=xQSEjKF9+TAx9bS0qhzOZYSsMIp6RxpLA4okz3PLv2YWSt1axpWoWABZB6bq+wCyI OzScU7VF95jVJbmvVKZdZf2N/22vwNX66oSRxIb0nbHR73zcGV1bzpjrC8GFpVwVet 1O1DPcM5zP5Sg0VJfEp3D0rTYlD/NUhO2oiWi+P4= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp00.avonet.cz (smtp00.avonet.cz [217.112.162.55]) by sourceware.org (Postfix) with ESMTP id 24E083858C83 for ; Wed, 11 Jan 2023 18:02:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 24E083858C83 Received: from ktus.lan (217-115-245-101.cust.avonet.cz [217.115.245.101]) by smtp00.avonet.cz (Postfix) with ESMTP id 4Nsb9M04HGz1xtM; Wed, 11 Jan 2023 19:01:58 +0100 (CET) To: gdb-patches@sourceware.org Cc: Tomas Vanek Subject: [PATCH v3] gdb: Modify until_break_command to act correctly on SIGTRAMP_FRAME Date: Wed, 11 Jan 2023 19:01:36 +0100 Message-Id: <1673460096-20054-1-git-send-email-vanekt@fbl.cz> X-Mailer: git-send-email 1.9.1 X-Spam-Status: No, score=-12.8 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 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Tomas Vanek via Gdb-patches From: Tomas Vanek Reply-To: Tomas Vanek Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" 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