Walk YJIT JIT frames via frame pointers for full stack unwinding (upstream prep)#36
Draft
dalehamel wants to merge 18 commits intoruby-jit-upstream-prepfrom
Draft
Walk YJIT JIT frames via frame pointers for full stack unwinding (upstream prep)#36dalehamel wants to merge 18 commits intoruby-jit-upstream-prepfrom
dalehamel wants to merge 18 commits intoruby-jit-upstream-prepfrom
Conversation
2dbcec0 to
c6ca0a4
Compare
8aabe9c to
1cb1fb6
Compare
c6ca0a4 to
cc382ef
Compare
c378267 to
e58fa0a
Compare
dalehamel
commented
Apr 8, 2026
Member
Author
dalehamel
left a comment
There was a problem hiding this comment.
Review assisted by pair-review
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
e58fa0a to
eab1249
Compare
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
eab1249 to
7e53798
Compare
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
e986777 to
5831ed4
Compare
… cleanup - Extract JIT region detection into standalone findJITRegion() function that handles both prctl-labeled mappings (spanning full reserved area including holes) and heuristic fallback (first anonymous executable mapping) - Add comprehensive unit tests for JIT region detection: no mappings, file-backed only, labeled single/multiple, heuristic fallback, precedence - Fix Detach() to clean up PidInterpreterMapping prefixes (previously only deleted proc data, leaking eBPF map entries) - Remove dead code: m.Vaddr < jitMapping.Vaddr branch that could never be true since /proc/pid/maps is sorted by address
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
Ruby 3.4.7 with --yjit on arm64. Verifies that: - JIT code region is detected from anonymous executable mappings - A JIT frame is pushed for the YJIT-compiled code - Ruby VM frames (is_prime, sum_of_primes, loop) are properly unwound after the JIT frame On main, this coredump fails with native_no_pid_page_mapping because the profiler doesn't know how to handle the anonymous JIT mapping. Moduledata tar for CI upload: ~/coredump-uploads/ruby-yjit-moduledata.tar
Ruby 3.4.7 with --yjit on x86_64. Same verification as arm64 test: - JIT code region detected from anonymous executable mappings - JIT frame pushed for YJIT-compiled code - Full Ruby VM stack unwound (is_prime, sum_of_primes, loop) Moduledata tar: ~/coredump-uploads/ruby-yjit-amd64-moduledata.tar
On systems with CONFIG_ANON_VMA_NAME=y, Ruby labels its JIT memory region via prctl(PR_SET_VMA), giving it a path like '[anon:Ruby:rb_yjit_reserve_addr_space]'. Without this fix, IsFileBacked() returns true for these mappings (non-empty Path), causing IsAnonymous() to return false. This prevents the LPM prefix registration loop in SynchronizeMappings from registering JIT executable pages for eBPF dispatch. Add IsPrctlNamed() helper and exclude prctl-named mappings from IsFileBacked(), so they are correctly classified as anonymous.
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
Replace the jit_detected flag approach with V8-style frame pointer unwinding through Ruby JIT frames. When YJIT emits frame pointers (always on arm64, with --yjit-perf on x86_64), the Ruby eBPF unwinder walks the native FP chain through JIT frames, pushes each as a RUBY_FRAME_TYPE_JIT frame, then resolves the post-JIT mapping so native unwinding can continue below the Ruby VM stack. When frame pointers are not available, the original behavior is preserved: a single dummy JIT frame is pushed, cfuncs are pushed inline, and native unwinding is stopped at the end of the Ruby stack. Also fixes parseMappings discarding prctl-labeled [anon:...] mappings, which prevented the YJIT JIT region from being visible to interpreter handlers.
…tion Restructure the eBPF JIT frame pointer walk: instead of a separate loop before the main CFP walk, advance the native FP chain by one frame per main loop iteration. This keeps the native unwind state in lockstep with the Ruby VM stack, supporting both YJIT (1 JIT frame, exits after first iteration) and ZJIT (1 JIT frame per iseq, 1:1 with CFPs). Fix JIT region detection in SynchronizeMappings to scan all mappings (including non-executable ---p reservations) for the prctl-labeled JIT region, then only register LPM prefixes for executable pages. This ensures jit_start/jit_end cover the full reserved address range even when the r-xp committed pages don't carry the label. Also fix IsAnonymous() to recognize [anon:...] labeled mappings, and remove debug log spam from parseMappings.
- Restore findJITRegion() from base (open-telemetry#1102) which was incorrectly removed during rebase - PR #26 now reuses the shared function instead of inline code - Remove redundant inline first-pass JIT detection (now handled by findJITRegion) - Detach cleanup is inherited from the updated base (open-telemetry#1102)
- Fix doc comment placement: move hasJitFramePointers comment to directly above the function definition instead of above findJITRegion - Fix containerized process support: use /proc/PID/root/tmp/ to access container filesystem for perf map detection, and glob perf-*.map to handle namespace PID mismatch (Ruby uses ns-local PID in filename) - Remove os import (replaced by path/filepath for Glob) - Rebuild BPF blobs
7e53798 to
eed8418
Compare
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
dalehamel
added a commit
that referenced
this pull request
Apr 8, 2026
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
With frame pointer unwinding enabled (always on arm64), the JIT region is walked via the FP chain, producing a richer stack with native frames between Ruby VM frames resolved. The amd64 test is unchanged since hasJitFramePointers returns false without --yjit-perf.
5831ed4 to
e151da4
Compare
dalehamel
commented
Apr 8, 2026
| "Object#is_prime+0 in /Users/dalehamel/src/github.com/Shopify/otel-ebpf-pr1102/tools/coredump/testsources/ruby/loop.rb:14", | ||
| "Object#sum_of_primes+0 in /Users/dalehamel/src/github.com/Shopify/otel-ebpf-pr1102/tools/coredump/testsources/ruby/loop.rb:24", | ||
| "block (2 levels) in <main>+0 in /Users/dalehamel/src/github.com/Shopify/otel-ebpf-pr1102/tools/coredump/testsources/ruby/loop.rb:34", | ||
| "libruby.so.3.4.7+0x31e19f", |
Member
Author
There was a problem hiding this comment.
this enrichment is expected and is a feature of this PR, as aarch64 always includes frame pointers we get this for 'free' with this PR.
99a3529 to
41b4e54
Compare
dalehamel
added a commit
that referenced
this pull request
Apr 9, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
dalehamel
added a commit
that referenced
this pull request
Apr 9, 2026
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
41b4e54 to
ecf5e25
Compare
Only push the JIT frame when trace->num_frames == 0 (leaf position), restoring the original behavior for both FP and non-FP paths.
This reverts commit 3860e11.
walk_ruby_stack is re-entered via tail calls to process more frames. On re-entry, in_jit was recomputed from record->state.pc which hasn't changed (non-FP path), causing the JIT frame to be pushed again on every tail call. Guard with !jit_detected so the JIT frame is only pushed once on the first entry.
eedbf07 to
33ae489
Compare
acebe8c to
83ef589
Compare
dalehamel
added a commit
that referenced
this pull request
Apr 9, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
dalehamel
added a commit
that referenced
this pull request
Apr 9, 2026
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
dalehamel
added a commit
that referenced
this pull request
Apr 9, 2026
- Fix JIT region check to use >= for lower bound (half-open interval [start, end)) matching V8 and HotSpot conventions - Reuse map lookup value to avoid double hashing of RawMapping struct key - Wrap CalculatePrefixList error for better debugging - Log stale prefix deletion errors instead of silently discarding - Move prctl(PR_SET_VMA) named anonymous mapping support from process/process.go (was in PR #36 but is needed here for findJITRegion labeled detection) - Remove stale 'assume all anonymous' comment - Rebuild BPF blobs for >= fix
dalehamel
added a commit
that referenced
this pull request
Apr 9, 2026
- Add 'Register LPM prefixes' comment to SynchronizeMappings loop - Extract in_jit bool variable from inline if condition for clarity - Improve JIT detection comments (read_ruby_frame and walk_ruby_stack) - Fix whitespace: add blank line after FRAMES_PER_WALK_RUBY_STACK define, normalize MAX_EP_CHECKS spacing - Rebuild BPF blobs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased version of Shopify PR #26, stacked on ruby-jit-upstream-prep (open-telemetry#1102 rebase).
BPF blobs will need rebuilding.