[RFC] [gdb] Stop on undetermined longjmp target during next

[RFC] [gdb] Stop on undetermined longjmp target during next

[Tom de Vries]

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

Re: [RFC] [gdb] Stop on undetermined longjmp target during next

[Andreas Schwab]

On Dez 08 2022, Tom de Vries via Gdb-patches wrote:

> 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"));

It's quite unusual to talk about yourself in the third person.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [RFC] [gdb] Stop on undetermined longjmp target during next

[Tom de Vries]

On 12/8/22 13:02, Andreas Schwab wrote:
> On Dez 08 2022, Tom de Vries via Gdb-patches wrote:
> 
>> 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"));
> 
> It's quite unusual to talk about yourself in the third person.
> 

I suppose it is.

It's not unusual though, I can find quite a few examples in the gdb sources.

I think that stressing that the warning comes from gdb is not a bad 
idea, because warnings might also be generated by inferiors.

Thanks,
- Tom

Re: [RFC] [gdb] Stop on undetermined longjmp target during next

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> --- a/gdb/infrun.c
Tom> +++ b/gdb/infrun.c
Tom> @@ -6650,7 +6650,8 @@ process_event_stop_test (struct execution_control_state *ecs)
Tom>  	    {
Tom>  	      infrun_debug_printf ("BPSTAT_WHAT_SET_LONGJMP_RESUME "
Tom>  				   "(!gdbarch_get_longjmp_target)");
Tom> -	      keep_going (ecs);
Tom> +	      warning (_("GDB can't determine the longjmp target"));
Tom> +	      end_stepping_range (ecs);

I'm not really an infrun expert, so I haven't replied to this.
But, FWIW, I think the idea makes sense.

Tom