summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPedro Alves <palves@redhat.com>2018-01-11 17:50:59 +0000
committerPedro Alves <palves@redhat.com>2018-01-11 17:50:59 +0000
commitf740fa5094817e948853bbcba46410a05175260d (patch)
tree7fa4f95e7773871bbf839f7dbe7892cdd8da5dec
parent936a312c04f9f1dd856571bf7573b5a8f51ed895 (diff)
gdb.base/breakpoint-in-ro-region.exp regression on sss targets (PR gdb/22583)
As Maciej reported at <https://sourceware.org/ml/gdb-patches/2017-12/msg00212.html>, this commit: commit d930703d68ae160ddfe8ebe5fdcf416fb6090e1e Date: Thu Nov 16 18:44:43 2017 +0000 Subject: Don't ever Quit out of resume caused regressions on software single-set targets, specifically: FAIL: gdb.base/breakpoint-in-ro-region.exp: always-inserted off: auto-hw off: single-step breakpoint is not left behind FAIL: gdb.base/breakpoint-in-ro-region.exp: always-inserted off: auto-hw on: single-step breakpoint is not left behind FAIL: gdb.base/breakpoint-in-ro-region.exp: always-inserted on: auto-hw off: step in ro region (cannot insert hw break) FAIL: gdb.base/breakpoint-in-ro-region.exp: always-inserted on: auto-hw off: single-step breakpoint is not left behind FAIL: gdb.base/breakpoint-in-ro-region.exp: always-inserted on: auto-hw on: single-step breakpoint is not left behind and indeed detailed logs indicate a breakpoint is left lingering, e.g.: (gdb) PASS: gdb.base/breakpoint-in-ro-region.exp: always-inserted off: auto-hw off: step in ro region (cannot insert sw break) maint info breakpoints 0 Num Type Disp Enb Address What 0 sw single-step keep y 0x00400774 in main at [...]/gdb/testsuite/gdb.base/breakpoint-in-ro-region.c:24 inf 1 thread 1 stop only in thread 1 (gdb) FAIL: gdb.base/breakpoint-in-ro-region.exp: always-inserted off: auto-hw off: single-step breakpoint is not left behind vs: (gdb) PASS: gdb.base/breakpoint-in-ro-region.exp: always-inserted off: auto-hw off: step in ro region (cannot insert sw break) maint info breakpoints 0 No breakpoint or watchpoint matching '0'. (gdb) PASS: gdb.base/breakpoint-in-ro-region.exp: always-inserted off: auto-hw off: single-step breakpoint is not left behind as at commit d930703d68ae^. Before commit d930703d68ae, we had a cleanup installed in 'resume' that would delete single-step breakpoints on error: /* Resuming. */ /* Things to clean up if we QUIT out of resume (). */ static void resume_cleanups (void *ignore) { if (!ptid_equal (inferior_ptid, null_ptid)) delete_single_step_breakpoints (inferior_thread ()); normal_stop (); } That whole function was removed by d930703d68ae mainly to eliminate the normal_stop call: ~~~~ Note that the exception called from within resume ends up calling normal_stop via resume_cleanups. That's very borked though, because normal_stop is going to re-handle whatever was the last reported event, possibly even re-running a hook stop... ~~~~ But as the regression shows, removing resume_cleanups completely went a bit too far, as the delete_single_step_breakpoints call is still necessary. So fix the regression by reinstating the delete_single_step_breakpoints call on error. However, since we're trying to eliminate cleanups, restore it in a different form (using TRY/CATCH). Tested on x86-64 GNU/Linux both top of master and on top of a series that implements software single-step on x86. gdb/ChangeLog: 2018-01-11 Pedro Alves <palves@redhat.com> PR gdb/22583 * infrun.c (resume): Rename to ... (resume_1): ... this. (resume): Reimplement as wrapper around resume_1.
-rw-r--r--gdb/ChangeLog7
-rw-r--r--gdb/infrun.c33
2 files changed, 37 insertions, 3 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8edb782f08..d40b914823 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
2018-01-11 Pedro Alves <palves@redhat.com>
+ PR gdb/22583
+ * infrun.c (resume): Rename to ...
+ (resume_1): ... this.
+ (resume): Reimplement as wrapper around resume_1.
+
+2018-01-11 Pedro Alves <palves@redhat.com>
+
PR remote/22597
* remote.c (remote_parse_stop_reply): Default to the last-set
general thread instead of to 'magic_null_ptid'.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 7e8d8da588..7d6aa7d069 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2355,10 +2355,11 @@ do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig)
}
/* Resume the inferior. SIG is the signal to give the inferior
- (GDB_SIGNAL_0 for none). */
+ (GDB_SIGNAL_0 for none). Note: don't call this directly; instead
+ call 'resume', which handles exceptions. */
-void
-resume (enum gdb_signal sig)
+static void
+resume_1 (enum gdb_signal sig)
{
struct regcache *regcache = get_current_regcache ();
struct gdbarch *gdbarch = regcache->arch ();
@@ -2730,6 +2731,32 @@ resume (enum gdb_signal sig)
do_target_resume (resume_ptid, step, sig);
tp->resumed = 1;
}
+
+/* Resume the inferior. SIG is the signal to give the inferior
+ (GDB_SIGNAL_0 for none). This is a wrapper around 'resume_1' that
+ rolls back state on error. */
+
+void
+resume (gdb_signal sig)
+{
+ TRY
+ {
+ resume_1 (sig);
+ }
+ CATCH (ex, RETURN_MASK_ALL)
+ {
+ /* If resuming is being aborted for any reason, delete any
+ single-step breakpoint resume_1 may have created, to avoid
+ confusing the following resumption, and to avoid leaving
+ single-step breakpoints perturbing other threads, in case
+ we're running in non-stop mode. */
+ if (inferior_ptid != null_ptid)
+ delete_single_step_breakpoints (inferior_thread ());
+ throw_exception (ex);
+ }
+ END_CATCH
+}
+
/* Proceeding. */