Closed Bug 1530552 Opened 6 years ago Closed 1 year ago

register JIT code with Windows runtime on ARM64/x64, so that RtlVirtualUnwind can unwind jit frames

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
112 Branch
Tracking Status
firefox112 --- fixed

People

(Reporter: luke, Assigned: bas.schouten)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

If we preserve %fp (bug 1426134), it's apparently not too hard to create the Windows 64-bit runtime unwind data that allows RtlVirtualUnwind() to unwind through JIT frames. Apparently V8 switched to doing this recently.

There are a lot of advantages for making RtlVirtualUnwind Just Work on JIT code:

  • Breakpad and the Gecko profiler will be able to reliably unwind through JIT frames to the C++ on the other side, giving us better stacks.
  • The Gecko profiler could stop using JS::ProfilingFrameIterator altogether on Win64.
  • Other Windows tools, like Xperf, Windows Performance Analyzer and the MSVC debugger, also won't get lost in JIT code

This would also enable further simplifications in the next bug.

Blocks: 1530555

Setting P3 as bug 1426134 is a P3 and a large piece of work.

Priority: -- → P3
Summary: register JIT code with Windows runtime on ARM64/x64 → register JIT code with Windows runtime on ARM64/x64, so that RtlVirtualUnwind can unwind jit frames
Blocks: sm-jits

From bug 1381585:

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)

I've started using ETW more for profiling so this would be really nice to have. V8 supports this now:
https://source.chromium.org/chromium/chromium/src/+/main:v8/src/diagnostics/unwinding-info-win64.cc;l=560;drc=fb9e129964cf89d7de13cd8f2a7bcaf6573fb8da

I don't think this bug strictly depends on bug 1426134. Sure, it would be easier to create these records if bug 1426134 were already fixed, because all functions could use the same dead-simple "use the frame pointer" unwinding information. But as long as we know the stack size of each JIT function, we can create accurate unwind information even if we don't use frame pointers.

No longer depends on: 1426134
See Also: → 1759173

My understanding is that the Windows AMD64 stack walking info requires using a frame pointer if the stack pointer changes outside of the prologue or epilogue. (e.g. instead of using push, clang will store arguments directly into their location on the stack and switches to using a frame pointer if the size of the stack is unknown)

That means that it's not possible to describe the JIT code that we currently generate because it uses push instructions when calling functions. https://searchfox.org/mozilla-central/rev/3e21f2bb69734804d311e16b914c4efcc94bb18d/js/src/jit/CodeGenerator.cpp#5452

Given this, it seems like it's more practical to preserve the stack pointer on AMD64 than it is to switch to a more static stack layout.

Depends on: 1426134
Severity: normal → S3
Assignee: nobody → bas
Status: NEW → ASSIGNED
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/114885c100cc
Tell Windows about our frame pointers in JIT code if we're on Windows > 8.1. r=jandem
Pushed by smolnar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40c430582786
Tell Windows about our frame pointers in JIT code if we're on Windows > 8.1. r=jandem
Flags: needinfo?(bas)
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch
Regressions: 1841250
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: