[3/3] selftests/powerpc: Don't rely on segfault to rerun the test

The test case tm-signal-context-force-tm expects a segfault to happen on
returning from signal handler, and then does a setcontext() to run the test
again. However, the test doesn't always segfault, causing the test to run a
single time.

This patch fixes the test by putting it within a loop and jumping, via
setcontext, just prior to the loop in case it segfaults. This way we get the
desired behavior (run the test COUNT_MAX times) regardless if it segfaults or
not. This also reduces the use of setcontext for control flow logic, keeping it
only in the segfault handler.

Also, since 'count' is changed within the signal handler, it is declared as
volatile to prevent any compiler optimization getting confused with
asynchronous changes.

Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
---
 .../powerpc/tm/tm-signal-context-force-tm.c   | 79 +++++++++----------
 1 file changed, 37 insertions(+), 42 deletions(-)

On 01/16/2020 07:05 PM, Gustavo Luiz Duarte wrote:
> The test case tm-signal-context-force-tm expects a segfault to happen on
> returning from signal handler, and then does a setcontext() to run the test
> again. However, the test doesn't always segfault, causing the test to run a
> single time.
> 
> This patch fixes the test by putting it within a loop and jumping, via
> setcontext, just prior to the loop in case it segfaults. This way we get the
> desired behavior (run the test COUNT_MAX times) regardless if it segfaults or
> not. This also reduces the use of setcontext for control flow logic, keeping it
> only in the segfault handler.
> 
> Also, since 'count' is changed within the signal handler, it is declared as
> volatile to prevent any compiler optimization getting confused with
> asynchronous changes.
> 
> Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
> ---
>   .../powerpc/tm/tm-signal-context-force-tm.c   | 79 +++++++++----------
>   1 file changed, 37 insertions(+), 42 deletions(-)
> 
> diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
> index 31717625f318..9ff7bdb6d47a 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
> @@ -42,9 +42,10 @@
>   #endif
>   
>   /* Setting contexts because the test will crash and we want to recover */
> -ucontext_t init_context, main_context;
> +ucontext_t init_context;
>   
> -static int count, first_time;
> +/* count is changed in the signal handler, so it must be volatile */
> +static volatile int count;
>   
>   void usr_signal_handler(int signo, siginfo_t *si, void *uc)
>   {
> @@ -98,11 +99,6 @@ void usr_signal_handler(int signo, siginfo_t *si, void *uc)
>   
>   void seg_signal_handler(int signo, siginfo_t *si, void *uc)
>   {
> -	if (count == COUNT_MAX) {
> -		/* Return to tm_signal_force_msr() and exit */
> -		setcontext(&main_context);
> -	}
> -
>   	count++;
>   
>   	/* Reexecute the test */
> @@ -126,37 +122,40 @@ void tm_trap_test(void)
>   	 */
>   	getcontext(&init_context);
>   
> -	/* Allocated an alternative signal stack area */
> -	ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
> -			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> -	ss.ss_size = SIGSTKSZ;
> -	ss.ss_flags = 0;
> -
> -	if (ss.ss_sp == (void *)-1) {
> -		perror("mmap error\n");
> -		exit(-1);
> -	}
> -
> -	/* Force the allocation through a page fault */
> -	if (madvise(ss.ss_sp, SIGSTKSZ, MADV_DONTNEED)) {
> -		perror("madvise\n");
> -		exit(-1);
> -	}
> -
> -	/* Setting an alternative stack to generate a page fault when
> -	 * the signal is raised.
> -	 */
> -	if (sigaltstack(&ss, NULL)) {
> -		perror("sigaltstack\n");
> -		exit(-1);
> +	while (count < COUNT_MAX) {
> +		/* Allocated an alternative signal stack area */
> +		ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
> +				MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> +		ss.ss_size = SIGSTKSZ;
> +		ss.ss_flags = 0;
> +
> +		if (ss.ss_sp == (void *)-1) {
> +			perror("mmap error\n");
> +			exit(-1);
> +		}
> +
> +		/* Force the allocation through a page fault */
> +		if (madvise(ss.ss_sp, SIGSTKSZ, MADV_DONTNEED)) {
> +			perror("madvise\n");
> +			exit(-1);
> +		}
> +
> +		/* Setting an alternative stack to generate a page fault when
> +		 * the signal is raised.
> +		 */
> +		if (sigaltstack(&ss, NULL)) {
> +			perror("sigaltstack\n");
> +			exit(-1);
> +		}
> +
> +		/* The signal handler will enable MSR_TS */
> +		sigaction(SIGUSR1, &usr_sa, NULL);
> +		/* If it does not crash, it might segfault, avoid it to retest */
> +		sigaction(SIGSEGV, &seg_sa, NULL);
> +
> +		raise(SIGUSR1);
> +		count++;
>   	}
> -
> -	/* The signal handler will enable MSR_TS */
> -	sigaction(SIGUSR1, &usr_sa, NULL);
> -	/* If it does not crash, it will segfault, avoid it to retest */
> -	sigaction(SIGSEGV, &seg_sa, NULL);
> -
> -	raise(SIGUSR1);
>   }
>   
>   int tm_signal_context_force_tm(void)
> @@ -169,11 +168,7 @@ int tm_signal_context_force_tm(void)
>   	 */
>   	SKIP_IF(!is_ppc64le());
>   
> -	/* Will get back here after COUNT_MAX interactions */
> -	getcontext(&main_context);
> -
> -	if (!first_time++)
> -		tm_trap_test();
> +	tm_trap_test();
>   
>   	return EXIT_SUCCESS;
>   }
> 

Reviewed-by: Gustavo Romero <gromero@linux.ibm.com>


Best regards,
Gustavo