[PATCH] Fix false positives in tst-pthread-getattr test case

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Hi,

Here's a patch that fixes the test case to remove the false-positives
most of us were getting in the result. Like I said in an earlier
thread, it does not mean that the bug has not been fixed -- just that
the test case was wrong.

In some cases, the compiler would optimize out the call to
allocate_and_test and thus result in a false positive for the test
case. Another problem was the fact that the compiler could move the
stack pointer beyond what we expect, resulting in the alloca call moving
the stack pointer beyond what is allowed by the rlimit. Hence,
accessing the stackaddr returned by pthread_getattr_np is safer than
relying on the alloca'd result.

The problem that Carlos reported still remains. I have seen this crash
occasionally after fixing the above and here's what is happening in my
case:

* The rlim_cur is returned as unlimited (-1)
* The test case sets the rlimit to the distance to the previous vma,
  which is quite large
* Since most systems are set with vm.overcommit_memory sysctl set to 0
  (heuristic overcommit), the heuristic fails for such a large value
  and hence results in a segfault.

Setting vm.overcommit_memory to 1 (always overcommit) fixes this crash.

As for where RLIMIT_STACK is being set as unlimited, it is definitely
not anything within glibc. The kernel, shell or make may be to blame
for it, the kernel being the most likely culprit.

Regards,
Siddhesh

nptl/ChangeLog:

2012-06-26  Siddhesh poyarekar  <siddhesh@redhat.com>

	[BZ #12416]
	* tst-pthread-getattr.c (allocate_and_test): Return STACKADDR.
	Access STACKADDR instead of MEM to test.
	(check_stack_top): Read valued written into STACKADDR in
	allocate_and_test.

diff --git a/nptl/tst-pthread-getattr.c b/nptl/tst-pthread-getattr.c
index 6f2cfc6..b035d05 100644
--- a/nptl/tst-pthread-getattr.c
+++ b/nptl/tst-pthread-getattr.c
@@ -23,16 +23,25 @@
 #include <sys/resource.h>
 #include <pthread.h>
 #include <alloca.h>
+#include <assert.h>
 
 /* Move the stack pointer so that stackaddr is accessible and then check if it
    really is accessible.  This will segfault if it fails.  */
-static void
+static void *
 allocate_and_test (void *stackaddr)
 {
   void *mem = &mem;
-  /* FIXME:  The difference will be negative for _STACK_GROWSUP.  */
+  /* FIXME:  mem >= stackaddr for _STACK_GROWSUP.  */
   mem = alloca ((size_t) (mem - stackaddr));
-  *(int *)(mem) = 0;
+  assert (mem <= stackaddr);
+
+  /* We don't access mem here because the compiler may move the stack pointer
+     beyond what we expect, thus making our alloca send the stack pointer
+     beyond stackaddr.  Using only stackaddr without the assert may make the
+     compiler think that this instruction is independent of the above alloca
+     and hence reshuffle to do this dereference before the alloca.  */
+  *(int *)stackaddr = 42;
+  return stackaddr;
 }
 
 static int
@@ -77,6 +86,8 @@ check_stack_top (void)
       return 1;
     }
 
+  printf ("current rlimit_stack is %zu\n", stack_limit.rlim_cur);
+
   if (get_self_pthread_attr ("check_stack_top", &stackaddr, &stacksize))
     return 1;
 
@@ -100,7 +111,10 @@ check_stack_top (void)
 
   printf ("Adjusted rlimit: stacksize=%zu, stackaddr=%p\n", stacksize,
           stackaddr);
-  allocate_and_test (stackaddr);
+
+  /* So that the compiler does not optimize out this call.  */
+  stackaddr = allocate_and_test (stackaddr);
+  assert (*(int *)stackaddr == 42);
 
   puts ("Stack top tests done");