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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* 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
>