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]

Re: [PATCH] Fix stacksize returned for main process thread when RLIMIT_STACKis relevant


(6/14/12 3:29 PM), Siddhesh Poyarekar wrote:
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.


Thanks! I have a few comment/questions.


@@ -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));

Hmm.. This comment is unclear to me. Which round up are you afraid? kernel always account vma size. And, when unaligned stacksize is happen? Both "to" and "stack_end" seems always page size aligned.


+  /* 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)))

This test is also unclear to me.


Why "if (stack_limit.rlim_cur > stacksize)" check is necessary? It is always true
because we have no way to create a process w/o auxv.

And if I understand correctly, this setrlimit() never change a stacksize because
current pthread_getattr_np() is always returned page aligned size.
i.e. roundup(N*pagesize - 4095) == N*pagesize.

I'm not sure why do we need to bother unaligned size case.

Finally, if we should care "the stack is limited by the vma below" case, we should
reduce a stack pagesize(), I think allocate_and_test() assume there is at least 1
unmapped (or PROT_NONE) region at neighbor of a stack.




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