Closed Bug 1566974 Opened 6 months ago Closed 6 months ago

Assertion failure: !lazyScript->treatAsRunOnce(), at js/src/frontend/BytecodeEmitter.cpp:2220

Categories

(Core :: JavaScript Engine, defect, P1, critical)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- fixed

People

(Reporter: gkw, Assigned: tcampbell)

References

(Blocks 2 open bugs, Regression)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(2 files, 1 obsolete file)

The following testcase crashes on mozilla-central revision 4d6f456872e1 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// jsfunfuzz-generated
(() => {
    // Adapted from randomly chosen test: js/src/tests/test262/annexB/language/eval-code/direct/func-switch-case-eval-func-existing-block-fn-no-init.js
    (function() {
        eval('}');
    }());
})();

Backtrace:

#0  js::frontend::BytecodeEmitter::isRunOnceLambda (this=0x7ffd20b168e8) at js/src/frontend/BytecodeEmitter.cpp:2219
#1  0x000055a641c7db7a in js::frontend::FunctionScriptEmitter::prepareForParameters (this=0x7ffd20b164f0) at js/src/frontend/FunctionEmitter.cpp:400
#2  0x000055a641c2df73 in js::frontend::BytecodeEmitter::emitFunctionScript (this=0x7ffd20b168e8, funNode=0x7f7937f58020, isTopLevel=js::frontend::BytecodeEmitter::TopLevelFunction::Yes) at js/src/frontend/BytecodeEmitter.cpp:2495
#3  0x000055a641c2c19e in CompileLazyFunctionImpl<char16_t> (cx=<optimized out>, lazy=..., units=0x7f7937fce92a u"() {\n        eval('}');\n    }());\n})();\n", length=29) at js/src/frontend/BytecodeCompiler.cpp:977
#4  js::frontend::CompileLazyFunction (cx=<optimized out>, lazy=..., units=<optimized out>, length=<optimized out>) at js/src/frontend/BytecodeCompiler.cpp:991
#5  0x000055a641669225 in JSFunction::createScriptForLazilyInterpretedFunction (cx=0x7f7938119000, fun=...) at js/src/vm/JSFunction.cpp:1643
/snip

For detailed crash information, see attachment.

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/61e233285734
user: Ted Campbell
date: Mon Jul 15 12:41:39 2019 +0000
summary: Bug 1565945 - Compute LazyScript::TreatAsRunOnce flag the same as JSScript. r=jandem

Ted, is bug 1565945 a likely regressor?

Flags: needinfo?(tcampbell)
Regressed by: 1565945
Type: task → defect
Assignee: nobody → tcampbell
Status: NEW → ASSIGNED
Flags: needinfo?(tcampbell)
Priority: -- → P1

The funbox that is used for inner functions of a delazifying parse are
largely incomplete and information still needed the original lazy parse
must be pass on script flags. This adds a new flag to indicate that the
LazyScript will not support the run-once optimization even if the parent
context would allow it. This fixes a regression introduced while trying
to make LazyScript::treatAsRunOnce and JSScript::treatAsRunOnce more
consistently defined.

The issue was my misunderstanding of the relation between the inner functionbox during delazification and the original functionbox that generated the inner lazy script. My fix is to add a flag to communicate what is last in the reconstructed function box.

In the future, it might make more sense to move this whole thing to purely the first parse the BCE provides little analysis value here.

This partially reverts a patch in Bug 1565945 that tried to define
LazyScript::TreatAsRunOnce in the same way to JSScript. The limitation
is that while delazifying, the inner lazy functions don't have a
complete function box. The patch reverts behaviour so that
LazyScript::TreatAsRunOnce matches emittingRunOnceContext, and the
shouldSuppressRunOnce condition is checked later.

I've posted an alternate fix which simply reverts the specific bug from before. If there is uncertainty about the first patch, I'm happy to use the partial-revert to fix the fuzzbug.

Attachment #9078931 - Attachment is obsolete: true
Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e821cf48d781
Partially revert LazyScript::TreatAsRunOnce behaviour r=jorendorff

There are BinAST failures with the first patch, so I'll go with the partial backout. Need week I may see if I can make the parser generate this flag directly (by emulating BCE::inLoop()).

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