Skip to content

Enable inline-frame symbolication; fix Windows return-address off-by-one#23

Merged
dylan-conway merged 1 commit intomainfrom
dylanc/stack-traces
Apr 11, 2026
Merged

Enable inline-frame symbolication; fix Windows return-address off-by-one#23
dylan-conway merged 1 commit intomainfrom
dylanc/stack-traces

Conversation

@dylan-conway
Copy link
Copy Markdown
Member

What

  • --inlines / -i for llvm-symbolizer (Linux/macOS) and pdb-addr2line (Windows). processSymbolizerOutput now parses blank-line-separated blocks so one input address can expand to N inline frames.
  • cleanFunctionName: keep the .method suffix after a Zig generic Type(params) group. Was turning UnboundedQueue(Job,.next).push into just UnboundedQueue.
  • adjustBunAddresses: decrement non-first bun addresses by 1 on Windows only. crash_handler.zig already encodes addr - 1 on macOS/Linux but emits raw return addresses on Windows; without the decrement the symbolizer lands on the next inlined region (e.g. the following defer), producing phantom frames like arena.deinit / fetchSub that aren't on the call path.
  • Break git.tsdebug-store.ts circular import (TDZ on cache_root when imported via capture-fixture.ts).
  • Hand-crafted symbolize fixtures: add blank-line separators between addresses to match real symbolizer output. Add 4 real captured fixtures (linux/macos×2/windows).

Why

--no-inlines was passed only because the original parser consumed exactly two lines per address. With it, the symbolizer collapses the inline chain and glues the inlining-site file:line onto the outermost function name — both lossy and misleading.

Before / after (real traces, see new fixtures)

Platform Before After
Linux HTTPThread segfault 2 frames; top = processEvents @ HTTPThread.zig:376 (wrong fn) 6 frames; top = drainQueuedShutdowns @ :376 (actual crash site)
macOS RuntimeTranspilerStore 3 frames; UnboundedQueue (method name lost) 17 frames; UnboundedQueue(...).pushBatch
Windows RuntimeTranspilerStore eventLoop / deinit / fetchSub (phantom frames from off-by-one) eventLoopdispatchToMainThread:241run:294runFromWorkerThread:286ThreadPool.run:581entryFn

149 tests pass.

…by-one

- llvm-symbolizer/pdb-addr2line now run with --inlines/-i; processSymbolizerOutput
  parses blank-line-separated blocks so one input address can yield N frames.
  Previously --no-inlines collapsed the inline chain and attributed the
  inlining-site file:line to the outermost function.
- cleanFunctionName: preserve `.method` suffix after a Zig generic `Type(params)`
  group instead of truncating at `(` (was turning `Queue(T).push` into `Queue`).
- adjustBunAddresses: decrement non-first bun addresses by 1 on Windows only.
  crash_handler.zig already encodes addr-1 on macOS/Linux but emits raw return
  addresses on Windows; without the decrement the symbolizer lands on the
  *next* inlined region (e.g. the following defer), producing phantom frames.
- Break git.ts <-> debug-store.ts circular import (TDZ on cache_root when
  imported via capture-fixture.ts).
- Hand-crafted symbolize fixtures: add blank-line separators between addresses
  to match real symbolizer output. Add 4 real captured fixtures (linux/macos/
  macos/windows) covering the above.
@dylan-conway dylan-conway merged commit a6f421f into main Apr 11, 2026
1 check passed
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 11, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

This change updates symbolization command arguments to enable inline information, refactors address adjustment and output parsing logic in the symbolizer, modifies cache directory resolution, and introduces regression test fixtures for Linux, macOS, and Windows crash scenarios.

Changes

Cohort / File(s) Summary
Symbolization Configuration
backend/remap.ts, scripts/capture-fixture.ts
Changed non-Windows symbolizer arguments from --no-inlines to --inlines and added -i flag to Windows pdb_addr2line invocation.
Symbolizer Processing
backend/symbolize.ts
Refactored adjustBunAddresses to apply Windows-specific decrement on bun frames after the first; refactored processSymbolizerOutput to use block-based parsing separated by blank lines; updated cleanFunctionName to preserve method names by reconstructing with (...) in place of arguments/generics.
Build Configuration
backend/git.ts
Changed local_clone_dir default resolution from cache_root-based path to import.meta.dir-relative path for the cache directory fallback.
Parse Test Fixtures
test/fixtures/parse/linux-http-thread-segfault.json, test/fixtures/parse/macos-prev-regression.json, test/fixtures/parse/macos-remap-stack-frames.json, test/fixtures/parse/windows-segfault-neg1.json
Added four new parse test fixtures with encoded input strings for regression testing.
Symbolize Test Fixtures
test/fixtures/symbolize/linux-http-thread-segfault.json, test/fixtures/symbolize/macos-prev-regression.json, test/fixtures/symbolize/macos-remap-stack-frames.json, test/fixtures/symbolize/windows-segfault-neg1.json
Added four new symbolize test fixtures with platform-specific addresses and expected stack trace outputs.
Symbolize Fixture Updates
test/fixtures/symbolize/all-unknown-reverts.json, test/fixtures/symbolize/mixed-js-and-foreign-frames.json, test/fixtures/symbolize/panic-prefix-stripped.json
Updated expected stdout strings to include additional blank line separators between stack frames.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the two main changes in the changeset: enabling inline-frame symbolication and fixing the Windows return-address off-by-one issue.
Description check ✅ Passed The description comprehensively details the changes across multiple files, provides clear before/after examples, and explains the rationale for each modification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant