public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@polymtl.ca>
Subject: [RFC] [gdb] Stop on undetermined longjmp target during next
Date: Thu,  8 Dec 2022 12:31:40 +0100	[thread overview]
Message-ID: <20221208113140.1231-1-tdevries@suse.de> (raw)

Currently, for a libc without probes, we have:
...
56            longjmp (env, 1);^M
(gdb) PASS: gdb.base/longjmp.exp: pattern 1: next to longjmp
next^M
^M
Breakpoint 3, main () at longjmp.c:59^M
59        i = 1; /* miss_step_1 */^M
(gdb) KFAIL: gdb.base/longjmp.exp: pattern 1: gdb/26967 (PRMS: next over longjmp)
...

In other words, the next degrades to a continue, and we stop only because of the
safety net breakpoint at miss_step_1.

Instead, stop in the longjmp, in order to not lose the context that the next
was issued from:
...
56            longjmp (env, 1);^M
(gdb) PASS: gdb.base/longjmp.exp: pattern 1: next to longjmp
next^M
warning: GDB can't determine the longjmp target^M
0x00007ffff76dc8de in siglongjmp () from /lib64/libc.so.6^M
(gdb) KFAIL: gdb.base/longjmp.exp: pattern 1: gdb/26967 (PRMS: next over longjmp)
...
We can get the old behaviour back by issuing a continue command.

However, it also changes behaviour in the following case.  Without this patch,
we have:
...
Breakpoint 6, main () at longjmp.c:75^M
75        hidden_longjmp (); /* patt3 */^M
(gdb) PASS: gdb.base/longjmp.exp: pattern 3: setup: \
  continue to breakpoint at pattern start
next^M
77        i = 77; /* longjmp caught */^M
(gdb) PASS: gdb.base/longjmp.exp: pattern 3: next over pattern
...
and with this patch, we get instead:
...
75        hidden_longjmp (); /* patt3 */^M
(gdb) PASS: gdb.base/longjmp.exp: pattern 3: setup: \
  continue to breakpoint at pattern start
next^M
warning: GDB can't determine the longjmp target^M
0x00007ffff76dc8de in siglongjmp () from /lib64/libc.so.6^M
(gdb) KFAIL: gdb.base/longjmp.exp: pattern 3: gdb/26967 (PRMS: next over pattern)
...
In this case, we cannot get the old behaviour back by issuing a continue
command, because the stop will have removed the temporary breakpoint on line 77
placed there by the next command.  Instead, our only option to "finish the next"
as before is to set a breakpoint on line 77 and continue.

Note that while the user experience in the first case improves, in the second
case it degrades, and possibly this is the reason that the old behaviour was
chosen in the first place.

[ It would be possible to address this conundrum by presenting the user with a
question to choose the behaviour: stop, or continue and hope to finish the next.
Another option would be to leave the address of the temporary breakpoint in
some convenience variable, such that the user could do something like:
...
warning: next aborted, tbreak address available in \
  $_aborted_next_tbreak_address
(gdb) tbreak *$_aborted_next_tbreak_address
(gdb) continue
... ]

Note also that this makes the behaviour the same between the scenarios that:
- the longjmp target cannot be found, and
- the longjmp target can be found, but is mangled and we fail to set a
  breakpoint,
in the sense that we now get a stop in both cases.

Tested on x86_64-linux, with a glibc that does support probes, and a gdb patch
that ignores the libc longjmp probe:
...
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -980,4 +980,6 @@ OBJECT matches the executable or shared library name.\n\
 If you do not specify any argument then the command will ignore\n\
 all defined probes.  Only supported for SystemTap probes"),
           &maintenancelist);
+
+  ignore_probes_command ("libc ^longjmp$", 0);
 }
...
using the RFC patch introducing the "maint ignore-probes" command (
https://sourceware.org/pipermail/gdb-patches/2022-December/194535.html ).

The gdb.base/longjmp.exp test-case has been updated to accept the new
behaviour.

I found these regressions (only first in test-case listed):
...
FAIL: gdb.base/longjmp-until-in-main.exp: until $line, in main
FAIL: gdb.base/premature-dummy-frame-removal.exp: p some_func ()
FAIL: gdb.base/stale-infcall.exp: test system longjmp tracking support
...
These will need to be updated to accept the new behaviour.  Since this is an
RFC, I've left that for now.

Suggested-By: Simon Marchi <simon.marchi@efficios.com>
---
 gdb/infrun.c                       |  3 ++-
 gdb/testsuite/gdb.base/longjmp.exp | 14 ++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index c67458b30b6..51bc72dae5f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6650,7 +6650,8 @@ process_event_stop_test (struct execution_control_state *ecs)
 	    {
 	      infrun_debug_printf ("BPSTAT_WHAT_SET_LONGJMP_RESUME "
 				   "(!gdbarch_get_longjmp_target)");
-	      keep_going (ecs);
+	      warning (_("GDB can't determine the longjmp target"));
+	      end_stepping_range (ecs);
 	      return;
 	    }
 
diff --git a/gdb/testsuite/gdb.base/longjmp.exp b/gdb/testsuite/gdb.base/longjmp.exp
index 0f78304a14a..c3f46f06698 100644
--- a/gdb/testsuite/gdb.base/longjmp.exp
+++ b/gdb/testsuite/gdb.base/longjmp.exp
@@ -81,6 +81,9 @@ set re_cannot_insert_bp \
 	 "Cannot insert breakpoint $decimal\\." \
 	 "Cannot access memory at address $hex"]
 
+set re_no_longjmp_target \
+    "warning: GDB can't determine the longjmp target"
+
 #
 # Pattern 1 - simple longjmp.
 #
@@ -111,7 +114,7 @@ with_test_prefix "pattern 1" {
 	    gdb_test "next" "resumes\\+\\+.*" "next into else block"
 	    gdb_test "next" "miss_step_1.*" "next into safety net"
 	}
-	-re "miss_step_1.*$gdb_prompt $" {
+	-re -wrap "\r\n$re_no_longjmp_target\r\n.*" {
 	    if { $have_longjmp_probe } {
 		fail $gdb_test_name
 	    } else {
@@ -158,7 +161,7 @@ with_test_prefix "pattern 2" {
 	    gdb_test "next" "resumes\\+\\+.*" "next into else block"
 	    gdb_test "next" "miss_step_2.*" "next into safety net"
 	}
-	-re "miss_step_2.*$gdb_prompt $" {
+	-re -wrap "\r\n$re_no_longjmp_target\r\n.*" {
 	    if { $have_longjmp_probe } {
 		fail $gdb_test_name
 	    } else {
@@ -194,6 +197,13 @@ with_test_prefix "pattern 3" {
 	-re -wrap "longjmp caught.*" {
 	    pass $gdb_test_name
 	}
+	-re -wrap "\r\n$re_no_longjmp_target\r\n.*" {
+	    if { $have_longjmp_probe } {
+		fail $gdb_test_name
+	    } else {
+		kfail $gdb_test_name "gdb/26967"
+	    }
+	}
 	-re -wrap "\r\n$re_cannot_insert_bp\r\n.*" {
 	    if { $have_longjmp_probe } {
 		fail $gdb_test_name

base-commit: c9a2e0cd0a03944351807b6ab2f1b80ea7724126
-- 
2.35.3


             reply	other threads:[~2022-12-08 11:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-08 11:31 Tom de Vries [this message]
2022-12-08 12:02 ` Andreas Schwab
2022-12-08 13:33   ` Tom de Vries
2022-12-15 21:31 ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221208113140.1231-1-tdevries@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).