Move stack-related fields out of GC_thread to GC_stack_context_t

(refactoring)

Issue #362 (bdwgc).

* darwin_stop_world.c (GC_stack_range_for): Define and crtn local
variable; use crtn instead of p to access stack_ptr, topOfStack,
stack_end, altstack, altstack_size, normstack, normstack_size,
backing_store_end, backing_store_ptr.
* pthread_stop_world.c [!NACL] (GC_suspend_handler_inner,
GC_push_all_stacks): Likewise.
* pthread_support.c [!GC_NO_FINALIZATION] (GC_reset_finalizer_nested,
GC_check_finalizer_nested): Likewise.
* pthread_support.c [!GC_WIN32_THREADS] (GC_register_altstack):
Likewise.
* pthread_support.c (do_blocking_enter, do_blocking_leave,
GC_set_stackbottom, GC_get_my_stackbottom, GC_call_with_gc_active):
Likewise.
* win32_threads.c (GC_push_stack_for, GC_get_next_stack): Likewise.
* darwin_stop_world.c (GC_push_all_stacks): Use p->crtn instead of p
to access traced_stack_sect.
* win32_threads.c (GC_suspend, GC_stop_world, GC_start_world):
Likewise.
* include/private/pthread_support.h (GC_StackContext_Rep): New struct
type (move dummy, stack_end, stack_ptr, last_stack_min,
initial_stack_base, topOfStack, backing_store_end, backing_store_ptr,
altstack, altstack_size, normstack, normstack_size, finalizer_nested,
finalizer_skipped, traced_stack_sect from GC_Thread_Rep).
* include/private/pthread_support.h [!GC_NO_THREADS_DISCOVERY
&& GC_WIN32_THREADS] (GC_StackContext_Rep.stack_end): Add volatile.
* include/private/pthread_support.h [!GC_NO_FINALIZATION]
(GC_StackContext_Rep.fnlz_pad): New field.
* include/private/pthread_support.h (GC_stack_context_t): New type.
* include/private/pthread_support.h (GC_Thread_Rep.crtn,
GC_Thread_Rep.flags_pad): New field.
* include/private/pthread_support.h [GC_NO_FINALIZATION]
(GC_Thread_Rep.no_fnlz_pad): Remove field.
* include/private/pthread_support.h (GC_threads): Move (and refine)
comment from GC_Thread_Rep.
* include/private/pthread_support.h [GC_WIN32_THREADS]
(GC_record_stack_base): Change me argument to crtn.
* pthread_stop_world.c [!NACL] (GC_store_stack_ptr): Likewise.
* pthread_support.c (GC_record_stack_base): Likewise.
* pthread_stop_world.c [NACL] (NACL_STORE_REGS, nacl_pre_syscall_hook,
__nacl_suspend_thread_if_needed): Use p->crtn instead of p to access
stack_ptr.
* pthread_support.c (first_crtn): New static variable.
* pthread_support.c (first_thread): Update comment.
* pthread_support.c (first_thread_used): Remove variable.
* pthread_support.c (GC_push_thread_structures): Push
first_thread.crtn symbol
* pthread_support.c (GC_push_thread_structures): Push
first_crtn.backing_store_end instead of that in first_thread.
* pthread_support.c [MPROTECT_VDB && GC_WIN32_THREADS]
(GC_win32_unprotect_thread): Call GC_remove_protection() for t->crtn.
* pthread_support.c (GC_new_thread): Use first_thread.crtn!=0 instead
of first_thread_used; set first_thread.crtn to &first_crtn; allocate
GC_StackContext_Rep object using GC_INTERNAL_MALLOC() and store the
pointer to result->crtn.
* pthread_support.c [CPPCHECK] (GC_new_thread): Call GC_noop1() for
first_thread.flags_pad, for first_crtn.dummy instead of result->dummy,
and for first_crtn.fnlz_pad instead of result->no_fnlz_pad.
* pthread_support.c (GC_delete_thread): Call GC_INTERNAL_FREE(p->crtn)
along with that for p.
* pthread_support.c [CAN_HANDLE_FORK && (!THREAD_SANITIZER
|| !CAN_CALL_ATFORK)] (GC_remove_all_threads_but_me): Likewise.
* pthread_support.c [GC_PTHREADS] (GC_pthread_join, GC_pthread_detach):
Likewise.
* pthread_support.c (GC_segment_is_thread_stack,
GC_greatest_stack_base_below): Use p->crtn instead of p to access
stack_end.
* pthread_support.c (GC_call_with_gc_active): Move assertions about me
fields to be after LOCK; add assertion that me->crtn==crtn.
* win32_threads.c (dll_thread_table): Make it static.
* win32_threads.c [!GC_NO_THREADS_DISCOVERY] (dll_crtn_table): New
static variable.
* win32_threads.c (GC_register_my_thread_inner): Reformat comment;
set me->crtn.
* win32_threads.c (GC_push_stack_for): Define stack_end local variable;
immediately return (zero) if stack_end is NULL.
* win32_threads.c (GC_push_all_stacks): Call GC_push_stack_for() (and
increment nthreads) even if stack_end is NULL.
* win32_threads.c (GC_get_next_stack): Rename s local variable to
stack_end.

