Closed Bug 1782188 Opened 6 months ago Closed 6 months ago

Frame::endStackAddress can be invalid if we're executing JIT/Wasm code

Categories

(Core :: JavaScript Engine: JIT, task)

task
Not set
normal

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: jandem, Assigned: jandem)

Details

Attachments

(2 files)

We set endStackAddress like this in ProfilingFrameIterator::getPhysicalFrameAndEntry:

  frame.endStackAddress = activation_->asJit()->jsOrWasmExitFP();

This is fine for activations that have a valid exit FP, but it's not if we're currently in JIT or Wasm code, because we don't clear the exit FP when returning to JIT code.

Furthermore, we don't always set the exit FP for C++ calls: sometimes we want to have faster calls without constructing an exit frame (see masm.callWithABI).

I think we have the following failure cases:

  • endStackAddress is non-nullptr, stale, points to an address higher up on the stack than the most recent JIT/Wasm frame. In this case we'd probably get unsymbolicated frames.
  • endStackAddress is non-nullptr, stale, and we called into C++ with callWithABI. This could result in missing stack frames because we incorrectly filter them out?
  • endStackAddress is nullptr and we're either executing JIT/Wasm code or we called into C++ with callWithABI. In this case we could get unsymbolicated frames too I think because we don't filter anything out.
Attached file Simple testcase

Here's a simple micro-benchmark where the profiler doesn't show the call to atan at all, only time in f in shown.

Setting endStackAddress to nullptr "fixes" it.

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED

We were using the activation's exitFP for this, but that value isn't cleared
when we return to JIT code from C++ so we could use stale values if we're in JIT
code (or used callWithABI calls without an exit frame). This could result in
unsymbolicated frames or missing frames in profiles.

This patch changes the JIT and Wasm frame iterators to set endStackAddress from
their constructor. In the outer iterator we then keep track of the first value for
the current activation and use that instead of the activation's exitFP.

Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bc1d41e88ae3
Use more reliable endStackAddress when profiling JIT/Wasm frames. r=iain
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in before you can comment on or make changes to this bug.