From patchwork Tue Apr 7 12:49:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 6049 Received: (qmail 130755 invoked by alias); 7 Apr 2015 12:50:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 130677 invoked by uid 89); 7 Apr 2015 12:50:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 07 Apr 2015 12:50:04 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t37Co2Ae020840 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 7 Apr 2015 08:50:02 -0400 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t37Cnowu022139 for ; Tue, 7 Apr 2015 08:50:02 -0400 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 10/23] PPC64: Fix step-over-trips-on-watchpoint.exp with displaced stepping on Date: Tue, 7 Apr 2015 13:49:37 +0100 Message-Id: <1428410990-28560-11-git-send-email-palves@redhat.com> In-Reply-To: <1428410990-28560-1-git-send-email-palves@redhat.com> References: <1428410990-28560-1-git-send-email-palves@redhat.com> PPC64 current fails this test like: FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: no thread-specific bp: step: step FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: no thread-specific bp: next: next FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: no thread-specific bp: continue: continue (the program exited) FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: with thread-specific bp: step: step FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: with thread-specific bp: next: next FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: displaced=on: with thread-specific bp: continue: continue (the program exited) The problem is that PPC is a non-continuable watchpoints architecture and the displaced stepping code isn't coping with that correctly. On such targets/architectures, a watchpoint traps _before_ the instruction executes/completes. On a watchpoint trap, the PC points at the instruction that triggers the watchpoint (side effects haven't happened yet). In order to move past the watchpoint, GDB needs to remove the watchpoint, single-step, and reinsert the watchpoint, just like when stepping past a breakpoint. The trouble is that if GDB is stepping over a breakpoint with displaced stepping, and the instruction under the breakpoint triggers a watchpoint, we get the watchpoint SIGTRAP, expecting a finished (hard or software) step trap. Even though the thread's PC hasn't advanced yet (must remove watchpoint for that), since we get a SIGTRAP, displaced_step_fixup thinks the single-step finished successfuly anyway, and calls gdbarch_displaced_step_fixup, which then adjusts the thread's registers incorrectly. The fix is to cancel the displaced step if we trip on a watchpoint. handle_inferior_event then processes the watchpoint event, and starts a new step-over, but this time, since we have a watchpoint to step over, watchpoints are removed from the target, so the step-over succeeds. The keep_going/resume changes are necessary because if we have a watchpoint to step over, we need to remove it from the target - displaced stepping doesn't help, the copy of the instruction in the scratch pad reads/writes to the same addresses, thus triggers the watchpoint too... So without those changes we keep triggering the watchpoint forever, never making progress. With non-stop that means we need to pause all threads momentarily. We could avoid that by removing the watchpoint _only_ from the thread that is moving past the watchpoint, but GDB is not prepared for that today. For remote targets, that would need new packets, so good to have this as fallback anyway. gdb/ChangeLog: 2015-04-07 Pedro Alves * infrun.c (displaced_step_fixup): Switch to the event ptid earlier. If the thread stopped for a watchpoint and the target/arch has non-continuable watchpoints, cancel the displaced step. (resume): Don't start a displaced step if in-line step-over info is valid. --- gdb/infrun.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 937a29d..fac0beb 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1792,6 +1792,8 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal) struct cleanup *old_cleanups; struct displaced_step_inferior_state *displaced = get_displaced_stepping_state (ptid_get_pid (event_ptid)); + struct regcache *regcache; + struct gdbarch *gdbarch; /* Was any thread of this process doing a displaced step? */ if (displaced == NULL) @@ -1806,13 +1808,20 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal) displaced_step_restore (displaced, displaced->step_ptid); + /* Fixup may need to read memory/registers. Switch to the thread + that we're fixing up. Also, target_stopped_by_watchpoint checks + the current thread. */ + switch_to_thread (event_ptid); + + regcache = get_thread_regcache (event_ptid); + gdbarch = get_regcache_arch (regcache); + /* Did the instruction complete successfully? */ - if (signal == GDB_SIGNAL_TRAP) + if (signal == GDB_SIGNAL_TRAP + && !(target_stopped_by_watchpoint () + && (target_have_steppable_watchpoint + || gdbarch_have_nonsteppable_watchpoint (gdbarch)))) { - /* Fixup may need to read memory/registers. Switch to the - thread that we're fixing up. */ - switch_to_thread (event_ptid); - /* Fix up the resulting state. */ gdbarch_displaced_step_fixup (displaced->step_gdbarch, displaced->step_closure, @@ -1824,7 +1833,6 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal) { /* Since the instruction didn't complete, all we can do is relocate the PC. */ - struct regcache *regcache = get_thread_regcache (event_ptid); CORE_ADDR pc = regcache_read_pc (regcache); pc = displaced->step_original + (pc - displaced->step_copy); @@ -2261,7 +2269,9 @@ resume (enum gdb_signal sig) /* If enabled, step over breakpoints by executing a copy of the instruction at a different address. */ - if (tp->control.trap_expected && use_displaced_stepping_now_p (gdbarch, sig)) + if (tp->control.trap_expected + && !step_over_info_valid_p () + && use_displaced_stepping_now_p (gdbarch, sig)) { struct displaced_step_inferior_state *displaced; @@ -2405,6 +2415,7 @@ resume (enum gdb_signal sig) if (debug_displaced && tp->control.trap_expected + && !step_over_info_valid_p () && use_displaced_stepping_now_p (gdbarch, sig)) { struct regcache *resume_regcache = get_thread_regcache (tp->ptid); @@ -6351,15 +6362,18 @@ keep_going (struct execution_control_state *ecs) || (step_what & STEP_OVER_BREAKPOINT)); remove_wps = (step_what & STEP_OVER_WATCHPOINT); - if (remove_bp - && !use_displaced_stepping_now_p (get_regcache_arch (regcache), - signo)) + if (remove_wps && remove_bp) + set_step_over_info (get_regcache_aspace (regcache), + regcache_read_pc (regcache), remove_wps); + else if (remove_wps) + set_step_over_info (NULL, 0, remove_wps); + else if (remove_bp + && !use_displaced_stepping_now_p (get_regcache_arch (regcache), + signo)) { set_step_over_info (get_regcache_aspace (regcache), regcache_read_pc (regcache), remove_wps); } - else if (remove_wps) - set_step_over_info (NULL, 0, remove_wps); else clear_step_over_info ();