Skip to content

Add SafeAccess protection and frame type validation for signal handlers#349

Merged
jbachorik merged 4 commits intomainfrom
jb/fp_safeaccess
Feb 4, 2026
Merged

Add SafeAccess protection and frame type validation for signal handlers#349
jbachorik merged 4 commits intomainfrom
jb/fp_safeaccess

Conversation

@jbachorik
Copy link
Collaborator

@jbachorik jbachorik commented Jan 30, 2026 •
edited
Loading

What does this PR do?:

This PR adds SafeAccess protection to several critical code paths in signal handler context and fixes invalid frame type handling:

  1. SafeAccess for frame pointer walking: Replaces raw pointer dereference with SafeAccess::load() in StackWalker::walkFP() to prevent ASan stack-buffer-overflow errors during frame pointer chain traversal.

  2. Frame type sanitization for J9: Adds validation and clamping for frame types to prevent invalid FrameTypeId values (e.g., value 48 causing ASAN errors). Implements defensive clamping in FrameType::decode() using FRAME_TYPE_MAX constant and properly handles negative BCI values (BCI_WALL, BCI_ERROR, etc.) through <= 0 check instead of == 0.

  3. SafeAccess for NativeFunc marking: Adds SafeAccess protection to NativeFunc::read_mark() which is called from signal handler context during stack walking.

  4. Test stability improvements: Fixes NPE and timing issues in flaky profiler tests, particularly in JfrDumpTest and BaseContextWallClockTest.

Motivation:

ASan detected multiple issues during signal handler execution:

  • Stack-buffer-overflow in walkFP() when traversing into stack frames with local variables
  • Invalid FrameTypeId values (e.g., 48) causing "not a valid value for type 'FrameTypeId'" errors
  • SEGV in NativeFunc::read_mark() when accessing potentially unmapped memory

The walkDwarf() function already uses SafeAccess::load() consistently (lines 190, 194), but walkFP() was inconsistent - using SafeAccess for PC but not for FP.

Additional Notes:

  • SafeAccess mechanism:
    • Uses assembly-level fault protection via safefetch64_impl / safefetch32_impl
    • If the read faults, the handler returns the default value (0/null), causing loops to exit naturally
    • Aligns walkFP() and read_mark() with the protection pattern used in walkDwarf()
  • Frame type clamping:
    • FRAME_TYPE_MAX constant provides future-proof clamping instead of hardcoded bit masks
    • Properly handles negative special BCI values through arithmetic right shift sign extension
  • Test infrastructure: run_docker_tests.sh can now target J9/OpenJ9 JVMs (8-j9, 11-j9, 17-j9, 21-j9)

How to test the change?:

Run ASan tests via CI (testAsan task).

The fixes prevent the following ASan errors that were occurring:

AddressSanitizer: stack-buffer-overflow in StackWalker::walkFP
READ of size 8 at address pointing to 'thread_count' (4 bytes)

runtime error: load of value 48, which is not a valid value for type 'FrameTypeId'

AddressSanitizer: SEGV in NativeFunc::read_mark(char const*)

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

🤖 Generated with Claude Code

@jbachorik jbachorik added the AI label Jan 30, 2026
@dd-octo-sts
Copy link

dd-octo-sts bot commented Jan 30, 2026 •
edited
Loading

Scan-Build Report

User:runner@runnervmkj6or
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Tue Feb 3 17:18:25 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs1
Unused code
Dead assignment1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Unused codeDead assignmentlibraryPatcher_linux.cpppatch_library_unlocked941

@jbachorik jbachorik added the test:asan Run CI tests with AddressSanitizer configuration label Feb 2, 2026
@jbachorik jbachorik force-pushed the jb/fp_safeaccess branch 2 times, most recently from eedf923 to c394d1d Compare February 2, 2026 16:45
@jbachorik jbachorik changed the title Use SafeAccess for frame pointer dereference in walkFP WIP: Use SafeAccess for frame pointer dereference in walkFP Feb 2, 2026
@jbachorik jbachorik changed the title WIP: Use SafeAccess for frame pointer dereference in walkFP Use SafeAccess for frame pointer dereference in walkFP Feb 3, 2026
@jbachorik jbachorik changed the title Use SafeAccess for frame pointer dereference in walkFP Add SafeAccess protection and frame type validation for signal handlers Feb 3, 2026
@jbachorik jbachorik marked this pull request as ready for review February 3, 2026 09:39
@jbachorik jbachorik requested a review from a team as a code owner February 3, 2026 09:39
@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes wall wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 24 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes wall wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 memleak,alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak,alloc memleak,alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 cpu]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu cpu
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 memleak,alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak,alloc memleak,alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes alloc alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 17 metrics, 21 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 cpu,wall,alloc,memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes cpu,wall,alloc,memleak cpu,wall,alloc,memleak
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 22 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak memleak
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 cpu,wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu,wall cpu,wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [aarch64 cpu]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu cpu
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 17 metrics, 21 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes alloc alloc
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 cpu,wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu,wall cpu,wall
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 24 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 cpu,wall,alloc,memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes cpu,wall,alloc,memleak cpu,wall,alloc,memleak
wall on on

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 22 unstable metrics.

@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Benchmarks [x86_64 memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.37.0 1.38.0-jb_fp_safeaccess-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak memleak
wall off off

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics.

jbachorik and others added 3 commits February 3, 2026 15:44
@jbachorik @claude
Use SafeAccess for frame pointer dereference in walkFP
91a04d4 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jbachorik @claude
Sanitize J9 frame types to prevent invalid FrameTypeId values
b0fc682 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jbachorik @claude
Use SafeAccess for NativeFunc::read_mark() in signal handler
fcdfdd1 
The read_mark() function can be called from signal handler context during
stack walking. Direct dereferencing of func->_mark can cause SEGV if the
computed pointer happens to be aligned but points to unmapped memory.

Use SafeAccess::safeFetch32() to safely read the mark field, extracting
the byte at offset 2 from the NativeFunc header.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@pr-commenter
Copy link

pr-commenter bot commented Feb 3, 2026 •
edited
Loading

Integration Tests

❔ 0 passed, 0 failed out of 0 configurations

Test Matrix

Platform JDK 8 JDK 11 JDK 17 JDK 21 JDK 25
glibc-x64-hotspot ❔ ❔ ❔ ❔ ❔
glibc-x64-openj9 ❔ ❔ ❔ ❔ ❔
glibc-arm64-hotspot ❔ ❔ ❔ ❔ ❔
glibc-arm64-openj9 ❔ ❔ ❔ ❔ ❔
musl-x64-hotspot ❔ ❔ ❔ ❔ ❔
musl-x64-openj9 ❔ ❔ ❔ ❔ ❔
musl-arm64-hotspot ❔ ❔ ❔ ❔ ❔
musl-arm64-openj9 ❔ ❔ ❔ ❔ ❔

Links

rkennke
rkennke approved these changes Feb 4, 2026
View reviewed changes
Copy link
Contributor

@rkennke rkennke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jbachorik jbachorik merged commit a01b927 into main Feb 4, 2026
131 of 136 checks passed
@jbachorik jbachorik deleted the jb/fp_safeaccess branch February 4, 2026 14:12
@github-actions github-actions bot added this to the 1.38.0 milestone Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rkennke rkennke rkennke approved these changes

Assignees

No one assigned

Labels

AI test:asan Run CI tests with AddressSanitizer configuration

Projects

None yet

Milestone

1.38.0

Development

Successfully merging this pull request may close these issues.

2 participants

@jbachorik @rkennke