[PATCH] gdb.base/sigstep.exp: Mark xfail on unsupported scenarios on riscv64-linux-gnu

[PATCH] gdb.base/sigstep.exp: Mark xfail on unsupported scenarios on riscv64-linux-gnu

[Lancelot SIX]

Currently, gdb.base/sigstep.exp has a lot of failures on riscv64-linux-gnu.

When a program is interrupted because a signal is sent to the process,
and the user wants to step/stepi, then GDB creates a software breakpoint
on the next instruction on the uninterrupted main program execution
flow. As a result, the signal handler is executed entirely before the
program is interrupted again. The expected behavior is that the
step/stepi stop on the first instruction of the triggered signal
handler.

This is due to the fact that on this platform, GDB uses software
breakpoints to singlestep (PTRACE_SINGLESTEP is not available, so the
kernel is not helping us, and we do not rely on hardware breakpoints).
The situation is summarized in the following comment in infrun.c:

  /* Currently, our software single-step implementation leads to different
     results than hardware single-stepping in one situation: when stepping
     into delivering a signal which has an associated signal handler,
     hardware single-step will stop at the first instruction of the handler,
     while software single-step will simply skip execution of the handler.

     For now, this difference in behavior is accepted since there is no
     easy way to actually implement single-stepping into a signal handler
     without kernel support.

     […] */

The following session illustrates the behavior (based on sigstep.c):

	$ gdb -quiet testsuite/outputs/gdb.base/sigstep/sigstep
	Reading symbols from ./testsuite/outputs/gdb.base/sigstep/sigstep...
	(gdb) display/i $pc
	1: x/i $pc
	<error: No registers.>
	(gdb) handle SIGVTALRM pass print stop
	Signal        Stop      Print   Pass to program Description
	SIGVTALRM     Yes       Yes     Yes             Virtual timer expired
	(gdb) run
	Starting program: /home/lsix/dev/gnu/worktrees/gdb/riscv-sigstep/_build/gdb/testsuite/outputs/gdb.base/sigstep/sigstep

	Program received signal SIGVTALRM, Virtual timer expired.
	0x0000002aaaaaa934 in main () at /home/lsix/dev/gnu/worktrees/gdb/riscv-sigstep/_build/gdb/../../gdb/testsuite/gdb.base/sigstep.c:88
	88            dummy = 0; dummy = 0; while (!done);
	1: x/i $pc
	=> 0x2aaaaaa934 <main+316>:     addi    a5,a5,-2032
	(gdb) stepi
	------>                                                            <------
	------>                      Handler executing here                <------
	------>                                                            <------
	0x0000002aaaaaa938      88            dummy = 0; dummy = 0; while (!done);
	1: x/i $pc
	=> 0x2aaaaaa938 <main+320>:     lw      a5,0(a5)
	(gdb)

The situation is similar when stepping outside of the signal handler.
GDB has no clue where the program will resume so cannot place a software
breakpoint there, so unless there is a breakpoint waiting for us for
some other reason, then the program will resume when the user expects
it to be interrupted on the first instruction that gets executed once
control goes back to the main program.

Since there is not much GDB can do here, this patch proposes to mark the
failing test scenarios as XFAIL.

Before the patch (with TESTS=gdb.base/sigstep.exp):

	                === gdb Summary ===

	# of expected passes            711
	# of unexpected failures        58

After this patch (with TESTS=gdb.base/sigstep.exp):

	                === gdb Summary ===

	# of expected passes            683
	# of unexpected failures        2
	# of expected failures          28

There are still 2 failure I have not gone through yet:

	FAIL: gdb.base/sigstep.exp: stepi from handleri: leave signal trampoline
	FAIL: gdb.base/sigstep.exp: nexti from handleri: leave signal trampoline

Tested on riscv64-linux-gnu.
---
 gdb/testsuite/gdb.base/sigstep.exp | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index ea254af5297..7b6d45397d5 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -93,7 +93,25 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
 	    gdb_test "handle SIGVTALRM print pass stop"
 	    gdb_test "continue" "Program received signal.*" "continue to signal"
 	}
-	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
+
+	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
+	    -re  ".*handler .*$gdb_prompt $" {
+		pass "$enter_cmd to handler"
+            }
+	    -re  ".*main.*$gdb_prompt $" {
+		if {$enter_cmd != "continue"} {
+		    # riscv64-linux uses software breakpoints to step/stepi.
+		    # With software breakpoint, we will fail to enter the
+		    # signal with step/stepi.  Instead, the signal handler is
+		    # executed and the process is then stopped again in main.
+	            setup_xfail "riscv64-unknown-linux-gnu"
+		}
+		fail "$enter_cmd to handler (stepped over signal handleR)"
+		# The remaining of the test is irrelevant since we never got into
+		# the signal handler.
+		return
+	    }
+	}
 
 	delete_breakpoints
 
@@ -134,6 +152,15 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
 		# on the first instruction of "while...".  Accept both cases.
 		pass "$test"
 	    }
+	    timeout {
+	        if { $exit_cmd != "continue" } {
+		    # Similarly to entering the signal handler, software
+		    # breakpoints used on riscv64-linux fail to step/stepi
+		    # back into main.
+	            setup_xfail "riscv64-unknown-linux-gnu"
+	        }
+		fail "$test (timeout)"
+	    }
 	}
     }
 }
-- 
2.30.2

Re: [PATCH] gdb.base/sigstep.exp: Mark xfail on unsupported scenarios on riscv64-linux-gnu

[Andrew Burgess]

* Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> [2021-07-04 17:00:54 +0000]:

> Currently, gdb.base/sigstep.exp has a lot of failures on riscv64-linux-gnu.
> 
> When a program is interrupted because a signal is sent to the process,
> and the user wants to step/stepi, then GDB creates a software breakpoint
> on the next instruction on the uninterrupted main program execution
> flow. As a result, the signal handler is executed entirely before the
> program is interrupted again. The expected behavior is that the
> step/stepi stop on the first instruction of the triggered signal
> handler.
> 
> This is due to the fact that on this platform, GDB uses software
> breakpoints to singlestep (PTRACE_SINGLESTEP is not available, so the
> kernel is not helping us, and we do not rely on hardware breakpoints).
> The situation is summarized in the following comment in infrun.c:
> 
>   /* Currently, our software single-step implementation leads to different
>      results than hardware single-stepping in one situation: when stepping
>      into delivering a signal which has an associated signal handler,
>      hardware single-step will stop at the first instruction of the handler,
>      while software single-step will simply skip execution of the handler.
> 
>      For now, this difference in behavior is accepted since there is no
>      easy way to actually implement single-stepping into a signal handler
>      without kernel support.
> 
>      […] */
> 
> The following session illustrates the behavior (based on sigstep.c):
> 
> 	$ gdb -quiet testsuite/outputs/gdb.base/sigstep/sigstep
> 	Reading symbols from ./testsuite/outputs/gdb.base/sigstep/sigstep...
> 	(gdb) display/i $pc
> 	1: x/i $pc
> 	<error: No registers.>
> 	(gdb) handle SIGVTALRM pass print stop
> 	Signal        Stop      Print   Pass to program Description
> 	SIGVTALRM     Yes       Yes     Yes             Virtual timer expired
> 	(gdb) run
> 	Starting program: /home/lsix/dev/gnu/worktrees/gdb/riscv-sigstep/_build/gdb/testsuite/outputs/gdb.base/sigstep/sigstep
> 
> 	Program received signal SIGVTALRM, Virtual timer expired.
> 	0x0000002aaaaaa934 in main () at /home/lsix/dev/gnu/worktrees/gdb/riscv-sigstep/_build/gdb/../../gdb/testsuite/gdb.base/sigstep.c:88
> 	88            dummy = 0; dummy = 0; while (!done);
> 	1: x/i $pc
> 	=> 0x2aaaaaa934 <main+316>:     addi    a5,a5,-2032
> 	(gdb) stepi
> 	------>                                                            <------
> 	------>                      Handler executing here                <------
> 	------>                                                            <------
> 	0x0000002aaaaaa938      88            dummy = 0; dummy = 0; while (!done);
> 	1: x/i $pc
> 	=> 0x2aaaaaa938 <main+320>:     lw      a5,0(a5)
> 	(gdb)
> 
> The situation is similar when stepping outside of the signal handler.
> GDB has no clue where the program will resume so cannot place a software
> breakpoint there, so unless there is a breakpoint waiting for us for
> some other reason, then the program will resume when the user expects
> it to be interrupted on the first instruction that gets executed once
> control goes back to the main program.
> 
> Since there is not much GDB can do here, this patch proposes to mark the
> failing test scenarios as XFAIL.

