[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: glibc 2.24 testsuite issues on hppa



On 2016-08-07 19:54, John David Anglin wrote:
> On 2016-08-05, at 6:23 PM, Aurelien Jarno wrote:
> 
> > I have looked at the testsuite results from the latest uploads, there
> > are 9 real regressions (ie not new tests) mostly related to NPTL:
> > 
> > * FAIL: nptl/tst-default-attr
> >    original exit status 22
> >    stacksize test
> >    guardsize test
> >    sched test
> >    tst-default-attr.c:101: pthread_create returned 22 (errno = 22)
> >    tst-default-attr.c:371: do_sched_test returned 22 (errno = 22)
> 
> This was probably introduced by the following change:
> 
> 2016-02-19  Carlos O'Donell  <carlos@systemhalted.org>
> 
>         * nptl/allocatestack.c (allocate_stack): Declare new stackaddr,
>         assign attr->stackaddr to it, and adjust it down when
>         _STACK_GROWS_UP.  Change all attr->stackaddr to stackaddr.
>         [_STACK_GROWS_UP]: Delete assert.
>         * nptl/pthread_create.c (START_THREAD_DEFN) [!_STACK_GROWS_DOWN]:
>         Implement stack grows up logic.
>         * nptl/pthread_getattr_np.c (pthread_getattr_np): Implement
>         stack grows up logic.
> 
> I presume that the stack grows up patch that we had before was removed.  We are getting a mprotect error in test:

Indeed that is correct. We have removed it in favor of the upstream
version.

> [pid 21422] mprotect(0xfce4a480, 4096, PROT_READ|PROT_WRITE|PROT_EXEC) = -1 EINVAL (Invalid argument)
> 
> It looks like the error comes from here in allocatestack.c:
> 
> #elif _STACK_GROWS_UP
>           if (mprotect ((char *) pd - pd->guardsize,
>                         pd->guardsize - guardsize, prot) != 0)
>             goto mprot_error;
> #endif

Looking more in details, there are small differences in the two
implementations, but the only one that changes the behaviour is due to
this missing hunk in the upstream version:

--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -683,9 +692,13 @@
 			prot) != 0)
 	    goto mprot_error;
 #elif _STACK_GROWS_UP
-	  if (mprotect ((char *) pd - pd->guardsize,
-			pd->guardsize - guardsize, prot) != 0)
-	    goto mprot_error;
+	  char *new_guard = (char *) (((uintptr_t) pd - guardsize) & ~pagesize_m1);
+	  char *old_guard = (char *) (((uintptr_t) pd - pd->guardsize) & ~pagesize_m1);
+	  /* The guard size difference might be > 0, but once rounded
+	     to the nearest page the size difference might be zero.  */
+	  if (old_guard - new_guard > 0)
+	    if (mprotect (old_guard, new_guard - old_guard, prot) != 0)
+	      goto mprot_error;
 #endif
 
 	  pd->guardsize = guardsize;

It therefore looks like this should also be upstreamed. We can add it
back in the debian version in the meantime.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net


Reply to: