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

[Adhemerval Zanella Netto]

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.