From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24200 invoked by alias); 15 Jun 2007 19:26:36 -0000 Received: (qmail 24188 invoked by uid 22791); 15 Jun 2007 19:26:35 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate6.de.ibm.com (HELO mtagate6.de.ibm.com) (195.212.29.155) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 15 Jun 2007 19:26:31 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.8/8.13.8) with ESMTP id l5FJQSXJ1588534 for ; Fri, 15 Jun 2007 19:26:28 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 l5FJQS5Q3854360 for ; Fri, 15 Jun 2007 21:26:28 +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 l5FJQSCh009786 for ; Fri, 15 Jun 2007 21:26:28 +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 l5FJQS6e009783 for ; Fri, 15 Jun 2007 21:26:28 +0200 Message-Id: <200706151926.l5FJQS6e009783@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Fri, 15 Jun 2007 21:26:28 +0200 Subject: [rfc] Fix infrun.c fall-out from the software single-step logic change To: gdb-patches@sourceware.org Date: Fri, 15 Jun 2007 19:26:00 -0000 From: "Ulrich Weigand" 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/msg00299.txt.bz2 Hello, this fixes one outstanding issue after the ppc software single-step patch. adjust_pc_after_break was using the assumption that SOFTWARE_SINGLE_STEP_P was true if and only if we're using software single-step. With the new logic this is no longer true; instead the arch can decide on a case-by-case basis whether to use software or hardware single-stepping. This patch re-works the adjust_pc_after_break logic to remove that assumption. It also simplifies the logic a bit, due to the fact that now the software single-step breakpoints also show up via software_breakpoint_inserted_here_p. I've also removed a couple of checks for SOFTWARE_SINGLE_STEP_P in handle_inferior_event -- simply checking for singlestep_breakpoints_inserted_p should be OK now. Tested on powerpc64-linux and spu-elf. Any comments? I don't actually have a test case where the old code shows a bug, but it is clearly wrong ... 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. (handle_inferior_event): Do not check for SOFTWARE_SINGLE_STEP_P. diff -urNp gdb-orig/gdb/infrun.c gdb-head/gdb/infrun.c --- gdb-orig/gdb/infrun.c 2007-06-15 19:20:34.030743111 +0200 +++ gdb-head/gdb/infrun.c 2007-06-15 21:08:14.534043469 +0200 @@ -1183,53 +1183,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 the software breakpoint, and have to back + up the breakpoint address. + + As a special case, we could have hardware single-stepped a + 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 @@ -1372,7 +1353,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; @@ -1392,7 +1373,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; @@ -1548,8 +1529,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)); @@ -1598,7 +1578,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. */ @@ -1669,7 +1649,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 (); @@ -1718,7 +1698,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; @@ -1740,7 +1720,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