1 files changed, 68 insertions, 60 deletions

diff --git a/win32_threads.c b/win32_threads.c
index c60a2592..c5eb9a24 100644
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -158,7 +158,10 @@ GC_API void GC_CALL GC_use_threads_discovery(void)
 
 /* Things may get quite slow for large numbers of threads,      */
 /* since we look them up with sequential search.                */
-volatile struct GC_Thread_Rep dll_thread_table[MAX_THREADS];
+static volatile struct GC_Thread_Rep dll_thread_table[MAX_THREADS];
+#ifndef GC_NO_THREADS_DISCOVERY
+  static struct GC_StackContext_Rep dll_crtn_table[MAX_THREADS];
+#endif
 
 STATIC volatile LONG GC_max_thread_index = 0;
                         /* Largest index in dll_thread_table    */
@@ -216,9 +219,9 @@ GC_INNER GC_thread GC_register_my_thread_inner(const struct GC_stack_base *sb,
       }
       /* Update GC_max_thread_index if necessary.  The following is     */
       /* safe, and unlike CompareExchange-based solutions seems to work */
-      /* on all Windows95 and later platforms.                          */
-      /* Unfortunately, GC_max_thread_index may be temporarily out of   */
-      /* bounds, so readers have to compensate.                         */
+      /* on all Windows 95 and later platforms.  Unfortunately,         */
+      /* GC_max_thread_index may be temporarily out of bounds, so       */
+      /* readers have to compensate.                                    */
       while (i > GC_max_thread_index) {
         InterlockedIncrement((LONG *)&GC_max_thread_index);
                 /* Cast away volatile for older versions of Win32 headers. */
@@ -229,6 +232,7 @@ GC_INNER GC_thread GC_register_my_thread_inner(const struct GC_stack_base *sb,
         GC_max_thread_index = MAX_THREADS - 1;
       }
       me = (GC_thread)(dll_thread_table + i);
+      me -> crtn = &dll_crtn_table[i];
     } else
 # endif
   /* else */ /* Not using DllMain */ {
@@ -253,8 +257,8 @@ GC_INNER GC_thread GC_register_my_thread_inner(const struct GC_stack_base *sb,
                    ": errcode= 0x%X", (unsigned)GetLastError());
     }
 # endif
-  me -> last_stack_min = ADDR_LIMIT;
-  GC_record_stack_base(me, sb);
+  me -> crtn -> last_stack_min = ADDR_LIMIT;
+  GC_record_stack_base(me -> crtn, sb);
   /* Up until this point, GC_push_all_stacks considers this thread      */
   /* invalid.                                                           */
   /* Up until this point, this entry is viewed as reserved but invalid  */
