Closed Bug 1777479 Opened 2 years ago Closed 2 years ago

Local builds symbolication is broken by the removal of eval-like commands in JSM

Categories

(Core :: Gecko Profiler, defect, P1)

defect

Tracking

()

RESOLVED FIXED
104 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox-esr102 --- unaffected
firefox102 --- unaffected
firefox103 --- unaffected
firefox104 --- fixed

People

(Reporter: julienw, Assigned: julienw)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

We use a WASM file to demangle symbols when symbolicating local object files.
But since bug 1772378 we get this error:

eval() and eval-like uses are not allowed in the Parent Process or in System Contexts (Blocked usage in “resource://devtools/client/performance-new/symbolication.jsm.js”)

The related code is:
https://searchfox.org/mozilla-central/rev/4654ce1e24a6af17bc96ab60f1f70c218755142f/devtools/client/performance-new/symbolication.jsm.js#82

It is pretty important for the firefox developers that this use cases still works.

Hey Tom, any suggestion to fix this?

Flags: needinfo?(tom)

Set release status flags based on info from the regressing bug 1772378

So EvalRestrictions are analogous to JS Filename Load Restrictions. We have a flag on JS::InstatiateOptions that allows individual callsites to opt-out of that check which is used in a few places.

This is the perfect use case for this type of exemption, it's a permanent exemption and it's this one location. Unfortunately we don't have a "opt out of eval restrictions" flag anywhere.

The WASM check happens inside nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction which gets called by JSContext::isRuntimeCodeGenEnabled. That, in turn, is called in a few places.

This is well outside code I own for how the APIs would want to be changed, but one idea, perhaps temporary, would be to add a boolean to compileStreaming, false by default, that indicates exemption. Plumb that boolean through to what I assume is WebAssembly_compileStreaming and when it gets there, pass either JS::RuntimeCode::WASM or a new JS::RuntimeCode::WASMEvalExempted.

That is pretty ugly though, so you could also change the function signature of ContentSecurityPolicyPermitsJSAction to pass the boolean in. Ideally there would be some sort of Options structure, like InstatiateOptions that it could live on, but I guess one hasn't been needed before.

Flags: needinfo?(tom)
Severity: -- → S3
Priority: -- → P2

Selfish P1, because this is making it very difficult for Firefox developers to work on/with the profiler in local DEBUG builds.

Workarounds:

(Symbolication still won't happen, but at least Firefox will not crash.)

Priority: P2 → P1

Hey Tom,

would that be acceptable if we'd put "resource://devtools/client/performance-new/symbolication.jsm.js" in the eval allowlist, while we work on a longer term solution? I checked locally and this seems to fix the problem for me. I see the debugger is already there as well, I believe for a similar reason.

Flags: needinfo?(tom)

Yes of course I don't know why I didn't think of that first 🤦‍♂️

Flags: needinfo?(tom)

(In reply to Tom Ritter [:tjr] (OOTO until July 6) from comment #5)

Yes of course I don't know why I didn't think of that first 🤦‍♂️

Nice, I'm firing up a patch for just this.

Just to make sure: are you thinking of this as a temporary solution only, or is it acceptable long-term as well?

Flags: needinfo?(tom)

Taking a step back, I wonder if wasm should be restricted like eval is. Indeed if I understand correctly any use of wasm inside the codebase will be subject to the same restrictions, and therefore be blocked. On the other hand eval is more for niche uses.
Do we really want to block all uses of wasm in our privileged code?

This could be a long term solution. As far as wasm goes, I was advised too encompass wasm in my recent patch. I don't believe the applies to all uses of wasm, only dynamically loaded ones....

Flags: needinfo?(tom)

Indeed the profiler's symbolication jsm uses a dynamically loaded wasm
file to unpack symbols from binary object files.

Assignee: nobody → felash
Status: NEW → ASSIGNED

My understanding is that all wasm are dynamically loaded. But I'm definitely not an expert in this.
Also maybe we should look at loading it from some less-privileged code...

Anyway thanks for the pointers!

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 104 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: