[v4] i386: Use pthread_barrier for synchronization on tst-bz21269

  To improve the false negative in patchwork buildbot.

Checked on i686-linux-gnu and I also checked by reverting the original
bz21269 fix to pass a bogus sa_restore value (and thus triggering the
test failure).
---
 sysdeps/unix/sysv/linux/i386/tst-bz21269.c | 62 +++++++---------------
 1 file changed, 19 insertions(+), 43 deletions(-)
  

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>  static void *
>  threadproc (void *ctx)
>  {
> -  while (1)
> +  for (int i = 0; i < NITER; i++)
>      {
> -      futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0);
> -      while (atomic_load (&ftx) != 2)
> -	{
> -	  if (atomic_load (&ftx) >= 3)
> -	    return NULL;
> -	}
> +      xpthread_barrier_wait (&barrier);
>  
>        /* clear LDT entry 0.  */
>        const struct user_desc desc = { 0 };
>        xmodify_ldt (1, &desc, sizeof (desc));
>  
> -      /* If ftx == 2, set it to zero,  If ftx == 100, quit.  */
> -      if (atomic_fetch_add (&ftx, -2) != 2)
> -	return NULL;
> +      /* Wait for 'ss' set in main thread.  */
> +      xpthread_barrier_wait (&barrier);
>      }
> +
> +  return NULL;
>  }
>  
>  
> @@ -180,20 +163,21 @@ do_test (void)
> -  for (int i = 0; i < 5; i++)
> +  for (int i = 0; i < NITER; i++)
>      {
>        if (sigsetjmp (jmpbuf, 1) != 0)
>  	continue;
>  
>        /* Make sure the thread is ready after the last test. */
> -      while (atomic_load (&ftx) != 0)
> -	;
> +      xpthread_barrier_wait (&barrier);
>  
> -      struct user_desc desc = {
> +      const struct user_desc desc = {
>  	.entry_number       = 0,
>  	.base_addr          = 0,
>  	.limit              = 0xffff,
> @@ -207,28 +191,20 @@ do_test (void)
>  
>        xmodify_ldt (0x11, &desc, sizeof (desc));
>  
> -      /* Arm the thread.  */
> -      ftx = 1;
> -      futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
> +      /* Wait thread clear LDT entry 0.  */
> +      xpthread_barrier_wait (&barrier);
>  
>        asm volatile ("mov %0, %%ss" : : "r" (0x7));
>  
> -      /* Fire up thread modify_ldt call.  */
> -      atomic_store (&ftx, 2);
> -
> -      while (atomic_load (&ftx) != 0)
> -	;
> -
>        /* On success, modify_ldt will segfault us synchronously and we will
>  	 escape via siglongjmp.  */
>        support_record_failure ();
>      }

this does...

child		parent
-----		------

<nothing>	setjmp

barrier	-------	barrier

clear LDT	set LDT		<-- these happen at the "same time"

barrier	-------	barrier

<nothing>	set ss
		support_record_failure (which hopefully segfaults before
			recording failure


IIRC what's supposed to happen (based on the original test) is:

<nothing>	setjmp

barrier	-------	barrier

		set LDT
<nothing>	set ss

 -- do some syscall --

clear LDT	-- longjmp


When the test fails, it does this:
		setjmp
		set LDT
		set ss
- task switch -
segfaults

Part of me wonders if the taken longjmp causes one of the barriers to be
skipped, putting the test out of sync.  If so, we won't be able to use
barriers this way.

Ok so I did some testing on the original test case, and it seems that
the first syscall after setting %ss causes the signal, handler, and
longjmp.  Which means two things:

1. The test must have some sort of forced syscall (INT3) or other
   stack-using operation after setting %ss, to trigger the test, and

2. At that point, the parent longjumps to the top of the loop

I'm not sure how we can satisfy both of these with pthread barriers,
since the one on the parent side would fault and not sync with the child
side.

Atomics do no syscalls so do not care if %ss is bogus.

However, I also found a bug in the original code:

      futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);

Per the manual, this wakes up at most zero threads.  The child thread
avoids this because ftx is never set right and it's FUTEX_WAIT always
returns with EAGAIN - except when it is right, and hangs.

/me is still investigating...
  

On 01/03/23 18:37, DJ Delorie wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>  static void *
>>  threadproc (void *ctx)
>>  {
>> -  while (1)
>> +  for (int i = 0; i < NITER; i++)
>>      {
>> -      futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0);
>> -      while (atomic_load (&ftx) != 2)
>> -	{
>> -	  if (atomic_load (&ftx) >= 3)
>> -	    return NULL;
>> -	}
>> +      xpthread_barrier_wait (&barrier);
>>  
>>        /* clear LDT entry 0.  */
>>        const struct user_desc desc = { 0 };
>>        xmodify_ldt (1, &desc, sizeof (desc));
>>  
>> -      /* If ftx == 2, set it to zero,  If ftx == 100, quit.  */
>> -      if (atomic_fetch_add (&ftx, -2) != 2)
>> -	return NULL;
>> +      /* Wait for 'ss' set in main thread.  */
>> +      xpthread_barrier_wait (&barrier);
>>      }
>> +
>> +  return NULL;
>>  }
>>  
>>  
>> @@ -180,20 +163,21 @@ do_test (void)
>> -  for (int i = 0; i < 5; i++)
>> +  for (int i = 0; i < NITER; i++)
>>      {
>>        if (sigsetjmp (jmpbuf, 1) != 0)
>>  	continue;
>>  
>>        /* Make sure the thread is ready after the last test. */
>> -      while (atomic_load (&ftx) != 0)
>> -	;
>> +      xpthread_barrier_wait (&barrier);
>>  
>> -      struct user_desc desc = {
>> +      const struct user_desc desc = {
>>  	.entry_number       = 0,
>>  	.base_addr          = 0,
>>  	.limit              = 0xffff,
>> @@ -207,28 +191,20 @@ do_test (void)
>>  
>>        xmodify_ldt (0x11, &desc, sizeof (desc));
>>  
>> -      /* Arm the thread.  */
>> -      ftx = 1;
>> -      futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
>> +      /* Wait thread clear LDT entry 0.  */
>> +      xpthread_barrier_wait (&barrier);
>>  
>>        asm volatile ("mov %0, %%ss" : : "r" (0x7));
>>  
>> -      /* Fire up thread modify_ldt call.  */
>> -      atomic_store (&ftx, 2);
>> -
>> -      while (atomic_load (&ftx) != 0)
>> -	;
>> -
>>        /* On success, modify_ldt will segfault us synchronously and we will
>>  	 escape via siglongjmp.  */
>>        support_record_failure ();
>>      }
> 
> this does...
> 
> child		parent
> -----		------
> 
> <nothing>	setjmp
> 
> barrier	-------	barrier
> 
> clear LDT	set LDT		<-- these happen at the "same time"

Yeah, ant is is clearly wrong...

> 
> barrier	-------	barrier
> 
> <nothing>	set ss
> 		support_record_failure (which hopefully segfaults before
> 			recording failure
> 
> 
> IIRC what's supposed to happen (based on the original test) is:
> 
> <nothing>	setjmp
> 
> barrier	-------	barrier
> 
> 		set LDT
> <nothing>	set ss
> 
>  -- do some syscall --
> 
> clear LDT	-- longjmp
> 
> 
> When the test fails, it does this:
> 		setjmp
> 		set LDT
> 		set ss
> - task switch -
> segfaults
> 
> Part of me wonders if the taken longjmp causes one of the barriers to be
> skipped, putting the test out of sync.  If so, we won't be able to use
> barriers this way.
> 
> Ok so I did some testing on the original test case, and it seems that
> the first syscall after setting %ss causes the signal, handler, and
> longjmp.  Which means two things:
> 
> 1. The test must have some sort of forced syscall (INT3) or other
>    stack-using operation after setting %ss, to trigger the test, and
> 
> 2. At that point, the parent longjumps to the top of the loop
> 
> I'm not sure how we can satisfy both of these with pthread barriers,
> since the one on the parent side would fault and not sync with the child
> side.
> 
> Atomics do no syscalls so do not care if %ss is bogus.

It makes sense, the problem is indeed the stack access that triggers
the expected failure.

> 
> However, I also found a bug in the original code:
> 
>       futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
> 
> Per the manual, this wakes up at most zero threads.  The child thread
> avoids this because ftx is never set right and it's FUTEX_WAIT always
> returns with EAGAIN - except when it is right, and hangs.
> 
> /me is still investigating...
> 

Another possibility I was wondering was to just remove this test altogether,
it is already covered by kernel testing code.