8303852: current_stack_region() gets called twice unnecessarily

Reviewed-by: stuefe, pchilanomate

src/hotspot/os/aix/os_aix.cpp

@@ -2948,29 +2948,22 @@ void os::Aix::initialize_libperfstat() {
 /////////////////////////////////////////////////////////////////////////////
 // thread stack
 
-// Get the current stack base from the OS (actually, the pthread library).
+// Get the current stack base and size from the OS (actually, the pthread library).
-// Note: usually not page aligned.
+// Note: base usually not page aligned.
-address os::current_stack_base() {
-  AixMisc::stackbounds_t bounds;
-  bool rc = AixMisc::query_stack_bounds_for_current_thread(&bounds);
-  guarantee(rc, "Unable to retrieve stack bounds.");
-  return bounds.base;
-}
-
-// Get the current stack size from the OS (actually, the pthread library).
 // Returned size is such that (base - size) is always aligned to page size.
-size_t os::current_stack_size() {
+void os::current_stack_base_and_size(address* stack_base, size_t* stack_size) {
   AixMisc::stackbounds_t bounds;
   bool rc = AixMisc::query_stack_bounds_for_current_thread(&bounds);
   guarantee(rc, "Unable to retrieve stack bounds.");
-  // Align the returned stack size such that the stack low address
+  *stack_base = bounds.base;
+
+  // Align the reported stack size such that the stack low address
   // is aligned to page size (Note: base is usually not and we do not care).
   // We need to do this because caller code will assume stack low address is
   // page aligned and will place guard pages without checking.
   address low = bounds.base - bounds.size;
   address low_aligned = (address)align_up(low, os::vm_page_size());
-  size_t s = bounds.base - low_aligned;
+  *stack_size = bounds.base - low_aligned;
-  return s;
 }
 
 // Get the default path to the core file

src/hotspot/os/linux/os_linux.cpp

@@ -5370,19 +5370,22 @@ bool os::start_debugging(char *buf, int buflen) {
 //    |                        |/
 // P2 +------------------------+ Thread::stack_base()
 //
-// ** P1 (aka bottom) and size (P2 = P1 - size) are the address and stack size
+// ** P1 (aka bottom) and size are the address and stack size
 //    returned from pthread_attr_getstack().
+// ** P2 (aka stack top or base) = P1 + size
 // ** If adjustStackSizeForGuardPages() is true the guard pages have been taken
 //    out of the stack size given in pthread_attr. We work around this for
 //    threads created by the VM. We adjust bottom to be P1 and size accordingly.
 //
 #ifndef ZERO
-static void current_stack_region(address * bottom, size_t * size) {
+void os::current_stack_base_and_size(address* base, size_t* size) {
+  address bottom;
   if (os::is_primordial_thread()) {
     // primordial thread needs special handling because pthread_getattr_np()
     // may return bogus value.
-    *bottom = os::Linux::initial_thread_stack_bottom();
+    bottom = os::Linux::initial_thread_stack_bottom();
-    *size   = os::Linux::initial_thread_stack_size();
+    *size = os::Linux::initial_thread_stack_size();
+    *base = bottom + *size;
   } else {
     pthread_attr_t attr;
 
@@ -5397,42 +5400,28 @@ static void current_stack_region(address * bottom, size_t * size) {
       }
     }
 
-    if (pthread_attr_getstack(&attr, (void **)bottom, size) != 0) {
+    if (pthread_attr_getstack(&attr, (void **)&bottom, size) != 0) {
       fatal("Cannot locate current stack attributes!");
     }
 
+    *base = bottom + *size;
+
     if (os::Linux::adjustStackSizeForGuardPages()) {
       size_t guard_size = 0;
       rslt = pthread_attr_getguardsize(&attr, &guard_size);
       if (rslt != 0) {
         fatal("pthread_attr_getguardsize failed with error = %d", rslt);
       }
-      *bottom += guard_size;
+      bottom += guard_size;
-      *size   -= guard_size;
+      *size  -= guard_size;
     }
 
     pthread_attr_destroy(&attr);
-
   }
-  assert(os::current_stack_pointer() >= *bottom &&
+  assert(os::current_stack_pointer() >= bottom &&
-         os::current_stack_pointer() < *bottom + *size, "just checking");
+         os::current_stack_pointer() < *base, "just checking");
 }
 
-address os::current_stack_base() {
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return (bottom + size);
-}
-
-size_t os::current_stack_size() {
-  // This stack size includes the usable stack and HotSpot guard pages
-  // (for the threads that have Hotspot guard pages).
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return size;
-}
 #endif
 
 static inline struct timespec get_mtime(const char* filename) {

src/hotspot/os/windows/os_windows.cpp

@@ -425,40 +425,31 @@ int os::get_native_stack(address* stack, int frames, int toSkip) {
   return captured;
 }
 
-// os::current_stack_base()
-//
 //   Returns the base of the stack, which is the stack's
 //   starting address.  This function must be called
 //   while running on the stack of the thread being queried.
 
-address os::current_stack_base() {
+void os::current_stack_base_and_size(address* stack_base, size_t* stack_size) {
   MEMORY_BASIC_INFORMATION minfo;
   address stack_bottom;
-  size_t stack_size;
+  size_t size;
 
   VirtualQuery(&minfo, &minfo, sizeof(minfo));
-  stack_bottom =  (address)minfo.AllocationBase;
+  stack_bottom = (address)minfo.AllocationBase;
-  stack_size = minfo.RegionSize;
+  size = minfo.RegionSize;
 
   // Add up the sizes of all the regions with the same
   // AllocationBase.
   while (1) {
-    VirtualQuery(stack_bottom+stack_size, &minfo, sizeof(minfo));
+    VirtualQuery(stack_bottom + size, &minfo, sizeof(minfo));
     if (stack_bottom == (address)minfo.AllocationBase) {
-      stack_size += minfo.RegionSize;
+      size += minfo.RegionSize;
     } else {
       break;
     }
   }
-  return stack_bottom + stack_size;
+  *stack_base = stack_bottom + size;
-}
+  *stack_size = size;
-
-size_t os::current_stack_size() {
-  size_t sz;
-  MEMORY_BASIC_INFORMATION minfo;
-  VirtualQuery(&minfo, &minfo, sizeof(minfo));
-  sz = (size_t)os::current_stack_base() - (size_t)minfo.AllocationBase;
-  return sz;
 }
 
 bool os::committed_in_range(address start, size_t size, address& committed_start, size_t& committed_size) {

src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp

@@ -354,13 +354,13 @@ size_t os::Posix::default_stack_size(os::ThreadType thr_type) {
   size_t s = (thr_type == os::compiler_thread ? 4 * M : 1 * M);
   return s;
 }
-
+void os::current_stack_base_and_size(address* base, size_t* size) {
-static void current_stack_region(address * bottom, size_t * size) {
+  address bottom;
 #ifdef __APPLE__
   pthread_t self = pthread_self();
-  void *stacktop = pthread_get_stackaddr_np(self);
+  *base = (address) pthread_get_stackaddr_np(self);
   *size = pthread_get_stacksize_np(self);
-  *bottom = (address) stacktop - *size;
+  bottom = *base - *size;
 #elif defined(__OpenBSD__)
   stack_t ss;
   int rslt = pthread_stackseg_np(pthread_self(), &ss);
@@ -368,8 +368,9 @@ static void current_stack_region(address * bottom, size_t * size) {
   if (rslt != 0)
     fatal("pthread_stackseg_np failed with error = %d", rslt);
 
-  *bottom = (address)((char *)ss.ss_sp - ss.ss_size);
+  *base = (address) ss.ss_sp;
-  *size   = ss.ss_size;
+  *size = ss.ss_size;
+  bottom = *base - *size;
 #else
   pthread_attr_t attr;
 
@@ -384,30 +385,17 @@ static void current_stack_region(address * bottom, size_t * size) {
   if (rslt != 0)
     fatal("pthread_attr_get_np failed with error = %d", rslt);
 
-  if (pthread_attr_getstackaddr(&attr, (void **)bottom) != 0 ||
+  if (pthread_attr_getstackaddr(&attr, (void **)&bottom) != 0 ||
-    pthread_attr_getstacksize(&attr, size) != 0) {
+      pthread_attr_getstacksize(&attr, size) != 0) {
     fatal("Can not locate current stack attributes!");
   }
 
+  *base = bottom + *size;
+
   pthread_attr_destroy(&attr);
 #endif
-  assert(os::current_stack_pointer() >= *bottom &&
+  assert(os::current_stack_pointer() >= bottom &&
-         os::current_stack_pointer() < *bottom + *size, "just checking");
+         os::current_stack_pointer() < *base, "just checking");
-}
-
-address os::current_stack_base() {
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return (bottom + size);
-}
-
-size_t os::current_stack_size() {
-  // stack size includes normal stack and HotSpot guard pages
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return size;
 }
 
 /////////////////////////////////////////////////////////////////////////////

src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp

@@ -79,7 +79,7 @@
 # include <pthread_np.h>
 #endif
 
-// needed by current_stack_region() workaround for Mavericks
+// needed by current_stack_base_and_size() workaround for Mavericks
 #if defined(__APPLE__)
 # include <errno.h>
 # include <sys/types.h>
@@ -709,13 +709,15 @@ size_t os::Posix::default_stack_size(os::ThreadType thr_type) {
 //    |                        |/
 // P2 +------------------------+ Thread::stack_base()
 //
-// ** P1 (aka bottom) and size ( P2 = P1 - size) are the address and stack size returned from
+// ** P1 (aka bottom) and size are the address and stack size
-//    pthread_attr_getstack()
+//    returned from pthread_attr_getstack().
+// ** P2 (aka stack top or base) = P1 + size
 
-static void current_stack_region(address * bottom, size_t * size) {
+void os::current_stack_base_and_size(address* base, size_t* size) {
+  address bottom;
 #ifdef __APPLE__
   pthread_t self = pthread_self();
-  void *stacktop = pthread_get_stackaddr_np(self);
+  *base = (address) pthread_get_stackaddr_np(self);
   *size = pthread_get_stacksize_np(self);
   // workaround for OS X 10.9.0 (Mavericks)
   // pthread_get_stacksize_np returns 128 pages even though the actual size is 2048 pages
@@ -738,7 +740,7 @@ static void current_stack_region(address * bottom, size_t * size) {
       }
     }
   }
-  *bottom = (address) stacktop - *size;
+  bottom = *base - *size;
 #elif defined(__OpenBSD__)
   stack_t ss;
   int rslt = pthread_stackseg_np(pthread_self(), &ss);
@@ -746,8 +748,9 @@ static void current_stack_region(address * bottom, size_t * size) {
   if (rslt != 0)
     fatal("pthread_stackseg_np failed with error = %d", rslt);
 
-  *bottom = (address)((char *)ss.ss_sp - ss.ss_size);
+  *base = (address) ss.ss_sp;
-  *size   = ss.ss_size;
+  *size = ss.ss_size;
+  bottom = *base - *size;
 #else
   pthread_attr_t attr;
 
@@ -762,30 +765,17 @@ static void current_stack_region(address * bottom, size_t * size) {
   if (rslt != 0)
     fatal("pthread_attr_get_np failed with error = %d", rslt);
 
-  if (pthread_attr_getstackaddr(&attr, (void **)bottom) != 0 ||
+  if (pthread_attr_getstackaddr(&attr, (void **)&bottom) != 0 ||
-    pthread_attr_getstacksize(&attr, size) != 0) {
+      pthread_attr_getstacksize(&attr, size) != 0) {
     fatal("Can not locate current stack attributes!");
   }
 
+  *base = bottom + *size;
+
   pthread_attr_destroy(&attr);
 #endif
-  assert(os::current_stack_pointer() >= *bottom &&
+  assert(os::current_stack_pointer() >= bottom &&
-         os::current_stack_pointer() < *bottom + *size, "just checking");
+         os::current_stack_pointer() < *base, "just checking");
-}
-
-address os::current_stack_base() {
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return (bottom + size);
-}
-
-size_t os::current_stack_size() {
-  // stack size includes normal stack and HotSpot guard pages
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return size;
 }
 
 /////////////////////////////////////////////////////////////////////////////

src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp

@@ -176,26 +176,24 @@ size_t os::Posix::default_stack_size(os::ThreadType thr_type) {
   return s;
 }
 
-static void current_stack_region(address *bottom, size_t *size) {
+void os::current_stack_base_and_size(address* base, size_t* size)
-  address stack_bottom;
+  address bottom;
-  address stack_top;
-  size_t stack_bytes;
 
 #ifdef __APPLE__
   pthread_t self = pthread_self();
-  stack_top = (address) pthread_get_stackaddr_np(self);
+  *base = (address) pthread_get_stackaddr_np(self);
-  stack_bytes = pthread_get_stacksize_np(self);
+  *size = pthread_get_stacksize_np(self);
-  stack_bottom = stack_top - stack_bytes;
+  bottom = *base - *size;
 #elif defined(__OpenBSD__)
   stack_t ss;
   int rslt = pthread_stackseg_np(pthread_self(), &ss);
 
   if (rslt != 0)
-    fatal("pthread_stackseg_np failed with error = " INT32_FORMAT, rslt);
+    fatal("pthread_stackseg_np failed with error = %d", rslt);
 
-  stack_top = (address) ss.ss_sp;
+  *base = (address) ss.ss_sp;
-  stack_bytes  = ss.ss_size;
+  *size  = ss.ss_size;
-  stack_bottom = stack_top - stack_bytes;
+  bottom = *base - *size;
 #else
   pthread_attr_t attr;
 
@@ -203,43 +201,25 @@ static void current_stack_region(address *bottom, size_t *size) {
 
   // JVM needs to know exact stack location, abort if it fails
   if (rslt != 0)
-    fatal("pthread_attr_init failed with error = " INT32_FORMAT, rslt);
+    fatal("pthread_attr_init failed with error = %d", rslt);
 
   rslt = pthread_attr_get_np(pthread_self(), &attr);
 
   if (rslt != 0)
-    fatal("pthread_attr_get_np failed with error = " INT32_FORMAT, rslt);
+    fatal("pthread_attr_get_np failed with error = %d", rslt);
 
-  if (pthread_attr_getstackaddr(&attr, (void **) &stack_bottom) != 0 ||
+  if (pthread_attr_getstackaddr(&attr, (void **) &bottom) != 0 ||
-      pthread_attr_getstacksize(&attr, &stack_bytes) != 0) {
+      pthread_attr_getstacksize(&attr, size) != 0) {
     fatal("Can not locate current stack attributes!");
   }
 
+  *base = bottom + *size;
+
   pthread_attr_destroy(&attr);
 
-  stack_top = stack_bottom + stack_bytes;
 #endif
-
+  assert(os::current_stack_pointer() >= bottom &&
-  assert(os::current_stack_pointer() >= stack_bottom, "should do");
+         os::current_stack_pointer() < *base, "just checking");
-  assert(os::current_stack_pointer() < stack_top, "should do");
-
-  *bottom = stack_bottom;
-  *size = stack_top - stack_bottom;
-}
-
-address os::current_stack_base() {
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return bottom + size;
-}
-
-size_t os::current_stack_size() {
-  // stack size includes normal stack and HotSpot guard pages
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return size;
 }
 
 /////////////////////////////////////////////////////////////////////////////

src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp

@@ -306,89 +306,68 @@ size_t os::Posix::default_stack_size(os::ThreadType thr_type) {
   return s;
 }
 
-static void current_stack_region(address *bottom, size_t *size) {
+void os::current_stack_base_and_size(address* base, size_t* size) {
+  address bottom;
   if (os::is_primordial_thread()) {
     // primordial thread needs special handling because pthread_getattr_np()
     // may return bogus value.
-    address stack_bottom = os::Linux::initial_thread_stack_bottom();
+    bottom = os::Linux::initial_thread_stack_bottom();
-    size_t stack_bytes  = os::Linux::initial_thread_stack_size();
+    *size = os::Linux::initial_thread_stack_size();
+    *base = bottom + *size;
+  } else {
 
-    assert(os::current_stack_pointer() >= stack_bottom, "should do");
+    pthread_attr_t attr;
-    assert(os::current_stack_pointer() < stack_bottom + stack_bytes, "should do");
 
-    *bottom = stack_bottom;
+    int rslt = pthread_getattr_np(pthread_self(), &attr);
-    *size = stack_bytes;
-    return;
-  }
 
-  pthread_attr_t attr;
+    // JVM needs to know exact stack location, abort if it fails
-  int res = pthread_getattr_np(pthread_self(), &attr);
+    if (rslt != 0) {
-  if (res != 0) {
+      if (rslt == ENOMEM) {
-    if (res == ENOMEM) {
+        vm_exit_out_of_memory(0, OOM_MMAP_ERROR, "pthread_getattr_np");
-      vm_exit_out_of_memory(0, OOM_MMAP_ERROR, "pthread_getattr_np");
+      } else {
+        fatal("pthread_getattr_np failed with error = %d", rslt);
+      }
     }
-    else {
+
-      fatal("pthread_getattr_np failed with error = %d", res);
+    if (pthread_attr_getstack(&attr, (void **)&bottom, size) != 0) {
+      fatal("Cannot locate current stack attributes!");
     }
-  }
 
-  address stack_bottom;
+    *base = bottom + *size;
-  size_t stack_bytes;
-  res = pthread_attr_getstack(&attr, (void **) &stack_bottom, &stack_bytes);
-  if (res != 0) {
-    fatal("pthread_attr_getstack failed with error = %d", res);
-  }
-  address stack_top = stack_bottom + stack_bytes;
 
-  // The block of memory returned by pthread_attr_getstack() includes
+    // The block of memory returned by pthread_attr_getstack() includes
-  // guard pages where present.  We need to trim these off.
+    // guard pages where present.  We need to trim these off.
-  size_t page_bytes = os::vm_page_size();
+    size_t page_bytes = os::vm_page_size();
-  assert(((intptr_t) stack_bottom & (page_bytes - 1)) == 0, "unaligned stack");
+    assert(((intptr_t) bottom & (page_bytes - 1)) == 0, "unaligned stack");
 
-  size_t guard_bytes;
+    size_t guard_bytes;
-  res = pthread_attr_getguardsize(&attr, &guard_bytes);
+    rslt = pthread_attr_getguardsize(&attr, &guard_bytes);
-  if (res != 0) {
+    if (rslt != 0) {
-    fatal("pthread_attr_getguardsize failed with errno = %d", res);
+      fatal("pthread_attr_getguardsize failed with errno = %d", rslt);
-  }
+    }
-  int guard_pages = align_up(guard_bytes, page_bytes) / page_bytes;
+    int guard_pages = align_up(guard_bytes, page_bytes) / page_bytes;
-  assert(guard_bytes == guard_pages * page_bytes, "unaligned guard");
+    assert(guard_bytes == guard_pages * page_bytes, "unaligned guard");
 
 #ifdef IA64
-  // IA64 has two stacks sharing the same area of memory, a normal
+    // IA64 has two stacks sharing the same area of memory, a normal
-  // stack growing downwards and a register stack growing upwards.
+    // stack growing downwards and a register stack growing upwards.
-  // Guard pages, if present, are in the centre.  This code splits
+    // Guard pages, if present, are in the centre.  This code splits
-  // the stack in two even without guard pages, though in theory
+    // the stack in two even without guard pages, though in theory
-  // there's nothing to stop us allocating more to the normal stack
+    // there's nothing to stop us allocating more to the normal stack
-  // or more to the register stack if one or the other were found
+    // or more to the register stack if one or the other were found
-  // to grow faster.
+    // to grow faster.
-  int total_pages = align_down(stack_bytes, page_bytes) / page_bytes;
+    int total_pages = align_down(stack_bytes, page_bytes) / page_bytes;
-  stack_bottom += (total_pages - guard_pages) / 2 * page_bytes;
+    bottom += (total_pages - guard_pages) / 2 * page_bytes;
 #endif // IA64
 
-  stack_bottom += guard_bytes;
+    bottom += guard_bytes;
+    *size = *base - bottom;
 
-  pthread_attr_destroy(&attr);
+    pthread_attr_destroy(&attr);
+  }
 
-  assert(os::current_stack_pointer() >= stack_bottom, "should do");
+  assert(os::current_stack_pointer() >= bottom &&
-  assert(os::current_stack_pointer() < stack_top, "should do");
+         os::current_stack_pointer() < *base, "just checking");
-
-  *bottom = stack_bottom;
-  *size = stack_top - stack_bottom;
-}
-
-address os::current_stack_base() {
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return bottom + size;
-}
-
-size_t os::current_stack_size() {
-  // stack size includes normal stack and HotSpot guard pages
-  address bottom;
-  size_t size;
-  current_stack_region(&bottom, &size);
-  return size;
 }
 
 /////////////////////////////////////////////////////////////////////////////

src/hotspot/share/runtime/os.hpp

@@ -596,8 +596,7 @@ class os: AllStatic {
   static bool start_debugging(char *buf, int buflen);
 
   static address current_stack_pointer();
-  static address current_stack_base();
+  static void current_stack_base_and_size(address* base, size_t* size);
-  static size_t current_stack_size();
 
   static void verify_stack_alignment() PRODUCT_RETURN;
 

src/hotspot/share/runtime/thread.cpp

@@ -169,8 +169,11 @@ void Thread::record_stack_base_and_size() {
   // any members being initialized. Do not rely on Thread::current() being set.
   // If possible, refrain from doing anything which may crash or assert since
   // quite probably those crash dumps will be useless.
-  set_stack_base(os::current_stack_base());
+  address base;
-  set_stack_size(os::current_stack_size());
+  size_t size;
+  os::current_stack_base_and_size(&base, &size);
+  set_stack_base(base);
+  set_stack_size(size);
 
   // Set stack limits after thread is initialized.
   if (is_Java_thread()) {

src/hotspot/share/utilities/vmError.cpp

@@ -178,7 +178,10 @@ static void print_bug_submit_message(outputStream *out, Thread *thread) {
 }
 
 static bool stack_has_headroom(size_t headroom) {
-  const size_t stack_size = os::current_stack_size();
+  size_t stack_size = 0;
+  address stack_base = nullptr;
+  os::current_stack_base_and_size(&stack_base, &stack_size);
+
   const size_t guard_size = StackOverflow::stack_guard_zone_size();
   const size_t unguarded_stack_size = stack_size - guard_size;
 
@@ -186,7 +189,6 @@ static bool stack_has_headroom(size_t headroom) {
     return false;
   }
 
-  const address stack_base          = os::current_stack_base();
   const address unguarded_stack_end = stack_base - unguarded_stack_size;
   const address stack_pointer       = os::current_stack_pointer();
 
@@ -199,9 +201,11 @@ PRAGMA_INFINITE_RECURSION_IGNORED
 void VMError::reattempt_test_hit_stack_limit(outputStream* st) {
   if (stack_has_headroom(_reattempt_required_stack_headroom)) {
     // Use all but (_reattempt_required_stack_headroom - K) unguarded stack space.
-    const size_t stack_size     = os::current_stack_size();
+    size_t stack_size = 0;
+    address stack_base = nullptr;
+    os::current_stack_base_and_size(&stack_base, &stack_size);
+
     const size_t guard_size     = StackOverflow::stack_guard_zone_size();
-    const address stack_base    = os::current_stack_base();
     const address stack_pointer = os::current_stack_pointer();
 
     const size_t unguarded_stack_size = stack_size - guard_size;
@@ -975,8 +979,7 @@ void VMError::report(outputStream* st, bool _verbose) {
       stack_top = _thread->stack_base();
       stack_size = _thread->stack_size();
     } else {
-      stack_top = os::current_stack_base();
+      os::current_stack_base_and_size(&stack_top, &stack_size);
-      stack_size = os::current_stack_size();
     }
 
     address stack_bottom = stack_top - stack_size;