1884837 - Assertion failure: !stencil.canLazilyParse

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Ask]

tested at commit
poc

const o2 = {
    "newCompartment": true,
};
o2.discardSource = true;
const v3 = newGlobal(o2);
Debugger().addDebuggee(v3).createSource(true);
gc();

stack trace

#0  js::frontend::CompilationStencil::instantiateStencilAfterPreparation (cx=cx@entry=0x7ffff6439100, input=..., stencil=..., gcOutput=...) at /home/uuu/gecko-dev.git/js/src/frontend/Stencil.cpp:2625
#1  0x0000555557e73890 in js::frontend::CompilationStencil::instantiateStencils (cx=0x7ffff6439100, input=..., stencil=..., gcOutput=...) at /home/uuu/gecko-dev.git/js/src/frontend/Stencil.cpp:2612
#2  0x0000555557dc84b9 in js::frontend::InstantiateStencils (cx=cx@entry=0x7ffff6439100, input=..., stencil=..., gcOutput=...) at /home/uuu/gecko-dev.git/js/src/frontend/BytecodeCompiler.cpp:482
#3  0x0000555557dfcf7c in CompileGlobalScriptToStencilAndMaybeInstantiate<char16_t> (maybeCx=0x7ffff6439100, fc=fc@entry=0x7fffffffcad8, tempLifoAlloc=..., input=..., scopeCache=scopeCache@entry=0x7fffffffc960, srcBuf=..., scopeKind=js::ScopeKind::Global, maybeExtraBindings=0x0, output=...) at /home/uuu/gecko-dev.git/js/src/frontend/BytecodeCompiler.cpp:390
#4  0x0000555557dc8a9d in CompileGlobalScriptImpl<char16_t> (cx=cx@entry=0x7ffff6439100, fc=fc@entry=0x7fffffffcad8, options=..., srcBuf=..., scopeKind=js::ScopeKind::Global, maybeExtraBindings=maybeExtraBindings@entry=0x0) at /home/uuu/gecko-dev.git/js/src/frontend/BytecodeCompiler.cpp:509
#5  0x0000555557dc887a in js::frontend::CompileGlobalScript (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, cx@entry=0x7ffff6439100, fc=0x0, fc@entry=0x7fffffffcad8, options=..., srcBuf=..., scopeKind=js::ScopeKind::Function) at /home/uuu/gecko-dev.git/js/src/frontend/BytecodeCompiler.cpp:521
#6  0x00005555574a11d9 in CompileSourceBuffer<char16_t> (cx=0x7ffff6439100, options=..., srcBuf=...) at /home/uuu/gecko-dev.git/js/src/vm/CompilationAndEvaluation.cpp:97
#7  JS::Compile (cx=0x7ffff6439100, options=..., srcBuf=...) at /home/uuu/gecko-dev.git/js/src/vm/CompilationAndEvaluation.cpp:104
#8  0x0000555557cf6c83 in js::DebuggerObject::CallData::createSource (this=this@entry=0x7fffffffce80) at /home/uuu/gecko-dev.git/js/src/debugger/Object.cpp:1312
#9  0x0000555557d15266 in js::DebuggerObject::CallData::ToNative<&js::DebuggerObject::CallData::createSource> (cx=cx@entry=0x7ffff6439100, argc=<optimized out>, vp=<optimized out>) at /home/uuu/gecko-dev.git/js/src/debugger/Object.cpp:236
#10 0x0000555557261697 in CallJSNative (cx=cx@entry=0x7ffff6439100, native=native@entry=0x555557d150f0 <js::DebuggerObject::CallData::ToNative<&js::DebuggerObject::CallData::createSource>(JSContext*, unsigned int, JS::Value*)>, reason=reason@entry=js::CallReason::Call, args=...) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:479
#11 0x00005555572608f3 in js::InternalCallOrConstruct (cx=0x7ffff6439100, args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:573
#12 0x00005555572628d6 in InternalCall (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, args=..., reason=1506078464) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:640
#13 0x0000555557277a1a in js::CallFromStack (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, args=..., reason=<optimized out>) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:645
#14 js::Interpret (cx=0x7ffff6439100, state=...) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:3060
#15 0x000055555725fe17 in MaybeEnterInterpreterTrampoline (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, cx@entry=0x7ffff6439100, state=...) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:393
#16 0x000055555725fb0a in js::RunScript (cx=cx@entry=0x7ffff6439100, state=...) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:451
#17 0x0000555557264c8d in js::ExecuteKernel (cx=cx@entry=0x7ffff6439100, script=script@entry=..., envChainArg=envChainArg@entry=..., evalInFrame=evalInFrame@entry=..., result=result@entry=...) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:838
#18 0x00005555572655ad in js::Execute (cx=cx@entry=0x7ffff6439100, script=script@entry=..., envChain=..., rval=rval@entry=...) at /home/uuu/gecko-dev.git/js/src/vm/Interpreter.cpp:870
#19 0x00005555574a600a in ExecuteScript (cx=cx@entry=0x7ffff6439100, envChain=..., script=..., rval=rval@entry=...) at /home/uuu/gecko-dev.git/js/src/vm/CompilationAndEvaluation.cpp:494
#20 0x00005555574a62eb in JS_ExecuteScript (cx=cx@entry=0x7ffff6439100, scriptArg=scriptArg@entry=...) at /home/uuu/gecko-dev.git/js/src/vm/CompilationAndEvaluation.cpp:518
#21 0x000055555719b267 in RunFile (cx=0x7ffff6439100, filename=<optimized out>, file=<optimized out>, compileMethod=CompileUtf8::DontInflate, compileOnly=false, fullParse=<optimized out>) at /home/uuu/gecko-dev.git/js/src/shell/js.cpp:1200
#22 0x000055555719a49e in Process (cx=cx@entry=0x7ffff6439100, filename=0x7ffff537c100 "program_20240312065635_09FF4BA7-2A22-48CF-97DC-75849C06DC9F_deterministic.js", forceTTY=<optimized out>, kind=kind@entry=FileScript) at /home/uuu/gecko-dev.git/js/src/shell/js.cpp:1780
#23 0x000055555714f153 in ProcessArgs (cx=0x7ffff6439100, op=0x7fffffffddb8) at /home/uuu/gecko-dev.git/js/src/shell/js.cpp:10991
#24 Shell (cx=0x7ffff6439100, op=op@entry=0x7fffffffddb8) at /home/uuu/gecko-dev.git/js/src/shell/js.cpp:11250
#25 0x00005555571477d9 in main (argc=argc@entry=2, argv=argv@entry=0x7fffffffe048) at /home/uuu/gecko-dev.git/js/src/shell/js.cpp:11751
#26 0x00007ffff7829d90 in __libc_start_call_main (main=main@entry=0x555557147070 <main(int, char**)>, argc=argc@entry=2, argv=argv@entry=0x7fffffffe048) at ../sysdeps/nptl/libc_start_call_main.h:58
#27 0x00007ffff7829e40 in __libc_start_main_impl (main=0x555557147070 <main(int, char**)>, argc=2, argv=0x7fffffffe048, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe038) at ../csu/libc-start.c:392
#28 0x0000555557110879 in _start ()

