1644536 - Assertion failure: !fun->baseScript()->getFieldInitializers().valid, at frontend/Parser.cpp:2738

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200608-63dc5e9b1b02 (debug build, run with --fuzzing-safe --ion-offthread-compile=off):

function evalWithCache(code, ctx) {
  code = code instanceof Object ? code : cacheEntry(code);
  ctx.global = newGlobal();
  ctx_save = Object.create(ctx, { saveIncrementalBytecode: { value: true } });
  ctx.global.generation = 0;
  var res1 = evaluate(code, ctx_save);
  ctx.global.generation = 1;
  var res2 = evaluate(code, Object.create(ctx_save, { loadBytecode: { value: true } }));
}
evalWithCache(`
    function f(x) {
        function ifTrue() {};
        function ifFalse() {
            class y {
                constructor() {}
            }
        };
        if (x) return ifTrue();
        else  return ifFalse();
    }
    f((generation % 2) == 0);
`, {});

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000555556147649 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::skipLazyInnerFunction(js::frontend::FunctionNode*, unsigned int, js::frontend::FunctionSyntaxKind, bool) ()
#0  0x0000555556147649 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::skipLazyInnerFunction(js::frontend::FunctionNode*, unsigned int, js::frontend::FunctionSyntaxKind, bool) ()
#1  0x0000555556150d89 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionDefinition(js::frontend::FunctionNode*, unsigned int, js::frontend::InHandling, js::frontend::YieldHandling, JS::Handle<JSAtom*>, js::frontend::FunctionSyntaxKind, js::GeneratorKind, js::FunctionAsyncKind, bool) ()
#2  0x000055555615cf5b in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::methodDefinition(unsigned int, js::frontend::PropertyType, JS::Handle<JSAtom*>) ()
#3  0x000055555615f206 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::classMember(js::frontend::YieldHandling, js::frontend::ParseContext::ClassStatement const&, JS::Handle<js::PropertyName*>, unsigned int, js::frontend::HasHeritage, js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::ClassFields&, js::frontend::ListNode*&, bool*) ()
#4  0x000055555614d22c in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::classDefinition(js::frontend::YieldHandling, js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::ClassContext, js::frontend::DefaultHandling) ()
#5  0x0000555556147893 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementListItem(js::frontend::YieldHandling, bool) ()
#6  0x00005555561458b5 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementList(js::frontend::YieldHandling) ()
#7  0x000055555614f4f4 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionSyntaxKind, js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::FunctionBodyType) ()
#8  0x000055555614e181 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionFormalParametersAndBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionNode**, js::frontend::FunctionSyntaxKind, mozilla::Maybe<unsigned int> const&, bool) ()
#9  0x000055555617ecaf in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::standaloneLazyFunction(JS::Handle<JSFunction*>, unsigned int, bool, js::GeneratorKind, js::FunctionAsyncKind) ()
#10 0x000055555618fb56 in js::frontend::CompileLazyFunction(JSContext*, JS::Handle<js::BaseScript*>, char16_t const*, unsigned long) ()
#11 0x0000555555c2e651 in JSFunction::delazifyLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) ()
#12 0x00005555557f5459 in JSFunction::getOrCreateScript(JSContext*, JS::Handle<JSFunction*>) ()
#13 0x0000555555c2e2dd in JSFunction::delazifyLazilyInterpretedFunction(JSContext*, JS::Handle<JSFunction*>) ()
#14 0x00005555557f5459 in JSFunction::getOrCreateScript(JSContext*, JS::Handle<JSFunction*>) ()
#15 0x000055555592d600 in Interpret(JSContext*, js::RunState&) ()
#16 0x0000555555923be2 in js::RunScript(JSContext*, js::RunState&) ()
#17 0x000055555593b815 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) ()
#18 0x000055555593bf26 in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) ()
#19 0x0000555555b1b692 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) ()
#20 0x0000555555b1bbf8 in ExecuteScript(JSContext*, JS::Handle<JS::StackGCVector<JSObject*, js::TempAllocPolicy> >, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) ()
#21 0x00005555557d0bb4 in Evaluate(JSContext*, unsigned int, JS::Value*) ()
[...]
#34 0x00005555557afdcd in main ()
rax	0x555557069f97	93825020632983
rbx	0x0	0
rcx	0x555558369980	93825040554368
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffff91d0	140737488327120
rsp	0x7fffffff9170	140737488327024
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f9bd40	140737353727296
r10	0x58	88
r11	0x7ffff6dac7a0	140737334921120
r12	0x7ffff4afe240	140737298555456
r13	0x7ffff4afe200	140737298555392
r14	0x7fffffff9180	140737488327040
r15	0x7fffffff9e20	140737488330272
rip	0x555556147649 <js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::skipLazyInnerFunction(js::frontend::FunctionNode*, unsigned int, js::frontend::FunctionSyntaxKind, bool)+1049>
=> 0x555556147649 <_ZN2js8frontend6ParserINS0_16FullParseHandlerEDsE21skipLazyInnerFunctionEPNS0_12FunctionNodeEjNS0_18FunctionSyntaxKindEb+1049>:	movl   $0xab2,0x0
   0x555556147654 <_ZN2js8frontend6ParserINS0_16FullParseHandlerEDsE21skipLazyInnerFunctionEPNS0_12FunctionNodeEjNS0_18FunctionSyntaxKindEb+1060>:	callq  0x55555583f74e <abort>

