Fix stack walking alignment issues causing ASAN violations (#362)

Author: jbachorik

.github/workflows/nightly.yml

@@ -20,18 +20,30 @@ jobs:
     needs: run-test
     if: failure()
     steps:
-      - name: Download failed tests artifact
+      - name: Download all failure artifacts
         uses: actions/download-artifact@v4
         with:
-          name: failures
+          pattern: failures-*
           path: ./artifacts
+          merge-multiple: true
       - name: Report failures
         run: |
-          find ./artifacts -name 'failures_*' -exec cat {} \; > ./artifacts/failures.txt
-          scenarios=$(cat ./artifacts/failures.txt | tr '\n' ',')
-          
+          # Check if any failure files exist
+          if ! ls ./artifacts/failures_*.txt 1> /dev/null 2>&1; then
+            echo "No failure artifacts found"
+            exit 0
+          fi
+
+          # Combine all failure files
+          find ./artifacts -name 'failures_*.txt' -exec cat {} \; > ./artifacts/all_failures.txt
+          scenarios=$(cat ./artifacts/all_failures.txt | tr '\n' ',' | sed 's/,$//')
+
           echo "Failed scenarios: $scenarios"
 
-          curl -X POST "${{ secrets.SLACK_WEBHOOK }}" \
-               -H 'Content-Type: application/json' \
-               -d "{'scenarios': '${scenarios}', 'failed_run_url': '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}'}"
+          if [ -n "${{ secrets.SLACK_WEBHOOK }}" ]; then
+            curl -X POST "${{ secrets.SLACK_WEBHOOK }}" \
+                 -H 'Content-Type: application/json' \
+                 -d "{\"scenarios\": \"${scenarios}\", \"failed_run_url\": \"${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}\"}"
+          else
+            echo "SLACK_WEBHOOK not configured, skipping notification"
+          fi

AGENTS.md

@@ -167,6 +167,38 @@ Release builds automatically extract debug symbols via `NativeLinkTask`, reducin
 
 **See:** `build-logic/README.md` for full documentation
 
+### Docker-based Testing (Recommended for ASan/Non-Local Environments)
+
+**When to use**: For ASan testing, cross-architecture testing (aarch64), different libc variants (musl), or reproducing CI environment issues.
+
+```bash
+# ASan tests on aarch64 Linux
+./utils/run-docker-tests.sh --arch=aarch64 --config=asan --libc=glibc --jdk=21
+
+# Run specific test pattern
+./utils/run-docker-tests.sh --arch=aarch64 --tests="*SpecificTest*"
+
+# Enable C++ gtests
+./utils/run-docker-tests.sh --arch=aarch64 --gtest
+
+# Drop to shell for debugging
+./utils/run-docker-tests.sh --arch=aarch64 --shell
+
+# Test with musl libc
+./utils/run-docker-tests.sh --libc=musl --jdk=21
+
+# Test with OpenJ9
+./utils/run-docker-tests.sh --jdk=21-j9
+
+# Use mounted repo (faster, but may have stale artifacts)
+./utils/run-docker-tests.sh --mount
+
+# Rebuild Docker images
+./utils/run-docker-tests.sh --rebuild
+```
+
+**Note**: The Docker script supports `--config=debug|release|asan|tsan`. Use this for cross-architecture testing and reproducing CI environments. For local development, use `./gradlew testAsan` directly.
+
 ### Build Options
 ```bash
 # Skip native compilation

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

@@ -17,6 +17,7 @@
 
 const uintptr_t MAX_WALK_SIZE = 0x100000;
 const intptr_t MAX_INTERPRETER_FRAME_SIZE = 0x1000;
+const intptr_t MAX_FRAME_SIZE_WORDS = StackWalkValidation::MAX_FRAME_SIZE / sizeof(void*);  // 0x8000 = 32768 words
 
 static ucontext_t empty_ucontext{};
 
@@ -187,11 +188,19 @@ int StackWalker::walkDwarf(void* ucontext, const void** callchain, int max_depth
             pc = (const char*)pc + (f.fp_off >> 1);
         } else {
             if (f.fp_off != DW_SAME_FP && f.fp_off < MAX_FRAME_SIZE && f.fp_off > -MAX_FRAME_SIZE) {
-                fp = (uintptr_t)SafeAccess::load((void**)(sp + f.fp_off));
+                uintptr_t fp_addr = sp + f.fp_off;
+                if (!aligned(fp_addr)) {
+                    break;
+                }
+                fp = (uintptr_t)SafeAccess::load((void**)fp_addr);
             }
 
             if (EMPTY_FRAME_SIZE > 0 || f.pc_off != DW_LINK_REGISTER) {
-                pc = stripPointer(SafeAccess::load((void**)(sp + f.pc_off)));
+                uintptr_t pc_addr = sp + f.pc_off;
+                if (!aligned(pc_addr)) {
+                    break;
+                }
+                pc = stripPointer(SafeAccess::load((void**)pc_addr));
             } else if (depth == 1) {
                 pc = (const void*)frame.link();
             } else {
@@ -245,6 +254,10 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
 
     const void* pc = anchor->lastJavaPC();
     if (pc == NULL) {
+        // Verify alignment before dereferencing sp as pointer
+        if (!aligned(sp)) {
+            return 0;
+        }
         pc = ((const void**)sp)[-1];
     }
 
@@ -334,6 +347,12 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
             }
 
             if (nm->isNMethod()) {
+                // Check if deoptimization is in progress before walking compiled frames
+                if (vm_thread != NULL && vm_thread->inDeopt()) {
+                    fillFrame(frames[depth++], BCI_ERROR, "break_deopt_compiled");
+                    break;
+                }
+
                 int level = nm->level();
                 FrameTypeId type = details && level >= 1 && level <= 3 ? FRAME_C1_COMPILED : FRAME_JIT_COMPILED;
                 fillFrame(frames[depth++], type, 0, nm->method()->id());
@@ -360,7 +379,21 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
                     // Handle situations when sp is temporarily changed in the compiled code
                     frame.adjustSP(nm->entry(), pc, sp);
 
-                    sp += nm->frameSize() * sizeof(void*);
+                    // Validate NMethod metadata before using frameSize()
+                    int frame_size = nm->frameSize();
+                    if (frame_size <= 0 || frame_size > MAX_FRAME_SIZE_WORDS) {
+                        fillFrame(frames[depth++], BCI_ERROR, "break_invalid_framesize");
+                        break;
+                    }
+
+                    sp += frame_size * sizeof(void*);
+
+                    // Verify alignment before dereferencing sp as pointer (secondary defense)
+                    if (!aligned(sp)) {
+                        fillFrame(frames[depth++], BCI_ERROR, "break_misaligned_sp");
+                        break;
+                    }
+
                     fp = ((uintptr_t*)sp)[-FRAME_PC_SLOT - 1];
                     pc = ((const void**)sp)[-FRAME_PC_SLOT];
                     continue;
@@ -407,7 +440,7 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
                             sp = frame.senderSP();
                             fp = *(uintptr_t*)fp;
                         } else {
-                            pc = stripPointer(*(void**)sp);
+                            pc = stripPointer(SafeAccess::load((void**)sp));
                             sp = frame.senderSP();
                         }
                         continue;
@@ -455,7 +488,21 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
                 }
 
                 if (depth > 1 && nm->frameSize() > 0) {
-                    sp += nm->frameSize() * sizeof(void*);
+                    // Validate NMethod metadata before using frameSize()
+                    int frame_size = nm->frameSize();
+                    if (frame_size <= 0 || frame_size > MAX_FRAME_SIZE_WORDS) {
+                        fillFrame(frames[depth++], BCI_ERROR, "break_invalid_framesize");
+                        break;
+                    }
+
+                    sp += frame_size * sizeof(void*);
+
+                    // Verify alignment before dereferencing sp as pointer (secondary defense)
+                    if (!aligned(sp)) {
+                        fillFrame(frames[depth++], BCI_ERROR, "break_misaligned_sp");
+                        break;
+                    }
+
                     fp = ((uintptr_t*)sp)[-FRAME_PC_SLOT - 1];
                     pc = ((const void**)sp)[-FRAME_PC_SLOT];
                     continue;
@@ -572,7 +619,12 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
             }
 
             if (EMPTY_FRAME_SIZE > 0 || f.pc_off != DW_LINK_REGISTER) {
-                pc = stripPointer(*(void**)(sp + f.pc_off));
+                // Verify alignment before dereferencing sp + offset
+                uintptr_t pc_addr = sp + f.pc_off;
+                if (!aligned(pc_addr)) {
+                    break;
+                }
+                pc = stripPointer(SafeAccess::load((void**)pc_addr));
             } else if (depth == 1) {
                 pc = (const void*)frame.link();
             } else {

utils/run-docker-tests.sh

@@ -10,7 +10,7 @@
 #   --libc=glibc|musl        (default: glibc)
 #   --jdk=8|11|17|21|25|8-j9|11-j9|17-j9|21-j9  (default: 21)
 #   --arch=x64|aarch64       (default: auto-detect)
-#   --config=debug|release   (default: debug)
+#   --config=debug|release|asan|tsan   (default: debug)
 #   --tests="TestPattern"    (optional, specific test to run)
 #   --gtest                  (enable C++ gtests, disabled by default)
 #   --shell                  (drop to shell instead of running tests)
@@ -186,8 +186,8 @@ if [[ "$ARCH" != "x64" && "$ARCH" != "aarch64" ]]; then
     exit 1
 fi
 
-if [[ "$CONFIG" != "debug" && "$CONFIG" != "release" ]]; then
-    echo "Error: --config must be 'debug' or 'release'"
+if [[ "$CONFIG" != "debug" && "$CONFIG" != "release" && "$CONFIG" != "asan" && "$CONFIG" != "tsan" ]]; then
+    echo "Error: --config must be 'debug', 'release', 'asan', or 'tsan'"
     exit 1
 fi
 
@@ -291,14 +291,14 @@ ENV DEBIAN_FRONTEND=noninteractive
 
 # Install build dependencies
 # - libasan/libtsan for GCC sanitizers
-# - libclang-rt-dev for clang sanitizers and libFuzzer
+# - clang provides sanitizer runtimes and libFuzzer
 # - openssh-client for git clone over SSH
 RUN apt-get update && \
     apt-get install -y --no-install-recommends \
         curl wget bash make g++ clang git jq cmake \
         libgtest-dev libgmock-dev tar binutils libc6-dbg \
         ca-certificates linux-libc-dev \
-        libasan6 libtsan0 libclang-rt-dev openssh-client && \
+        libasan6 libtsan0 openssh-client && \
     rm -rf /var/lib/apt/lists/*
 
 # Set up Gradle cache directory
@@ -360,7 +360,9 @@ fi
 # ========== Run Tests ==========
 
 # Build gradle test command
-GRADLE_CMD="./gradlew -PCI -PkeepJFRs :ddprof-test:test${CONFIG}"
+# Capitalize first letter for gradle task names (testDebug, testAsan, etc.)
+CONFIG_CAPITALIZED="$(tr '[:lower:]' '[:upper:]' <<< ${CONFIG:0:1})${CONFIG:1}"
+GRADLE_CMD="./gradlew -PCI -PkeepJFRs :ddprof-test:test${CONFIG_CAPITALIZED}"
 if [[ -n "$TESTS" ]]; then
     GRADLE_CMD="$GRADLE_CMD --tests \"$TESTS\""
 fi