Cleanup/chaining jmp_buf/test cases

Author: zhengyu123

ddprof-lib/src/main/cpp/counters.h

@@ -122,7 +122,7 @@
    * paths (delegated and direct) go into SAMPLES_DROPPED_REC_LOCK. */         \
   X(JVMTI_STACKS_DROPPED_LOCK, "jvmti_stacks_dropped_lock")                   \
   X(SAMPLES_DROPPED_REC_LOCK, "samples_dropped_rec_lock")                     \
-  X(SAMPLES_DROOPED_THREAD_LOCAL, "samples_dropped_thread_local")
+  X(SAMPLES_DROPPED_THREAD_LOCAL, "samples_dropped_thread_local")
 #define X_ENUM(a, b) a,
 typedef enum CounterId : int {
   DD_COUNTER_TABLE(X_ENUM) DD_NUM_COUNTERS

ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp

@@ -20,7 +20,6 @@
 using StackWalkValidation::inDeadZone;
 using StackWalkValidation::aligned;
 using StackWalkValidation::MAX_FRAME_SIZE;
-using StackWalkValidation::sameStack;
 
 // Initialize once, they survive on profiler restart
 static jobject JAVA_PLATFORM_CLASSLOADER = nullptr;
@@ -234,7 +233,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex
 
     ProfiledThread* prof_thread = ProfiledThread::currentSignalSafe();
     if (prof_thread == nullptr) {
-        Counters::increment(SAMPLES_DROOPED_THREAD_LOCAL);
+        Counters::increment(SAMPLES_DROPPED_THREAD_LOCAL);
         return 0;
     }
 

ddprof-lib/src/main/cpp/jvmThread.cpp

@@ -23,15 +23,6 @@ bool JVMThread::initialize() {
   return _jvm_thread.isKeyValid();
 }
 
-/**
- * Per-thread initialization
- * 
- * This method *must* be called before signal is enabled
- * for the thread.
- */
-void JVMThread::initThread() {
-}
-
 bool JVMThread::isInitialized() {
     return (_tid != nullptr && _jvm_thread.isKeyValid());
 }

ddprof-lib/src/main/cpp/libraryPatcher_linux.cpp

@@ -259,7 +259,6 @@ static void init_tls_and_register() {
         pt->startInitWindow();
     }
     Profiler::registerThread(ProfiledThread::currentTid());
-    JVMThread::initThread();
 }
 
 // Wrapper around the real start routine.
@@ -375,7 +374,6 @@ static void* start_routine_wrapper(void* args) {
         ProfiledThread::initCurrentThread();
         ProfiledThread::currentSignalSafe()->startInitWindow();
         Profiler::registerThread(ProfiledThread::currentTid());
-        JVMThread::initThread();
     }
     // Use POSIX cleanup instead of C++ RAII to handle pthread_exit(): see run_with_cleanup.
     // cleanup_unregister has already run on run_with_cleanup's normal return path.

ddprof-lib/src/main/cpp/profiler.cpp

