Add CFI unwind info to x86_64 unix implementations of NS_InvokeByIndex and SharedStub

RESOLVED FIXED in Firefox 55

Status

()

Core
XPCOM
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: jseward, Assigned: froydnj)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 months ago
The Gecko Profiler on Linux fails to unwind (using LUL) through handwritten
assembly for NS_InvokeByIndex and SharedStub.  These are the implementations in

 xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S

and

 xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp

respectively.  This accounts for a substantial fraction of the failing
stacktraces in one test.  With possibly around 1000 failures, 338 were due to
NS_InvokeByIndex and 121 to SharedStub.  Given that many of the other failures
are due to unwinding into JIT generated code, this fraction (338+121) / ~1000 is
possibly more significant than it at first seems.
(Reporter)

Updated

9 months ago
Flags: needinfo?(nfroyd)
(Assignee)

Comment 1

9 months ago
Created attachment 8857538 [details] [diff] [review]
add CFI directives for x86-64 Linux xptcall assembly routines

Adding these directives means unwinding the stack with the profiler will
no longer get confused when we need to unwind through these functions.

I mostly cribbed from GCC-generated CFI directives to churn these out.
Attachment #8857538 - Flags: review?(jseward)
(Assignee)

Updated

9 months ago
Assignee: nobody → nfroyd
Blocks: 1345321
Flags: needinfo?(nfroyd)
(Reporter)

Comment 2

9 months ago
Comment on attachment 8857538 [details] [diff] [review]
add CFI directives for x86-64 Linux xptcall assembly routines

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

Thanks for the quick patch.  This looks plausible.  It appears to reduce the number
of LUL unwind failures in AOT compiled code that are caused by missing CFI info by
about a factor of 40, although it is difficult to say exactly.
Attachment #8857538 - Flags: review?(jseward) → review+

Comment 3

9 months ago
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c6b4dadccaf
add CFI directives for x86-64 Linux xptcall assembly routines; r=jseward

Comment 4

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3c6b4dadccaf
Status: NEW → RESOLVED
Last Resolved: 9 months 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.