@@ -421,7 +425,7 @@ STATIC void GC_suspend(GC_thread t)
           && exitCode != STILL_ACTIVE) {
         GC_release_dirty_lock();
 #       ifdef GC_PTHREADS
-          t -> stack_end = NULL; /* prevent stack from being pushed */
+          t -> crtn -> stack_end = NULL; /* prevent stack from being pushed */
 #       else
           /* This breaks pthread_join on Cygwin, which is guaranteed to */
           /* only see user threads.                                     */
@@ -459,7 +463,7 @@ STATIC void GC_suspend(GC_thread t)
         && exitCode != STILL_ACTIVE) {
       GC_release_dirty_lock();
 #     ifdef GC_PTHREADS
-        t -> stack_end = NULL; /* prevent stack from being pushed */
+        t -> crtn -> stack_end = NULL; /* prevent stack from being pushed */
 #     else
         GC_ASSERT(GC_win32_dll_threads);
         GC_delete_gc_thread_no_free(t);
@@ -523,7 +527,7 @@ GC_INNER void GC_stop_world(void)
       for (i = 0; i <= my_max; i++) {
         GC_thread p = (GC_thread)(dll_thread_table + i);
 
-        if (p -> stack_end != NULL && (p -> flags & DO_BLOCKING) == 0
+        if (p -> crtn -> stack_end != NULL && (p -> flags & DO_BLOCKING) == 0
             && p -> id != self_id) {
           GC_suspend(p);
         }
@@ -536,7 +540,7 @@ GC_INNER void GC_stop_world(void)
 
     for (i = 0; i < THREAD_TABLE_SZ; i++) {
       for (p = GC_threads[i]; p != NULL; p = p -> tm.next)
-        if (p -> stack_end != NULL && p -> id != self_id
+        if (p -> crtn -> stack_end != NULL && p -> id != self_id
             && (p -> flags & (FINISHED | DO_BLOCKING)) == 0)
           GC_suspend(p);
     }
@@ -571,7 +575,7 @@ GC_INNER void GC_start_world(void)
 #       ifdef DEBUG_THREADS
           GC_log_printf("Resuming 0x%x\n", (int)p->id);
 #       endif
-        GC_ASSERT(p -> stack_end != NULL && p -> id != self_id);
+        GC_ASSERT(p -> crtn -> stack_end != NULL && p -> id != self_id);
         if (ResumeThread(THREAD_HANDLE(p)) == (DWORD)-1)
           ABORT("ResumeThread failed");
         p -> flags &= ~IS_SUSPENDED;
@@ -590,7 +594,7 @@ GC_INNER void GC_start_world(void)
 #         ifdef DEBUG_THREADS
             GC_log_printf("Resuming 0x%x\n", (int)p->id);
 #         endif
-          GC_ASSERT(p -> stack_end != NULL && p -> id != self_id);
+          GC_ASSERT(p -> crtn -> stack_end != NULL && p -> id != self_id);
           if (ResumeThread(THREAD_HANDLE(p)) == (DWORD)-1)
             ABORT("ResumeThread failed");
           GC_win32_unprotect_thread(p);
@@ -734,8 +738,12 @@ STATIC word GC_push_stack_for(GC_thread thread, thread_id_t self_id,
 {
   GC_bool is_self = FALSE;
   ptr_t sp, stack_min;
-  struct GC_traced_stack_sect_s *traced_stack_sect =
-                                      thread -> traced_stack_sect;
+  GC_stack_context_t crtn = thread -> crtn;
+  ptr_t stack_end = crtn -> stack_end;
+  struct GC_traced_stack_sect_s *traced_stack_sect = crtn -> traced_stack_sect;
+
+  if (EXPECT(NULL == stack_end, FALSE)) return 0;
+
   if (thread -> id == self_id) {
     GC_ASSERT((thread -> flags & DO_BLOCKING) == 0);
     sp = GC_approx_sp();
@@ -743,7 +751,7 @@ STATIC word GC_push_stack_for(GC_thread thread, thread_id_t self_id,
     *pfound_me = TRUE;
   } else if ((thread -> flags & DO_BLOCKING) != 0) {
     /* Use saved sp value for blocked threads.  */
-    sp = thread -> stack_ptr;
+    sp = crtn -> stack_ptr;
   } else {
 #   ifdef RETRY_GET_THREAD_CONTEXT
       /* We cache context when suspending the thread since it may       */
@@ -767,7 +775,7 @@ STATIC word GC_push_stack_for(GC_thread thread, thread_id_t self_id,
         } else {
 #         ifdef RETRY_GET_THREAD_CONTEXT
             /* At least, try to use the stale context if saved. */
-            sp = thread->context_sp;
+            sp = thread -> context_sp;
             if (NULL == sp) {
               /* Skip the current thread, anyway its stack will */
               /* be pushed when the world is stopped.           */
@@ -810,12 +818,11 @@ STATIC word GC_push_stack_for(GC_thread thread, thread_id_t self_id,
             GC_log_printf("TIB stack limit/base: %p .. %p\n",
                           (void *)tib->StackLimit, (void *)tib->StackBase);
 #         endif
-          GC_ASSERT(!((word)(thread -> stack_end)
-                      COOLER_THAN (word)tib->StackBase));
-          if (thread -> stack_end != thread -> initial_stack_base
-              /* We are in a coroutine. */
-              && ((word)(thread -> stack_end) <= (word)tib->StackLimit
-                  || (word)tib->StackBase < (word)(thread -> stack_end))) {
+          GC_ASSERT(!((word)stack_end COOLER_THAN (word)tib->StackBase));
+          if (stack_end != crtn -> initial_stack_base
+              /* We are in a coroutine (old-style way of the support).  */
+              && ((word)stack_end <= (word)tib->StackLimit
+                  || (word)tib->StackBase < (word)stack_end)) {
             /* The coroutine stack is not within TIB stack.   */
             WARN("GetThreadContext might return stale register values"
                  " including ESP= %p\n", sp);
@@ -850,80 +857,78 @@ STATIC word GC_push_stack_for(GC_thread thread, thread_id_t self_id,
   /* or to an address in the thread stack no larger than sp,    */
   /* taking advantage of the old value to avoid slow traversals */
   /* of large stacks.                                           */
-  if (thread -> last_stack_min == ADDR_LIMIT) {
+  if (crtn -> last_stack_min == ADDR_LIMIT) {
 #   ifdef MSWINCE
       if (GC_dont_query_stack_min) {
         stack_min = GC_wince_evaluate_stack_min(traced_stack_sect != NULL ?
-                      (ptr_t)traced_stack_sect : thread -> stack_end);
+                                        (ptr_t)traced_stack_sect : stack_end);
         /* Keep last_stack_min value unmodified. */
       } else
 #   endif
     /* else */ {
       stack_min = GC_get_stack_min(traced_stack_sect != NULL ?
-                      (ptr_t)traced_stack_sect : thread -> stack_end);
+                                        (ptr_t)traced_stack_sect : stack_end);
       GC_win32_unprotect_thread(thread);
-      thread -> last_stack_min = stack_min;
+      crtn -> last_stack_min = stack_min;
     }
   } else {
     /* First, adjust the latest known minimum stack address if we       */
     /* are inside GC_call_with_gc_active().                             */
     if (traced_stack_sect != NULL &&
-        (word)thread->last_stack_min > (word)traced_stack_sect) {
+        (word)(crtn -> last_stack_min) > (word)traced_stack_sect) {
       GC_win32_unprotect_thread(thread);
-      thread -> last_stack_min = (ptr_t)traced_stack_sect;
+      crtn -> last_stack_min = (ptr_t)traced_stack_sect;
     }
 
-    if ((word)sp < (word)(thread -> stack_end)
-        && (word)sp >= (word)thread->last_stack_min) {
+    if ((word)sp < (word)stack_end
+        && (word)sp >= (word)(crtn -> last_stack_min)) {
       stack_min = sp;
     } else {
       /* In the current thread it is always safe to use sp value.       */
-      if (may_be_in_stack(is_self && (word)sp < (word)thread->last_stack_min ?
-                          sp : thread -> last_stack_min)) {
+      if (may_be_in_stack(is_self && (word)sp < (word)(crtn -> last_stack_min)
+                            ? sp : crtn -> last_stack_min)) {
         stack_min = (ptr_t)last_info.BaseAddress;
         /* Do not probe rest of the stack if sp is correct. */
-        if ((word)sp < (word)stack_min
-            || (word)sp >= (word)(thread -> stack_end))
-          stack_min = GC_get_stack_min(thread -> last_stack_min);
+        if ((word)sp < (word)stack_min || (word)sp >= (word)stack_end)
+          stack_min = GC_get_stack_min(crtn -> last_stack_min);
       } else {
         /* Stack shrunk?  Is this possible? */
-        stack_min = GC_get_stack_min(thread -> stack_end);
+        stack_min = GC_get_stack_min(stack_end);
       }
       GC_win32_unprotect_thread(thread);
-      thread -> last_stack_min = stack_min;
+      crtn -> last_stack_min = stack_min;
     }
   }
 
   GC_ASSERT(GC_dont_query_stack_min
-            || stack_min == GC_get_stack_min(thread -> stack_end)
+            || stack_min == GC_get_stack_min(stack_end)
             || ((word)sp >= (word)stack_min
-                && (word)stack_min < (word)(thread -> stack_end)
-                && (word)stack_min
-                        > (word)GC_get_stack_min(thread -> stack_end)));
+                && (word)stack_min < (word)stack_end
+                && (word)stack_min > (word)GC_get_stack_min(stack_end)));
 
-  if ((word)sp >= (word)stack_min && (word)sp < (word)(thread -> stack_end)) {
+  if ((word)sp >= (word)stack_min && (word)sp < (word)stack_end) {
 #   ifdef DEBUG_THREADS
       GC_log_printf("Pushing stack for 0x%x from sp %p to %p from 0x%x\n",
-                    (int)thread->id, (void *)sp,
-                    (void *)(thread -> stack_end), (int)self_id);
+                    (int)(thread -> id), (void *)sp, (void *)stack_end,
+                    (int)self_id);
 #   endif
-    GC_push_all_stack_sections(sp, thread -> stack_end, traced_stack_sect);
+    GC_push_all_stack_sections(sp, stack_end, traced_stack_sect);
   } else {
     /* If not current thread then it is possible for sp to point to     */
     /* the guarded (untouched yet) page just below the current          */
     /* stack_min of the thread.                                         */
-    if (is_self || (word)sp >= (word)(thread -> stack_end)
+    if (is_self || (word)sp >= (word)stack_end
         || (word)(sp + GC_page_size) < (word)stack_min)
       WARN("Thread stack pointer %p out of range, pushing everything\n", sp);
 #   ifdef DEBUG_THREADS
       GC_log_printf("Pushing stack for 0x%x from (min) %p to %p from 0x%x\n",
-                    (int)thread->id, (void *)stack_min,
-                    (void *)(thread -> stack_end), (int)self_id);
+                    (int)(thread -> id), (void *)stack_min, (void *)stack_end,
+                    (int)self_id);
 #   endif
     /* Push everything - ignore "traced stack section" data.            */
-    GC_push_all_stack(stack_min, thread -> stack_end);
+    GC_push_all_stack(stack_min, stack_end);
   }
-  return thread -> stack_end - sp; /* stack grows down */
+  return stack_end - sp; /* stack grows down */
 }
 
 /* Should do exactly the right thing if the world is stopped; should    */
@@ -947,7 +952,7 @@ GC_INNER void GC_push_all_stacks(void)
       for (i = 0; i <= my_max; i++) {
         GC_thread p = (GC_thread)(dll_thread_table + i);
 
-        if (p -> tm.in_use && p -> stack_end != NULL) {
+        if (p -> tm.in_use) {
 #         ifndef SMALL_CONFIG
             ++nthreads;
 #         endif
@@ -962,7 +967,7 @@ GC_INNER void GC_push_all_stacks(void)
       GC_thread p;
 
       for (p = GC_threads[i]; p != NULL; p = p -> tm.next)
-        if (!KNOWN_FINISHED(p) && p -> stack_end != NULL) {
+        if (!KNOWN_FINISHED(p)) {
 #         ifndef SMALL_CONFIG
             ++nthreads;
 #         endif
@@ -1010,13 +1015,14 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
     LONG my_max = GC_get_max_thread_index();
 
     for (i = 0; i <= my_max; i++) {
-      ptr_t s = (ptr_t)dll_thread_table[i].stack_end;
+      ptr_t stack_end = (ptr_t)dll_thread_table[i].crtn -> stack_end;
 
-      if ((word)s > (word)start && (word)s < (word)current_min) {
+      if ((word)stack_end > (word)start
+          && (word)stack_end < (word)current_min) {
         /* Update address of last_stack_min. */
         plast_stack_min = (ptr_t * /* no volatile */)
-                            &dll_thread_table[i].last_stack_min;
-        current_min = s;
+                            &(dll_thread_table[i].crtn -> last_stack_min);
+        current_min = stack_end;
 #       ifdef CPPCHECK
           /* To avoid a warning that thread is always null.     */
           thread = (GC_thread)&dll_thread_table[i];
@@ -1028,13 +1034,15 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       GC_thread p;
 
       for (p = GC_threads[i]; p != NULL; p = p -> tm.next) {
-        ptr_t s = p -> stack_end;
+        GC_stack_context_t crtn = p -> crtn;
+        ptr_t stack_end = crtn -> stack_end; /* read of a volatile field */
 
-        if ((word)s > (word)start && (word)s < (word)current_min) {
+        if ((word)stack_end > (word)start
+            && (word)stack_end < (word)current_min) {
           /* Update address of last_stack_min. */
-          plast_stack_min = &(p -> last_stack_min);
+          plast_stack_min = &(crtn -> last_stack_min);
           thread = p; /* Remember current thread to unprotect. */
-          current_min = s;
+          current_min = stack_end;
         }
       }
     }