Local builds symbolication is broken by the removal of eval-like commands in JSM
Categories
(Core :: Gecko Profiler, defect, P1)
Tracking
()
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)
(deleted),
text/x-phabricator-request
|
Details |
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?
Updated•2 years ago
|
Comment 1•2 years ago
|
||
Set release status flags based on info from the regressing bug 1772378
Comment 2•2 years ago
|
||
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.
Selfish P1, because this is making it very difficult for Firefox developers to work on/with the profiler in local DEBUG builds.
Workarounds:
- Build in non-DEBUG.
- Locally comment out the unwanted MOZ_CRASH. Be careful not to publish it!
(Symbolication still won't happen, but at least Firefox will not crash.)
Assignee | ||
Comment 4•2 years ago
|
||
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.
Comment 5•2 years ago
|
||
Yes of course I don't know why I didn't think of that first 🤦♂️
Assignee | ||
Comment 6•2 years ago
|
||
(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?
Assignee | ||
Comment 7•2 years ago
|
||
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?
Comment 8•2 years ago
|
||
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....
Assignee | ||
Comment 9•2 years ago
|
||
Indeed the profiler's symbolication jsm uses a dynamically loaded wasm
file to unpack symbols from binary object files.
Updated•2 years ago
|
Assignee | ||
Comment 10•2 years ago
|
||
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!
Comment 11•2 years ago
|
||
bugherder |
Updated•2 years ago
|
Description
•