I wonder if, rather than hard coding the xfail to check for RISC-V, we
could add a helper function to lib/gdb.exp like:

  proc uses_software_single_step_p {} {
    ...
  }

which would return true/false indicating if the current target uses
software single stepping?

We might want to use this test in other places later, and if there are
other targets that also have this issue it is then super easy for them
to add their architecture into this new proc.

However, rather than have the new proc check a list of architectures,
we could possible make it do something like this:

  (gdb) pipe maintenance print architecture | grep gdbarch_software_single_step_p
  gdbarch_dump: gdbarch_software_single_step_p() = 0

The above was run on x86-64 which doesn't use software single step,
but I believe on RISC-V/Linux the above should look like this:

  (gdb) pipe maintenance print architecture | grep gdbarch_software_single_step_p
  gdbarch_dump: gdbarch_software_single_step_p() = 1

This would then automatically catch any target that use software
single stepping.

What do you think?

Thanks,
Andrew





> 
> Before the patch (with TESTS=gdb.base/sigstep.exp):
> 
> 	                === gdb Summary ===
> 
> 	# of expected passes            711
> 	# of unexpected failures        58
> 
> After this patch (with TESTS=gdb.base/sigstep.exp):
> 
> 	                === gdb Summary ===
> 
> 	# of expected passes            683
> 	# of unexpected failures        2
> 	# of expected failures          28
> 
> There are still 2 failure I have not gone through yet:
> 
> 	FAIL: gdb.base/sigstep.exp: stepi from handleri: leave signal trampoline
> 	FAIL: gdb.base/sigstep.exp: nexti from handleri: leave signal trampoline
> 
> Tested on riscv64-linux-gnu.
> ---
>  gdb/testsuite/gdb.base/sigstep.exp | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
> index ea254af5297..7b6d45397d5 100644
> --- a/gdb/testsuite/gdb.base/sigstep.exp
> +++ b/gdb/testsuite/gdb.base/sigstep.exp
> @@ -93,7 +93,25 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
>  	    gdb_test "handle SIGVTALRM print pass stop"
>  	    gdb_test "continue" "Program received signal.*" "continue to signal"
>  	}
> -	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
> +
> +	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
> +	    -re  ".*handler .*$gdb_prompt $" {
> +		pass "$enter_cmd to handler"
> +            }
> +	    -re  ".*main.*$gdb_prompt $" {
> +		if {$enter_cmd != "continue"} {
> +		    # riscv64-linux uses software breakpoints to step/stepi.
> +		    # With software breakpoint, we will fail to enter the
> +		    # signal with step/stepi.  Instead, the signal handler is
> +		    # executed and the process is then stopped again in main.
> +	            setup_xfail "riscv64-unknown-linux-gnu"
> +		}
> +		fail "$enter_cmd to handler (stepped over signal handleR)"
> +		# The remaining of the test is irrelevant since we never got into
> +		# the signal handler.
> +		return
> +	    }
> +	}
>  
>  	delete_breakpoints
>  
> @@ -134,6 +152,15 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
>  		# on the first instruction of "while...".  Accept both cases.
>  		pass "$test"
>  	    }
> +	    timeout {
> +	        if { $exit_cmd != "continue" } {
> +		    # Similarly to entering the signal handler, software
> +		    # breakpoints used on riscv64-linux fail to step/stepi
> +		    # back into main.
> +	            setup_xfail "riscv64-unknown-linux-gnu"
> +	        }
> +		fail "$test (timeout)"
> +	    }
>  	}
>      }
>  }
> -- 
> 2.30.2
> 

