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

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task)

Description

[Jan de Mooij [:jandem]]

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.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: 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.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1782188 - Use more reliable endStackAddress when profiling JIT/Wasm frames. r?iain!

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.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc1d41e88ae3
Use more reliable endStackAddress when profiling JIT/Wasm frames. r=iain

Comment 4

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/bc1d41e88ae3