Re: [PATCH][BZ #15921] Mark success return value of tst-cleanup2 as volatile

[Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>]

Hi Siddhesh,

This patch is fine, thanks.

On 02-09-2013 10:00, Siddhesh Poyarekar wrote:
> Hi,
>
> The test case nptl/tst-cleanup2 fails on s390x and power6 due to
> instruction sheduling in gcc.  This was reported in gcc:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58034
>
> but it was concluded that gcc is allowed to assume that the first
> argument to sprintf is a character array - NULL not being a character
> array.  I have also verified that -fno-builtin-sprintf does not help
> either, nor does -fnon-call-exceptions help.  -fno-schedule-insns
> obviously helps, but I went with a documented hack in the test case
> source rather than a documented hack in the Makefile.
>
> This can be trivially fixed by marking the zero return value as
> volatile so that gcc reloads it before return.  Tested that it fixes
> the test case on s390x and power6 and also verified that it does not
> cause any regressions on x86 (i686 and x86_64).  OK to commit?
>
> Siddhesh
>
> 	[BZ #15921]
> 	* tst-cleanup2.c (do_test): New volatile variable RET to
> 	return success.
>
> diff --git a/nptl/tst-cleanup2.c b/nptl/tst-cleanup2.c
> index 5bd1609..65af0f2 100644
> --- a/nptl/tst-cleanup2.c
> +++ b/nptl/tst-cleanup2.c
> @@ -34,6 +34,12 @@ static int
>  do_test (void)
>  {
>    char *p = NULL;
> +  /* gcc can overwrite the success written value by scheduling instructions
> +     around sprintf.  It is allowed to do this since according to C99 the first
> +     argument of sprintf is a character array and NULL is not a valid character
> +     array.  Mark the return value as volatile so that it gets reloaded on
> +     return.  */
> +  volatile int ret = 0;
>    struct sigaction sa;
>
>    sa.sa_handler = sig_handler;
> @@ -50,7 +56,7 @@ do_test (void)
>    if (setjmp (jmpbuf))
>      {
>        puts ("Exiting main...");
> -      return 0;
> +      return ret;
>      }
>
>    sprintf (p, "This should segv\n");
>