8282306: os::is_first_C_frame(frame*) crashes on invalid link access

Reviewed-by: stuefe, mdoerr

src/hotspot/cpu/aarch64/frame_aarch64.inline.hpp

@@ -148,10 +148,12 @@ inline intptr_t* frame::id(void) const { return unextended_sp(); }
 inline bool frame::is_older(intptr_t* id) const   { assert(this->id() != NULL && id != NULL, "NULL frame id");
                                                     return this->id() > id ; }
 
-
-
 inline intptr_t* frame::link() const              { return (intptr_t*) *(intptr_t **)addr_at(link_offset); }
 
+inline intptr_t* frame::link_or_null() const {
+  intptr_t** ptr = (intptr_t **)addr_at(link_offset);
+  return os::is_readable_pointer(ptr) ? *ptr : NULL;
+}
 
 inline intptr_t* frame::unextended_sp() const     { return _unextended_sp; }
 

src/hotspot/cpu/arm/frame_arm.inline.hpp

@@ -124,9 +124,13 @@ inline intptr_t* frame::id(void) const { return unextended_sp(); }
 inline bool frame::is_older(intptr_t* id) const   { assert(this->id() != NULL && id != NULL, "NULL frame id");
                                                     return this->id() > id ; }
 
-
 inline intptr_t* frame::link() const              { return (intptr_t*) *(intptr_t **)addr_at(link_offset); }
 
+inline intptr_t* frame::link_or_null() const {
+  intptr_t** ptr = (intptr_t **)addr_at(link_offset);
+  return os::is_readable_pointer(ptr) ? *ptr : NULL;
+}
+
 inline intptr_t* frame::unextended_sp() const     { return _unextended_sp; }
 
 // Return address:

src/hotspot/cpu/ppc/frame_ppc.inline.hpp

@@ -117,6 +117,10 @@ inline intptr_t* frame::link() const {
   return (intptr_t*)callers_abi()->callers_sp;
 }
 
+inline intptr_t* frame::link_or_null() const {
+  return link();
+}
+
 inline intptr_t* frame::real_fp() const {
   return fp();
 }

src/hotspot/cpu/s390/frame_s390.inline.hpp

@@ -155,6 +155,10 @@ inline intptr_t* frame::link() const {
   return (intptr_t*) callers_abi()->callers_sp;
 }
 
+inline intptr_t* frame::link_or_null() const {
+  return link();
+}
+
 inline intptr_t** frame::interpreter_frame_locals_addr() const {
   return (intptr_t**) &(ijava_state()->locals);
 }

src/hotspot/cpu/x86/frame_x86.inline.hpp

@@ -138,10 +138,13 @@ inline intptr_t* frame::id(void) const { return unextended_sp(); }
 inline bool frame::is_older(intptr_t* id) const   { assert(this->id() != NULL && id != NULL, "NULL frame id");
                                                     return this->id() > id ; }
 
-
-
 inline intptr_t* frame::link() const              { return (intptr_t*) *(intptr_t **)addr_at(link_offset); }
 
+inline intptr_t* frame::link_or_null() const {
+  intptr_t** ptr = (intptr_t **)addr_at(link_offset);
+  return os::is_readable_pointer(ptr) ? *ptr : NULL;
+}
+
 inline intptr_t* frame::unextended_sp() const     { return _unextended_sp; }
 
 // Return address:

src/hotspot/cpu/zero/frame_zero.inline.hpp

@@ -82,6 +82,11 @@ inline intptr_t* frame::link() const {
   return NULL;
 }
 
+inline intptr_t* frame::link_or_null() const {
+  ShouldNotCallThis();
+  return NULL;
+}
+
 inline interpreterState frame::get_interpreterState() const {
   return zero_interpreterframe()->interpreter_state();
 }

src/hotspot/share/runtime/frame.hpp