Comment 1

[Ask]

tested at commit a3d5a112ddb2d665b0c7ac2919b6f4fc6c97366c

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Likely this is just a set of options that can't be put together; Bryan, could you take a look at this one?

Comment 3

[Bryan Thrall [:bthrall]]

Will do!

Comment 4

[Nicolas B. Pierron [:nbp]]

I agree, this is a consistency check to assert that one does not set the discardSource without removing the lazy parsing, made to report bugs to the caller. The discardSource feature should not be exposed to the Web.

Comment 5

[Bryan Thrall [:bthrall]]

This appears to be happening because of the realm used to initialize the CompileOptions is not the same as the realm used to compile.

Because this behavior is in the DebuggerObject::CallData::createSource() function (part of the Debugger API), I don't think we need to treat it as a security issue.

It also looks like the discardDataSource option is only used in self-hosted and system principal code, or sandboxes in ways that look like it is dead code. If it is as dead as it looks, we might be able to remove it. (Thanks to :mgaudet for this analysis)

Comment 6

[Bryan Thrall [:bthrall]]

Attachment: Bug 1884837 - Inherit realm options from the referent r=arai!

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to wh0tlif3 from comment #1)

tested at commit a3d5a112ddb2d665b0c7ac2919b6f4fc6c97366c

On these bugs please also include the command-line options you are passing to jsshell

Comment 8