Re: [PATCH] gdb.base/sigstep.exp: Mark xfail on unsupported scenarios on riscv64-linux-gnu

[Lancelot SIX]

On Mon, Jul 05, 2021 at 09:39:08AM +0100, Andrew Burgess wrote:
> 
> I wonder if, rather than hard coding the xfail to check for RISC-V, we
> could add a helper function to lib/gdb.exp like:
> 
>   proc uses_software_single_step_p {} {
>     ...
>   }
> 
> which would return true/false indicating if the current target uses
> software single stepping?
> 
> We might want to use this test in other places later, and if there are
> other targets that also have this issue it is then super easy for them
> to add their architecture into this new proc.
> 
> However, rather than have the new proc check a list of architectures,
> we could possible make it do something like this:
> 
>   (gdb) pipe maintenance print architecture | grep gdbarch_software_single_step_p
>   gdbarch_dump: gdbarch_software_single_step_p() = 0
> 
> The above was run on x86-64 which doesn't use software single step,
> but I believe on RISC-V/Linux the above should look like this:
> 
>   (gdb) pipe maintenance print architecture | grep gdbarch_software_single_step_p
>   gdbarch_dump: gdbarch_software_single_step_p() = 1

Hi

Yes, it does!

> 
> This would then automatically catch any target that use software
> single stepping.
> 
> What do you think?
> 
> Thanks,
> Andrew
> 

Your option is nice, but as Pedro pointed out in a later email there is
already a proc can_single_step_to_signal_handler that I somehow missed.
For the context of this particular patch, I think I’ll go with it.

That being said, I kinda prefer your approach that should be equivalent
(I guess) and avoid the hard written list of unsupported host targets.
I’ll add this to my TODO list.

Lancelot.

Re: [PATCH] gdb.base/sigstep.exp: Mark xfail on unsupported scenarios on riscv64-linux-gnu

[Pedro Alves]

There's already a "can_single_step_to_signal_handler" proc in gdb.exp for this.

 # Return 1 if target hardware or OS supports single stepping to signal
 # handler, otherwise, return 0.

 proc can_single_step_to_signal_handler {} {
     # Targets don't have hardware single step.  On these targets, when
     # a signal is delivered during software single step, gdb is unable
     # to determine the next instruction addresses, because start of signal
     # handler is one of them.
     return [can_hardware_single_step]
 }

this is already checked by sigstep.exp, here:

 foreach enter_cmd { "stepi" "nexti" "step" "next" "continue" } {
     if { $enter_cmd != "continue" && ![can_single_step_to_signal_handler] } {
         continue
     }

You should just need to make can_hardware_single_step return false for riscv64.

Re: [PATCH] gdb.base/sigstep.exp: Mark xfail on unsupported scenarios on riscv64-linux-gnu

[Lancelot SIX]

On Mon, Jul 05, 2021 at 11:26:15AM +0100, Pedro Alves wrote:
> There's already a "can_single_step_to_signal_handler" proc in gdb.exp for this.
> 
>  # Return 1 if target hardware or OS supports single stepping to signal
>  # handler, otherwise, return 0.
> 
>  proc can_single_step_to_signal_handler {} {
>      # Targets don't have hardware single step.  On these targets, when
>      # a signal is delivered during software single step, gdb is unable
>      # to determine the next instruction addresses, because start of signal
>      # handler is one of them.
>      return [can_hardware_single_step]
>  }
> 
> this is already checked by sigstep.exp, here:
> 
>  foreach enter_cmd { "stepi" "nexti" "step" "next" "continue" } {
>      if { $enter_cmd != "continue" && ![can_single_step_to_signal_handler] } {
>          continue
>      }
> 
> You should just need to make can_hardware_single_step return false for riscv64.
> 

Indeed.

Thanks for pointing this out!

Lancelot.