LUL on x86_64-linux: recover frames by following the frame pointer chain

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jseward, Assigned: jseward)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
On x86_64-Linux, LUL currently can only unwind frames for which CFI unwind data
is available.  This causes a noticeable number of junk samples in the profiler,
characterised by failures at transition points between JIT and native frame
sequences.  This patch allows LUL to try recovering the previous frame using
frame pointer chasing in the case where CFI isn't present.  This allows LUL to
unwind through or jump over interleaved JIT frames, because, respectively:

* The baseline JIT produces frame-pointerised code.

* IonMonkey doesn't produce frame-pointerised code, but also doesn't
  change the frame pointer register value.

The patch also adds counts of FP-recovered frames to LUL's statistics printing,
to make it possible to assess how often this feature is used.
(Assignee)

Comment 1

2 years ago
Created attachment 8859670 [details] [diff] [review]
LUL on x86_64-linux: recover frames by following the frame pointer chain

On x86_64-Linux, LUL currently can only unwind frames for which CFI unwind data
is available.  This causes a noticeable number of junk samples in the profiler,
characterised by failures at transition points between JIT and native frame
sequences.  This patch allows LUL to try recovering the previous frame using
frame pointer chasing in the case where CFI isn't present.  This allows LUL to
unwind through or jump over interleaved JIT frames, because, respectively:

* The baseline JIT produces frame-pointerised code.

* IonMonkey doesn't produce frame-pointerised code, but also doesn't
  change the frame pointer register value.

The patch also adds counts of FP-recovered frames to LUL's statistics printing,
to make it possible to assess how often this feature is used.
Attachment #8859670 - Flags: review?(nfroyd)
Attachment #8859670 - Flags: review?(jdemooij)
(Assignee)

Updated

2 years ago
Assignee: nobody → jseward
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 years ago
Jan, requesting review just of the commit comment re JIT generated
code and unwinding.  Nathan, requesting review of the whole thing.
Comment on attachment 8859670 [details] [diff] [review]
LUL on x86_64-linux: recover frames by following the frame pointer chain

Review of attachment 8859670 [details] [diff] [review]:
-----------------------------------------------------------------

Commit message makes sense, one nit: IonMonkey doesn't change the frame pointer register value *if the profiler is enabled*. It *can* use the FP register as arbitrary register if profiling is disabled.
Attachment #8859670 - Flags: review?(jdemooij) → review+
Comment on attachment 8859670 [details] [diff] [review]
LUL on x86_64-linux: recover frames by following the frame pointer chain

Review of attachment 8859670 [details] [diff] [review]:
-----------------------------------------------------------------

Sure, let's try this.

::: tools/profiler/lul/LulMain.cpp
@@ +1572,5 @@
> +      TaggedUWord old_xbp       = regs.xbp; // points at new_BP ?
> +      TaggedUWord old_xbp_plus1 = regs.xbp; // points at new_IP ?
> +      TaggedUWord old_xbp_plus2 = regs.xbp; // is the new_SP ?
> +      old_xbp_plus1 = old_xbp_plus1 + TaggedUWord(1 * wordSzB);
> +      old_xbp_plus2 = old_xbp_plus2 + TaggedUWord(2 * wordSzB);

I think it'd be slightly clearer to just initialize `old_xbp_plus1` (`old_xbp_plus2`, respectively) with the expressions here.
Attachment #8859670 - Flags: review?(nfroyd) → review+

Comment 5

2 years ago
Pushed by jseward@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca381bda5cb
LUL on x86_64-linux: recover frames by following the frame pointer chain.  r=froydnj,jandem.

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ca381bda5cb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.