[Daniel Veditz [:dveditz]]

Bryan: sounds like it's not a vulnerability, so we can unhide the bug? Or did I misunderstand your comments?

Comment 9

[Bryan Thrall [:bthrall]]

:dveditz, yes, that is my understanding. We can unhide the bug because the crash can only happen through the Debugger API.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f1fbe41260cc
Use same realm for CompileOptions as used to compile r=arai

Comment 11

[Narcis Beleuzu [:NarcisB]]

Backed out for dt failure on browser_dbg-features-asm.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/e98d395467c4df44e6f3098474513e8c3e894e2c
Log link: https://treeherder.mozilla.org/logviewer?job_id=451355750&repo=autoland&lineNumber=2851

Comment 12

[Bryan Thrall [:bthrall]]

Apparently, setting the realm used to compile also disables asm.js compilation because the referent's realm is being observed by the debugger (for the browser_dbg-features-asm.js test). Disabling asm.js compilation apparently prevents line 7 in devtools/client/debugger/test/mochitest/examples/asm.js from being breakable, which causes the test to fail (see browser_dbg-features-asm.js line 67).

:ochameau should asm.js lines be breakable even when they are treated as JavaScript instead of asm.js-compiled?

Comment 13

[Alexandre Poirot [:ochameau]]

(In reply to Bryan Thrall [:bthrall] from comment #12)

Apparently, setting the realm used to compile also disables asm.js compilation because the referent's realm is being observed by the debugger (for the browser_dbg-features-asm.js test). Disabling asm.js compilation apparently prevents line 7 in devtools/client/debugger/test/mochitest/examples/asm.js from being breakable, which causes the test to fail (see browser_dbg-features-asm.js line 67).

:ochameau should asm.js lines be breakable even when they are treated as JavaScript instead of asm.js-compiled?

TBH, I know very little to asm.js. Debugging asm.js isn't having any knowledgeable maintainer in DevTools team. We should do what makes sense.

I wrote this test a while ago in order to assert the current behavior, highlighting the miss of breakable lines before reload.
If we can have the breakable lines right after opening DevTools and before reloading, then that's great improvement!
But I'm wondering about the final impacts of this patch:

• Does it really disable asm.js only when DevTools are opened? Couldn't it disable asm.js without devtools?
• Why do we now correctly get the breakable line before reloading?
• Why weren't we getting breakble line before reloading without this patch?
  Regarding the two last questions, it is as if observedAsmJS had to be set before the source was compiled, while with your patch it is no longer related to this flag, or this flag is more dynamic?

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:bthrall, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 15

[Bryan Thrall [:bthrall]]

(In reply to Alexandre Poirot [:ochameau] from comment #13)

I wrote this test a while ago in order to assert the current behavior, highlighting the miss of breakable lines before reload.
If we can have the breakable lines right after opening DevTools and before reloading, then that's great improvement!

It is correct not to have breakable lines for asm.js code right after opening DevTools, and my patch only changes how createSource() compiles the source; it doesn't make it possible to break on the asm.js code that is running.

The current behavior goes like this: an asm.js file is compiled to WASM, then DevTools is opened and createSource() is called. createSource() compiles the asm.js source, and it doesn't detect that the DevTools is running. The JSScript returned from createSource() compiles the asm.js to WASM, and does not include breakpoints for the asm.js. Line 7 is not marked as breakable.

With my patch, it goes like this: an asm.js file is compiled to WASM, then DevTools is opened and createSource() is called. createSource() compiles the asm.js source, and because of the AutoRealm change, it detects that DevTools is running. The JSScript returned from createSource() compiles the asm.js as Javascript, and does include breakpoints for the asm.js. Line 7 is marked breakable.

However, with my patch, the asm.js JSScript that is executing is still the version that was compiled to WASM, so it is not possible to break on line 7.
In fact, I don't think there is a way to tell inside createSource() how the original asm.js JSScript was compiled.

But I'm wondering about the final impacts of this patch:

• Does it really disable asm.js only when DevTools are opened? Couldn't it disable asm.js without devtools?

Yes, I think it is only disabling asm.js when DevTools are opened.

We could disable asm.js always, but then the asm.js lines in DevTools would allow setting breakpoints and the breakpoints would not work.

We could enable asm.js always, but then the asm.js lines would never be breakable, even when the javascript.options.asmjs pref is false.

• Why do we now correctly get the breakable line before reloading?

The line is breakable, but it is not correct. The executing asm.js code was still compiled to WASM and we cannot set breakpoints on it.

• Why weren't we getting breakble line before reloading without this patch?

Basically, because without this patch, we aren't checking if the debugger is observing the WASM code. The patch provides a realm when the CompileOptions are constructed, which allows the constructor to check that condition.

Regarding the two last questions, it is as if observedAsmJS had to be set before the source was compiled, while with your patch it is no longer related to this flag, or this flag is more dynamic?

I think when createSource() is called, it doesn't know how the asm.js was compiled earlier, so it doesn't know what options to use to compile it now so we get the correct set of breakpoints.

The resolution is likely that we need to figure out how the asm.js was originally compiled so we can correctly recompile it, though I wonder if there's a way to avoid recompiling it somehow.

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.

Comment 17

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Bryan, any hope of at least an updated patch soon?

Comment 18

[Bryan Thrall [:bthrall]]

I think I can provide a patch that maintains the current behavior for browser_dbg-features-asm.js and avoids the assertion failure, but it is ugly and not a complete solution. We could create a bug to follow up on a better solution.

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1884837 - Part 1: Add forceEnableAsmJS option to Debugger.Object.prototype.createSource. r?bthrall!

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1884837 - Part 2: Use forceEnableAsmJS option in Debugger.Object.prototype.createSource when there wasm source is found. r?#devtools-reviewers!

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1884837 - Part 3: Create CompileOption in the debuggee global in Debugger.Object.prototype.createSource. r?bthrall!

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: Bug 1884837 - Part 4: Add tests. r?bthrall!

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0a1509e1717
Part 1: Add forceEnableAsmJS option to Debugger.Object.prototype.createSource. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/ed382634c391
Part 2: Use forceEnableAsmJS option in Debugger.Object.prototype.createSource when there wasm source is found. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/1018b22dd35a
Part 3: Create CompileOption in the debuggee global in Debugger.Object.prototype.createSource. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/e0230379bdba
Part 4: Add tests. r=bthrall

Comment 25

[Atila Butkovits]

Backed out for causing SM bustages at forceEnableAsmJS.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/b07ea9fae75ebdf728bb11988a119548e026eed1

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=e0230379bdbabb5fed9f89510df3602f54ab1893&selectedTaskRun=V5-8MAUlRMC_HawEHb976Q.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=489543428&repo=autoland&lineNumber=36637

Comment 26

[Tooru Fujisawa [:arai]]

the test needs to be disabled when wasm isn't available

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/004bcd8b119d
Part 1: Add forceEnableAsmJS option to Debugger.Object.prototype.createSource. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/6da9ac1a431f
Part 2: Use forceEnableAsmJS option in Debugger.Object.prototype.createSource when there wasm source is found. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/df71edc6878d
Part 3: Create CompileOption in the debuggee global in Debugger.Object.prototype.createSource. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/d29c7058d2a9
Part 4: Add tests. r=bthrall

Comment 28

[Atila Butkovits]

Backed out for causing SM bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/1281fc347db21e059c68ece5d3a0679f254bb5ee

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=d29c7058d2a9994de051e5b8f4c0c87f2eaa9686&selectedTaskRun=TE3p3eh5Tuq5DPeOQk5Iww.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=489571337&repo=autoland&lineNumber=25881

Comment 29

[Tooru Fujisawa [:arai]]

okay, so the cgc affects the testcase, given the expectation there depends on GC not being performed before the assertion.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5903b2db8c64
Part 1: Add forceEnableAsmJS option to Debugger.Object.prototype.createSource. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/c4549513725e
Part 2: Use forceEnableAsmJS option in Debugger.Object.prototype.createSource when there wasm source is found. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/47628d90bbe2
Part 3: Create CompileOption in the debuggee global in Debugger.Object.prototype.createSource. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/9cd97d6710f1
Part 4: Add tests. r=bthrall

Comment 31

[amarc]

https://hg.mozilla.org/mozilla-central/rev/5903b2db8c64
https://hg.mozilla.org/mozilla-central/rev/c4549513725e
https://hg.mozilla.org/mozilla-central/rev/47628d90bbe2
https://hg.mozilla.org/mozilla-central/rev/9cd97d6710f1