@@ -73,7 +73,6 @@ static CTimer ctimer;
 static CTimerJvmti ctimer_jvmti;
 
 void Profiler::onThreadStart(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread) {
-  JVMThread::initThread();
   ProfiledThread::initCurrentThread();
   ProfiledThread *current = ProfiledThread::current();
   current->setJavaThread(true);
@@ -946,8 +945,8 @@ int Profiler::crashHandlerInternal(int signo, siginfo_t *siginfo, void *ucontext
     have_tls_protection = true;
   }
   // If thrd == nullptr, we proceed but with limited handling capability.
-  // Only HotspotSupport::checkFault (which has its own sameStack fallback)
-  // and the JDK-8313796 workaround can safely handle faults without TLS.
+  // Only HotspotSupport::checkFault and the JDK-8313796 workaround can safely 
+  // handle faults without TLS.
 
   StackFrame frame(ucontext);
   uintptr_t pc = frame.pc();

ddprof-lib/src/main/cpp/threadLocalData.h

@@ -210,10 +210,6 @@ class ProfiledThread : public ThreadLocalData {
     return _jmp_buf != nullptr;
   }
 
-  void resetJmpCtx() {
-    _jmp_buf = nullptr;
-  }
-
   // Signal-handler depth counter used by SignalHandlerScope (guards.h).  All
   // access happens on the owning thread (signal handlers are delivered to the
   // thread that's interrupted), so plain reads/writes are AS-safe — no locks,

ddprof-lib/src/main/cpp/vmEntry.cpp

@@ -541,8 +541,7 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) {
 
 
   if (attach) {
-    JVMThread::initThread();
-    _jvmti->GenerateEvents(JVMTI_EVENT_DYNAMIC_CODE_GENERATED);
+2    _jvmti->GenerateEvents(JVMTI_EVENT_DYNAMIC_CODE_GENERATED);
     _jvmti->GenerateEvents(JVMTI_EVENT_COMPILED_METHOD_LOAD);
   } else {
     _jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_VM_INIT, NULL);

ddprof-lib/src/test/cpp/hotspot_crash_protection_ut.cpp

@@ -11,17 +11,21 @@
  * VMThread::isJavaThread() provides the gate.
  *
  * Crash recovery inside walkVM relies on setjmp/longjmp:
- *   1. walkVM stores a jmp_buf* in HotspotSupport::_jmp_ctx (thread-local).
- *   2. If a fault fires during the walk, checkFault() detects the live context
- *      via isThreadProtectedByLongjmp() and calls longjmp() to unwind.
+ *   1. walkVM stores a jmp_buf* on ProfiledThread (setJmpCtx/getJmpCtx),
+ *      chaining it with whatever context was already installed so a
+ *      signal-based sampler interrupting a non-signal-based sampler's own
+ *      in-flight walkVM() call doesn't clobber the outer call's context.
+ *   2. If a fault fires during the walk, checkFault() detects the live
+ *      context via ProfiledThread::isProtected() and calls longjmp() to
+ *      unwind through whatever context is currently installed.
  *   3. ProfiledThread tracks nested crash-handler depth so recursive faults
  *      (e.g. wall-clock signal inside a crash handler) are capped safely.
  *
  * Tests cover:
  *   A. ProfiledThread thread-type classification (isJavaThread fast path)
  *   B. Vtable majority-vote logic (isJavaThread slow path)
  *   C. Crash-handler nesting depth (ProfiledThread crash handler state)
- *   D. Longjmp-protection state via HotspotSupport public API
+ *   D. jmp_buf chaining across nested/interrupted walkVM() calls
  */
 
 #include <gtest/gtest.h>
@@ -253,4 +257,139 @@ TEST_F(CrashHandlerNestingTest, IsDeepOnlyAboveLimit) {
     _pt->resetCrashHandler();
 }
 
+// ---------------------------------------------------------------------------
+// D. jmp_buf chaining (ProfiledThread::setJmpCtx/getJmpCtx/isProtected)
+//
+// A non-signal-based sampler's walkVM() call can itself be interrupted by a
+// signal-based sampler, putting two walkVM() frames on the same thread's
+// stack. Each frame follows the same protocol:
+//   jmp_buf* prev = prof_thread->getJmpCtx();   // save whatever was there
+//   prof_thread->setJmpCtx(&my_ctx);            // install this frame's ctx
+//   ... walk ...
+//   prof_thread->setJmpCtx(prev);               // restore on every exit path
+// checkFault() always longjmps through whatever is currently installed
+// (thrd->getJmpCtx()), so the inner frame must never leave the outer frame's
+// context installed while the inner frame is doing its own protected work,
+// and must always hand it back — via normal completion or fault recovery —
+// before returning control to the outer frame.
+// ---------------------------------------------------------------------------
+
+class JmpCtxChainingTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        ProfiledThread::initCurrentThread();
+        _pt = ProfiledThread::currentSignalSafe();
+        ASSERT_NE(nullptr, _pt);
+    }
+
+    void TearDown() override {
+        ProfiledThread::release();
+    }
+
+    ProfiledThread* _pt = nullptr;
+};
+
+TEST_F(JmpCtxChainingTest, InitiallyUnprotected) {
+    EXPECT_FALSE(_pt->isProtected());
+    EXPECT_EQ(nullptr, _pt->getJmpCtx());
+}
+
+TEST_F(JmpCtxChainingTest, SetAndGetRoundTrip) {
+    jmp_buf ctx;
+    _pt->setJmpCtx(&ctx);
+    EXPECT_TRUE(_pt->isProtected());
+    EXPECT_EQ(&ctx, _pt->getJmpCtx());
+}
+
+// Replicates a single walkVM() call's save/install/restore around its body.
+TEST_F(JmpCtxChainingTest, SingleFrameRestoresPreviousOnExit) {
+    jmp_buf outer;
+    jmp_buf* prev = _pt->getJmpCtx();  // nullptr: no enclosing walkVM() call
+    ASSERT_EQ(nullptr, prev);
+
+    _pt->setJmpCtx(&outer);
+    EXPECT_EQ(&outer, _pt->getJmpCtx());
+
+    // Simulate walkVM()'s `done:` path.
+    _pt->setJmpCtx(prev);
+    EXPECT_EQ(nullptr, _pt->getJmpCtx());
+    EXPECT_FALSE(_pt->isProtected());
+}
+
+// Replicates two nested walkVM() calls: a signal-based sampler interrupting a
+// non-signal-based sampler's own in-flight walkVM(). The inner call must
+// chain off the outer's jmp_buf*, install its own, and hand the outer's back
+// on its way out — leaving the outer frame's context exactly as it left it.
+TEST_F(JmpCtxChainingTest, NestedFramesChainAndUnwindInOrder) {
+    jmp_buf outer_ctx;
+    jmp_buf* outer_prev = _pt->getJmpCtx();
+    ASSERT_EQ(nullptr, outer_prev);
+    _pt->setJmpCtx(&outer_ctx);
+    EXPECT_EQ(&outer_ctx, _pt->getJmpCtx());
+
+    {
+        // Inner walkVM() call, as if a signal fired while the outer one was
+        // mid-walk.
+        jmp_buf inner_ctx;
+        jmp_buf* inner_prev = _pt->getJmpCtx();
+        EXPECT_EQ(&outer_ctx, inner_prev);  // chained off the outer frame
+
+        _pt->setJmpCtx(&inner_ctx);
+        EXPECT_EQ(&inner_ctx, _pt->getJmpCtx());
+
+        // Inner call completes via its own `done:` path.
+        _pt->setJmpCtx(inner_prev);
+    }
+
+    // The outer frame's context must be untouched by the inner call.
+    EXPECT_EQ(&outer_ctx, _pt->getJmpCtx());
+
+    _pt->setJmpCtx(outer_prev);
+    EXPECT_EQ(nullptr, _pt->getJmpCtx());
+}
+
+// End-to-end with real setjmp/longjmp: a fault inside the inner frame must
+// land in the inner frame's own recovery branch — checkFault() always
+// longjmps through whatever is currently installed — and once the inner
+// frame has recovered and restored the outer's context, the outer frame must
+// be left exactly as it was, never having been unwound itself.
+TEST_F(JmpCtxChainingTest, FaultInInnerFrameDoesNotDisturbOuterFrame) {
+    jmp_buf outer_ctx;
+    jmp_buf* outer_prev = _pt->getJmpCtx();
+    int outer_landed = 0;
+    int inner_landed = 0;
+
+    if (setjmp(outer_ctx) != 0) {
+        outer_landed++;
+    } else {
+        _pt->setJmpCtx(&outer_ctx);
+
+        // --- inner "walkVM" call, interrupted mid-flight by a fault ---
+        jmp_buf inner_ctx;
+        jmp_buf* inner_prev = _pt->getJmpCtx();
+        ASSERT_EQ(&outer_ctx, inner_prev);
+
+        if (setjmp(inner_ctx) != 0) {
+            inner_landed++;
+            _pt->setJmpCtx(inner_prev);
+        } else {
+            _pt->setJmpCtx(&inner_ctx);
+            // Simulate checkFault(): longjmp through whatever is currently
+            // installed — this must hit the inner frame, not the outer.
+            longjmp(*_pt->getJmpCtx(), 1);
+            FAIL() << "unreachable: longjmp does not return";
+        }
+        // --- inner call has returned normally after recovering ---
+
+        EXPECT_EQ(&outer_ctx, _pt->getJmpCtx())
+            << "outer frame's context must survive the inner frame's fault";
+
+        _pt->setJmpCtx(outer_prev);
+    }
+
+    EXPECT_EQ(1, inner_landed);
+    EXPECT_EQ(0, outer_landed) << "the fault must not have unwound past the inner frame";
+    EXPECT_FALSE(_pt->isProtected());
+}
+
 #endif  // __linux__