This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix stacksize returned for main process thread whenRLIMIT_STACK is relevant


Hi,

This patch is a follow-up to the discussion:

http://sourceware.org/ml/libc-alpha/2012-06/msg00310.html

The stacksize returned by pthread_getattr_np is incorrect if the
stacksize computed is based on rlimit since it should be adjusted
against the pages we skip above __libc_stack_end. Further, if the
stacksize is based on rlimit, it should be truncated to align to page
size because the rlimit may not necessarily be page aligned.

I have added a test case that verifies that the stack can extend up to
the returned stackaddr for the main thread. Once the thread stack size
issue is fixed, an analogous check can be added in the same test case
for thread stacks too. I have verified that there are no regressions in
the testsuite resulting from this change.


Regards,
Siddhesh


nptl/ChangeLog:

2012-06-15  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* Makefile (tests): Add test case.
	* pthread_getattr_np.c (pthread_getattr_np): Deduct pages below
	the __libc_stack_end page from stacksize.  Truncate stacksize to
	make it page aligned when it is computed from RLIMIT_STACK.
	* tst-pthread-getattr.c: New test case. Verify that stackaddr is
	accessible.
diff --git a/nptl/Makefile b/nptl/Makefile
index 2a36d01..ef8e874 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -245,7 +245,7 @@ tests = tst-typesizes \
 	tst-exec1 tst-exec2 tst-exec3 tst-exec4 \
 	tst-exit1 tst-exit2 tst-exit3 \
 	tst-stdio1 tst-stdio2 \
-	tst-stack1 tst-stack2 tst-stack3 \
+	tst-stack1 tst-stack2 tst-stack3 tst-pthread-getattr \
 	tst-unload \
 	tst-dlsym1 \
 	tst-sysconf \
diff --git a/nptl/pthread_getattr_np.c b/nptl/pthread_getattr_np.c
index 75d717b..7309185 100644
--- a/nptl/pthread_getattr_np.c
+++ b/nptl/pthread_getattr_np.c
@@ -120,8 +120,15 @@ pthread_getattr_np (thread_id, attr)
 		      && (uintptr_t) __libc_stack_end < to)
 		    {
 		      /* Found the entry.  Now we have the info we need.  */
-		      iattr->stacksize = rl.rlim_cur;
 		      iattr->stackaddr = stack_end;
+		      iattr->stacksize =
+		        rl.rlim_cur - (size_t) (to - (uintptr_t) stack_end);
+
+		      /* Cut it down to align it to page size since otherwise we
+		         risk going beyond rlimit when the kernel rounds up the
+		         stack extension request.  */
+		      iattr->stacksize = (iattr->stacksize
+					  & -(intptr_t) GLRO(dl_pagesize));
 
 		      /* The limit might be too high.  */
 		      if ((size_t) iattr->stacksize
diff --git a/nptl/tst-pthread-getattr.c b/nptl/tst-pthread-getattr.c
new file mode 100644
index 0000000..853b24c
--- /dev/null
+++ b/nptl/tst-pthread-getattr.c
@@ -0,0 +1,104 @@
+/* Make sure that the stackaddr returned by pthread_getattr_np is
+   reachable.  */
+
+#include <stdio.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <pthread.h>
+#include <alloca.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
+allocate_and_test (void *stackaddr)
+{
+  void *mem = &mem;
+  mem = alloca ((size_t) (mem - stackaddr));
+  *(int *)(mem) = 0;
+}
+
+static int
+get_self_pthread_attr (const char *id, void **stackaddr, size_t *stacksize)
+{
+  pthread_attr_t attr;
+  int ret;
+  pthread_t me = pthread_self ();
+
+  if ((ret = pthread_getattr_np (me, &attr)))
+    {
+      printf ("%s: pthread_getattr_np failed: %s\n", id, strerror (ret));
+      return 1;
+    }
+
+  if ((ret = pthread_attr_getstack (&attr, stackaddr, stacksize)))
+    {
+      printf ("%s: pthread_attr_getstack returned error: %s\n", id,
+	      strerror (ret));
+      return 1;
+    }
+
+  return 0;
+}
+
+/* Verify that the stack size returned by pthread_getattr_np is usable when
+   the returned value is subject to rlimit.  */
+static int
+check_stack_top (void)
+{
+  struct rlimit stack_limit;
+  void *stackaddr;
+  size_t stacksize = 0;
+
+  puts ("Verifying that stack top is accessible");
+
+  int ret = getrlimit (RLIMIT_STACK, &stack_limit);
+  if (ret)
+    {
+      perror ("getrlimit failed");
+      return 1;
+    }
+
+  if (get_self_pthread_attr ("check_stack_top", &stackaddr, &stacksize))
+    return 1;
+
+  /* Reduce the rlimit to a page less that what is currently being returned so
+     that we ensure that pthread_getattr_np uses rlimit.  The figure is
+     intentionally unaligned so to verify that pthread_getattr_np returns an
+     aligned stacksize that correctly fits into the rlimit.  We don't bother
+     about the case where the stack is limited by the vma below it and not by
+     the rlimit because the stacksize returned in that case is computed from
+     the end of that vma and is hence safe.  */
+  if (stack_limit.rlim_cur > stacksize)
+    {
+      stack_limit.rlim_cur = stacksize - 4095;
+      printf ("Adjusting RLIMIT_STACK to %zu\n", stack_limit.rlim_cur);
+      if ((ret = setrlimit (RLIMIT_STACK, &stack_limit)))
+        {
+	  perror ("setrlimit failed");
+	  return 1;
+	}
+
+      if (get_self_pthread_attr ("check_stack_top2", &stackaddr, &stacksize))
+	return 1;
+
+      printf ("Adjusted rlimit: stacksize=%zu, stackaddr=%p\n", stacksize,
+	      stackaddr);
+      allocate_and_test (stackaddr);
+    }
+
+  puts ("Stack top tests done");
+
+  return 0;
+}
+
+/* TODO: Similar check for thread stacks once the thread stack sizes are
+   fixed.  */
+static int
+do_test (void)
+{
+  return check_stack_top ();
+}
+
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]