It might be harmless, but I'll mark it s-s anyway for now, because it is unclear to me what potential impact this has (violating parser invariants through the load/saveBytecode feature).

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Nicolas B. Pierron [:nbp] {away}]

Ted can you investigate this issue?

SaveIncrementalBytecode followed by loadBytecode which get executed differently the second time is actually an expected behavior which can reproduced in the web with the JSBC. Getting a different execution can be achieved in the web by using a time/random-based condition.

Comment 3

[Ted Campbell [:tcampbell]]

This is a benign issue with a overly strict assert. I'll fix the XDR (and parser) code to be more consistent about these invariants. Will open up bug once I finish patch and am sure.

Comment 4

[Ted Campbell [:tcampbell]]

Assertion was checking for double-init, but the value is never read back before the real initialization so this doesn't have any issues in opt builds.

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1644536 - Cleanup FieldInitializers initialization. r?mgaudet

FieldInitializers is only defined for functions that have a compiled
enclosing script. We should avoid setting the value when it is not defined.
Update initFromLazyFunction so that fieldInitializers are only read for the
function being delazified. Fix XDR to not track fieldInitializers for lazy
functions with lazy parent functions by checking for exisitance of an
enclosingScope.

Also ensure fieldInitializers are set correctly when cloning scripts. In
practice, we delazify entire script tree before cloning but this is a footgun
none the less.

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1644536 - Avoid setting lazy function TI type during XDR. r?mgaudet

Similar to previous patch, this info is not set on functions with lazy
enclosing functions. Also, inline FunctionBox::setTypeForExposedFunctions
into its only caller.

Depends on D79282

Comment 7

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200611093454-10ad7868f3ca.
The bug appears to have been introduced in the following build range:
> Start: b03dd4aa0e28b0f52d8a6a1846c35d029f08ac40 (20200414134316)
> End: c289ac40b54681da3699feda5790410e3be00a58 (20200414134513)
> Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b03dd4aa0e28b0f52d8a6a1846c35d029f08ac40&tochange=c289ac40b54681da3699feda5790410e3be00a58

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cad26877b1f5
Cleanup FieldInitializers initialization. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/3d89878a55ee
Avoid setting lazy function TI type during XDR. r=mgaudet

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/cad26877b1f5
https://hg.mozilla.org/mozilla-central/rev/3d89878a55ee

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/cad26877b1f5
https://hg.mozilla.org/mozilla-central/rev/3d89878a55ee

Comment 11

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20200616154959-89a54069f124.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.