@@ -207,8 +207,12 @@ class frame {
 
  public:
   // Link (i.e., the pointer to the previous frame)
+  // might crash if the frame has no parent
   intptr_t* link() const;
 
+  // Link (i.e., the pointer to the previous frame) or null if the link cannot be accessed
+  intptr_t* link_or_null() const;
+
   // Return address
   address  sender_pc() const;
 

src/hotspot/share/runtime/os.cpp

@@ -1173,34 +1173,34 @@ void os::print_location(outputStream* st, intptr_t x, bool verbose) {
   st->print_cr(INTPTR_FORMAT " is an unknown value", p2i(addr));
 }
 
+bool is_pointer_bad(intptr_t* ptr) {
+  return !is_aligned(ptr, sizeof(uintptr_t)) || !os::is_readable_pointer(ptr);
+}
+
 // Looks like all platforms can use the same function to check if C
 // stack is walkable beyond current frame.
+// Returns true if this is not the case, i.e. the frame is possibly
+// the first C frame on the stack.
 bool os::is_first_C_frame(frame* fr) {
 
 #ifdef _WINDOWS
   return true; // native stack isn't walkable on windows this way.
 #endif
-
   // Load up sp, fp, sender sp and sender fp, check for reasonable values.
   // Check usp first, because if that's bad the other accessors may fault
   // on some architectures.  Ditto ufp second, etc.
-  uintptr_t fp_align_mask = (uintptr_t)(sizeof(address)-1);
-  // sp on amd can be 32 bit aligned.
-  uintptr_t sp_align_mask = (uintptr_t)(sizeof(int)-1);
 
-  uintptr_t usp    = (uintptr_t)fr->sp();
-  if ((usp & sp_align_mask) != 0) return true;
+  if (is_pointer_bad(fr->sp())) return true;
 
   uintptr_t ufp    = (uintptr_t)fr->fp();
-  if ((ufp & fp_align_mask) != 0) return true;
+  if (is_pointer_bad(fr->fp())) return true;
 
   uintptr_t old_sp = (uintptr_t)fr->sender_sp();
-  if ((old_sp & sp_align_mask) != 0) return true;
-  if (old_sp == 0 || old_sp == (uintptr_t)-1) return true;
+  if ((uintptr_t)fr->sender_sp() == (uintptr_t)-1 || is_pointer_bad(fr->sender_sp())) return true;
 
-  uintptr_t old_fp = (uintptr_t)fr->link();
-  if ((old_fp & fp_align_mask) != 0) return true;
-  if (old_fp == 0 || old_fp == (uintptr_t)-1 || old_fp == ufp) return true;
+  uintptr_t old_fp = (uintptr_t)fr->link_or_null();
+  if (old_fp == 0 || old_fp == (uintptr_t)-1 || old_fp == ufp ||
+    is_pointer_bad(fr->link_or_null())) return true;
 
   // stack grows downwards; if old_fp is below current fp or if the stack
   // frame is too large, either the stack is corrupted or fp is not saved
@@ -1212,7 +1212,6 @@ bool os::is_first_C_frame(frame* fr) {
   return false;
 }
 
-
 // Set up the boot classpath.
 
 char* os::format_boot_path(const char* format_string,

src/hotspot/share/runtime/safefetch.inline.hpp

@@ -54,10 +54,20 @@ inline intptr_t SafeFetchN(intptr_t* adr, intptr_t errValue) {
 
 // returns true if SafeFetch32 and SafeFetchN can be used safely (stubroutines are already generated)
 inline bool CanUseSafeFetch32() {
+#if defined (__APPLE__) && defined(AARCH64)
+  if (Thread::current_or_null_safe() == NULL) { // workaround for JDK-8282475
+    return false;
+  }
+#endif // __APPLE__ && AARCH64
   return StubRoutines::SafeFetch32_stub() ? true : false;
 }
 
 inline bool CanUseSafeFetchN() {
+#if defined (__APPLE__) && defined(AARCH64)
+  if (Thread::current_or_null_safe() == NULL) {
+    return false;
+  }
+#endif // __APPLE__ && AARCH64
   return StubRoutines::SafeFetchN_stub() ? true : false;
 }
 

src/hotspot/share/runtime/thread.cpp

@@ -336,10 +336,10 @@ void Thread::call_run() {
 
   // Perform common initialization actions
 
-  register_thread_stack_with_NMT();
-
   MACOS_AARCH64_ONLY(this->init_wx());
 
+  register_thread_stack_with_NMT();
+
   JFR_ONLY(Jfr::on_thread_start(this);)
 
   log_debug(os, thread)("Thread " UINTX_FORMAT " stack dimensions: "

test/hotspot/gtest/runtime/test_os.cpp

@@ -32,6 +32,7 @@
 #include "utilities/ostream.hpp"
 #include "utilities/align.hpp"
 #include "unittest.hpp"
+#include "runtime/frame.inline.hpp"
 
 static size_t small_page_size() {
   return os::vm_page_size();
@@ -865,3 +866,13 @@ TEST_VM(os, iso8601_time) {
   // Canary should still be intact
   EXPECT_EQ(buffer[os::iso8601_timestamp_size], 'X');
 }
+
+TEST_VM(os, is_first_C_frame) {
+#ifndef _WIN32
+  frame invalid_frame;
+  EXPECT_TRUE(os::is_first_C_frame(&invalid_frame)); // the frame has zeroes for all values
+
+  frame cur_frame = os::current_frame(); // this frame has to have a sender
+  EXPECT_FALSE(os::is_first_C_frame(&cur_frame));
+#endif // _WIN32
+}