From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25659 invoked by alias); 21 Jun 2007 00:11:13 -0000 Received: (qmail 25641 invoked by uid 22791); 21 Jun 2007 00:11:11 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate8.de.ibm.com (HELO mtagate8.de.ibm.com) (195.212.29.157) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 21 Jun 2007 00:11:08 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate8.de.ibm.com (8.13.8/8.13.8) with ESMTP id l5L0B4dV431334 for ; Thu, 21 Jun 2007 00:11:04 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.3) with ESMTP id l5L0B536618558 for ; Thu, 21 Jun 2007 02:11:05 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id l5L0B47Z007028 for ; Thu, 21 Jun 2007 02:11:04 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id l5L0B4Dv007025; Thu, 21 Jun 2007 02:11:04 +0200 Message-Id: <200706210011.l5L0B4Dv007025@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 21 Jun 2007 02:11:04 +0200 Subject: Re: [rfc] Fix infrun.c fall-out from the software single-step logic change To: uweigand@de.ibm.com (Ulrich Weigand) Date: Thu, 21 Jun 2007 00:11:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <200706151926.l5FJQS6e009783@d12av02.megacenter.de.ibm.com> from "Ulrich Weigand" at Jun 15, 2007 09:26:28 PM X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-06/txt/msg00406.txt.bz2 > this fixes one outstanding issue after the ppc software single-step patch. Here's a slightly modified version of that patch; it fixes some errors in the comment wording, and also removes the SOFTWARE_SINGLE_STEP target macro (the last one remaining!). Tested on powerpc64-linux. Bye, Ulrich ChangeLog: * infrun.c (adjust_pc_after_break): Do not assume software single-step is always active if SOFTWARE_SINGLE_STEP_P is true. (resume): Use gdbarch_software_single_step[_p] instead of SOFTWARE_SINGLE_STEP[_P]. (handle_inferior_event): Do not check for SOFTWARE_SINGLE_STEP_P. * gdbarch.sh (software_single_step): Remove target macro. * gdbarch.h, gdbarch.c: Regenerate. diff -urNp gdb-orig/gdb/gdbarch.c gdb-head/gdb/gdbarch.c --- gdb-orig/gdb/gdbarch.c 2007-06-21 00:56:50.032443000 +0200 +++ gdb-head/gdb/gdbarch.c 2007-06-21 00:57:56.924903103 +0200 @@ -976,21 +976,9 @@ gdbarch_dump (struct gdbarch *current_gd fprintf_unfiltered (file, "gdbarch_dump: smash_text_address = <0x%lx>\n", (long) current_gdbarch->smash_text_address); -#ifdef SOFTWARE_SINGLE_STEP_P - fprintf_unfiltered (file, - "gdbarch_dump: %s # %s\n", - "SOFTWARE_SINGLE_STEP_P()", - XSTRING (SOFTWARE_SINGLE_STEP_P ())); -#endif fprintf_unfiltered (file, "gdbarch_dump: gdbarch_software_single_step_p() = %d\n", gdbarch_software_single_step_p (current_gdbarch)); -#ifdef SOFTWARE_SINGLE_STEP - fprintf_unfiltered (file, - "gdbarch_dump: %s # %s\n", - "SOFTWARE_SINGLE_STEP(frame)", - XSTRING (SOFTWARE_SINGLE_STEP (frame))); -#endif fprintf_unfiltered (file, "gdbarch_dump: software_single_step = <0x%lx>\n", (long) current_gdbarch->software_single_step); diff -urNp gdb-orig/gdb/gdbarch.h gdb-head/gdb/gdbarch.h --- gdb-orig/gdb/gdbarch.h 2007-06-21 00:56:50.074437000 +0200 +++ gdb-head/gdb/gdbarch.h 2007-06-21 00:57:56.967896904 +0200 @@ -526,30 +526,11 @@ extern void set_gdbarch_smash_text_addre A return value of 1 means that the software_single_step breakpoints were inserted; 0 means they were not. */ -#if defined (SOFTWARE_SINGLE_STEP) -/* Legacy for systems yet to multi-arch SOFTWARE_SINGLE_STEP */ -#if !defined (SOFTWARE_SINGLE_STEP_P) -#define SOFTWARE_SINGLE_STEP_P() (1) -#endif -#endif - extern int gdbarch_software_single_step_p (struct gdbarch *gdbarch); -#if !defined (GDB_TM_FILE) && defined (SOFTWARE_SINGLE_STEP_P) -#error "Non multi-arch definition of SOFTWARE_SINGLE_STEP" -#endif -#if !defined (SOFTWARE_SINGLE_STEP_P) -#define SOFTWARE_SINGLE_STEP_P() (gdbarch_software_single_step_p (current_gdbarch)) -#endif typedef int (gdbarch_software_single_step_ftype) (struct frame_info *frame); extern int gdbarch_software_single_step (struct gdbarch *gdbarch, struct frame_info *frame); extern void set_gdbarch_software_single_step (struct gdbarch *gdbarch, gdbarch_software_single_step_ftype *software_single_step); -#if !defined (GDB_TM_FILE) && defined (SOFTWARE_SINGLE_STEP) -#error "Non multi-arch definition of SOFTWARE_SINGLE_STEP" -#endif -#if !defined (SOFTWARE_SINGLE_STEP) -#define SOFTWARE_SINGLE_STEP(frame) (gdbarch_software_single_step (current_gdbarch, frame)) -#endif /* Return non-zero if the processor is executing a delay slot and a further single-step is needed before the instruction finishes. */ diff -urNp gdb-orig/gdb/gdbarch.sh gdb-head/gdb/gdbarch.sh --- gdb-orig/gdb/gdbarch.sh 2007-06-21 00:56:50.082436000 +0200 +++ gdb-head/gdb/gdbarch.sh 2007-06-21 00:57:57.012890417 +0200 @@ -582,7 +582,7 @@ f::CORE_ADDR:smash_text_address:CORE_ADD # # A return value of 1 means that the software_single_step breakpoints # were inserted; 0 means they were not. -F:=:int:software_single_step:struct frame_info *frame:frame +F::int:software_single_step:struct frame_info *frame:frame # Return non-zero if the processor is executing a delay slot and a # further single-step is needed before the instruction finishes. diff -urNp gdb-orig/gdb/infrun.c gdb-head/gdb/infrun.c --- gdb-orig/gdb/infrun.c 2007-06-21 00:56:50.132429000 +0200 +++ gdb-head/gdb/infrun.c 2007-06-21 00:57:57.060883498 +0200 @@ -537,10 +537,10 @@ how to step past a permanent breakpoint a command like `return' or `jump' to continue execution.")); } - if (SOFTWARE_SINGLE_STEP_P () && step) + if (step && gdbarch_software_single_step_p (current_gdbarch)) { /* Do it the hard way, w/temp breakpoints */ - if (SOFTWARE_SINGLE_STEP (get_current_frame ())) + if (gdbarch_software_single_step (current_gdbarch, get_current_frame ())) { /* ...and don't ask hardware to do it. */ step = 0; @@ -1184,53 +1184,34 @@ adjust_pc_after_break (struct execution_ breakpoint_pc = read_pc_pid (ecs->ptid) - gdbarch_decr_pc_after_break (current_gdbarch); - if (SOFTWARE_SINGLE_STEP_P ()) - { - /* When using software single-step, a SIGTRAP can only indicate - an inserted breakpoint. This actually makes things - easier. */ - if (singlestep_breakpoints_inserted_p) - /* When software single stepping, the instruction at [prev_pc] - is never a breakpoint, but the instruction following - [prev_pc] (in program execution order) always is. Assume - that following instruction was reached and hence a software - breakpoint was hit. */ - write_pc_pid (breakpoint_pc, ecs->ptid); - else if (software_breakpoint_inserted_here_p (breakpoint_pc)) - /* The inferior was free running (i.e., no single-step - breakpoints inserted) and it hit a software breakpoint. */ + /* Check whether there actually is a software breakpoint inserted + at that location. */ + if (software_breakpoint_inserted_here_p (breakpoint_pc)) + { + /* When using hardware single-step, a SIGTRAP is reported for both + a completed single-step and a software breakpoint. Need to + differentiate between the two, as the latter needs adjusting + but the former does not. + + The SIGTRAP can be due to a completed hardware single-step only if + - we didn't insert software single-step breakpoints + - the thread to be examined is still the current thread + - this thread is currently being stepped + + If any of these events did not occur, we must have stopped due + to hitting a software breakpoint, and have to back up to the + breakpoint address. + + As a special case, we could have hardware single-stepped a + software breakpoint. In this case (prev_pc == breakpoint_pc), + we also need to back up to the breakpoint address. */ + + if (singlestep_breakpoints_inserted_p + || !ptid_equal (ecs->ptid, inferior_ptid) + || !currently_stepping (ecs) + || prev_pc == breakpoint_pc) write_pc_pid (breakpoint_pc, ecs->ptid); } - else - { - /* When using hardware single-step, a SIGTRAP is reported for - both a completed single-step and a software breakpoint. Need - to differentiate between the two as the latter needs - adjusting but the former does not. - - When the thread to be examined does not match the current thread - context we can't use currently_stepping, so assume no - single-stepping in this case. */ - if (ptid_equal (ecs->ptid, inferior_ptid) && currently_stepping (ecs)) - { - if (prev_pc == breakpoint_pc - && software_breakpoint_inserted_here_p (breakpoint_pc)) - /* Hardware single-stepped a software breakpoint (as - occures when the inferior is resumed with PC pointing - at not-yet-hit software breakpoint). Since the - breakpoint really is executed, the inferior needs to be - backed up to the breakpoint address. */ - write_pc_pid (breakpoint_pc, ecs->ptid); - } - else - { - if (software_breakpoint_inserted_here_p (breakpoint_pc)) - /* The inferior was free running (i.e., no hardware - single-step and no possibility of a false SIGTRAP) and - hit a software breakpoint. */ - write_pc_pid (breakpoint_pc, ecs->ptid); - } - } } /* Given an execution control state that has been freshly filled in @@ -1373,7 +1354,7 @@ handle_inferior_event (struct execution_ (LONGEST) ecs->ws.value.integer)); gdb_flush (gdb_stdout); target_mourn_inferior (); - singlestep_breakpoints_inserted_p = 0; /* SOFTWARE_SINGLE_STEP_P() */ + singlestep_breakpoints_inserted_p = 0; stop_print_frame = 0; stop_stepping (ecs); return; @@ -1393,7 +1374,7 @@ handle_inferior_event (struct execution_ target_mourn_inferior (); print_stop_reason (SIGNAL_EXITED, stop_signal); - singlestep_breakpoints_inserted_p = 0; /* SOFTWARE_SINGLE_STEP_P() */ + singlestep_breakpoints_inserted_p = 0; stop_stepping (ecs); return; @@ -1549,8 +1530,7 @@ handle_inferior_event (struct execution_ if (stepping_past_singlestep_breakpoint) { - gdb_assert (SOFTWARE_SINGLE_STEP_P () - && singlestep_breakpoints_inserted_p); + gdb_assert (singlestep_breakpoints_inserted_p); gdb_assert (ptid_equal (singlestep_ptid, ecs->ptid)); gdb_assert (!ptid_equal (singlestep_ptid, saved_singlestep_ptid)); @@ -1599,7 +1579,7 @@ handle_inferior_event (struct execution_ if (!breakpoint_thread_match (stop_pc, ecs->ptid)) thread_hop_needed = 1; } - else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p) + else if (singlestep_breakpoints_inserted_p) { /* We have not context switched yet, so this should be true no matter which thread hit the singlestep breakpoint. */ @@ -1670,7 +1650,7 @@ handle_inferior_event (struct execution_ /* Saw a breakpoint, but it was hit by the wrong thread. Just continue. */ - if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p) + if (singlestep_breakpoints_inserted_p) { /* Pull the single step breakpoints out of the target. */ remove_single_step_breakpoints (); @@ -1719,7 +1699,7 @@ handle_inferior_event (struct execution_ return; } } - else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p) + else if (singlestep_breakpoints_inserted_p) { sw_single_step_trap_p = 1; ecs->random_signal = 0; @@ -1741,7 +1721,7 @@ handle_inferior_event (struct execution_ deprecated_context_hook (pid_to_thread_id (ecs->ptid)); } - if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p) + if (singlestep_breakpoints_inserted_p) { /* Pull the single step breakpoints out of the target. */ remove_single_step_breakpoints (); -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com