CFI for JaegerMonkey assembly code should apply one instruction *before* entry point

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jimb, Unassigned)

Tracking

unspecified
mozilla10
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Created attachment 568315 [details] [diff] [review]
Provide CFI for the code address GDB checks when a return address points at JaegerThrowpoline's or JaegerInterpoline's entry point.

Here's the explanation from the comments in the patch:

+ * Special rules for JaegerThrowpoline and friends:
+ *
+ * In ordinary code, return addresses always point directly after a call
+ * instruction. When GDB looks up the CFI for a return address it got from the
+ * stack (as opposed to the current PC), it uses the CFI just before the return
+ * address --- the CFI associated with the call instruction --- to do the
+ * unwinding. However, JaegerMonkey uses hacks that edit return addresses to
+ * point directly at the first instruction of JaegerThrowpoline,
+ * JaegerInterpoline, and their ilk, so GDB ends up trying to use the CFI
+ * associated with whatever instruction lies immediately *before* the given
+ * entry point.
+ *
+ * We make sure our CFI covers the code address GDB will actually use, by
+ * placing a 'nop' *before* the entry point --- it is never executed --- and
+ * having our CFI apply starting at that nop.
Attachment #568315 - Flags: review?(sphink)
Comment on attachment 568315 [details] [diff] [review]
Provide CFI for the code address GDB checks when a return address points at JaegerThrowpoline's or JaegerInterpoline's entry point.

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

Wow. Very nice. Uh... I'm glad you caught this, because I'd have gone bonkers if I ever noticed it happening and tried to debug it.

Great comment too. I should've done that in the first place, I guess.
Attachment #568315 - Flags: review?(sphink) → review+
(Reporter)

Comment 2

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/74a92a761da1
Status: NEW → ASSIGNED
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/74a92a761da1
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.