1887176 - TokenStreamAnyChars::fillExceptingContext: correctly handle wasm frames

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[lukas.bernhard]

Attachment: crash.js

Steps to reproduce:

The attached sample asserts in the js-shell when invoked as obj-x86_64-pc-linux-gnu/dist/bin/js --fuzzing-safe crash.js.
Setting s-s as a precaution, could be some harmless shell-builtin.

#0  JS::TaggedColumnNumberOneOrigin::toLimitedColumnNumber (this=<optimized out>)
    at obj-x86_64-pc-linux-gnu/dist/include/js/ColumnNumber.h:396
#1  0x0000555557e40006 in js::frontend::TokenStreamAnyChars::fillExceptingContext (
    this=<optimized out>, err=err@entry=0x7fffffffa380, offset=<optimized out>)
    at js/src/frontend/TokenStream.cpp:1492
#2  0x0000555557e63360 in js::frontend::GeneralTokenStreamChars<char16_t, js::frontend::ParserAnyCharsAccess<js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t> > >::fillExceptingContext (this=this@entry=0x7fffffffb098, err=0x0, err@entry=0x7fffffffa380, offset=4154455875,
    offset@entry=181) at js/src/frontend/TokenStream.h:2008
#3  0x0000555557e76346 in js::frontend::TokenStreamSpecific<char16_t, js::frontend::ParserAnyCharsAccess<js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t> > >::computeErrorMetadata (this=0x7fffffffb090, err=0x7fffffffa380, errorOffset=...)
    at js/src/frontend/TokenStream.cpp:1657
#4  0x0000555557cfb0ec in js::frontend::ErrorReportMixin::warningWithNotesAtVA (
    this=0x7fffffffac20, offset=..., args=0x7fffffffa3b0, notes=..., errorNumber=<optimized out>)
    at js/src/frontend/ErrorReporter.h:204
#5  js::frontend::ErrorReportMixin::warningAt (this=this@entry=0x7fffffffac20,
    offset=offset@entry=181, errorNumber=errorNumber@entry=275)
    at js/src/frontend/ErrorReporter.h:181
#6  0x0000555557d30b6a in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::statementList (this=this@entry=0x7fffffffac20,
    yieldHandling=yieldHandling@entry=js::frontend::YieldIsName)
    at js/src/frontend/Parser.cpp:4040
#7  0x0000555557d3a477 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionBody (this=this@entry=0x7fffffffac20, inHandling=inHandling@entry=js::frontend::InAllowed,
    yieldHandling=js::frontend::YieldIsName,
    kind=kind@entry=js::frontend::FunctionSyntaxKind::Method,
    type=js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::StatementListBody)
    at js/src/frontend/Parser.cpp:2421
#8  0x0000555557d3911e in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionFormalParametersAndBody (this=this@entry=0x7fffffffac20,
    inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=<optimized out>,
    funNode=funNode@entry=0x7fffffffa5c8, 
    kind=kind@entry=js::frontend::FunctionSyntaxKind::Method, parameterListEnd=...,
    isStandaloneFunction=<optimized out>)
    at js/src/frontend/Parser.cpp:3566
#9  0x0000555557d6f6c6 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::standaloneLazyFunction (this=0x7fffffffac20, input=..., toStringStart=<optimized out>, strict=false,
    generatorKind=<optimized out>, asyncKind=<optimized out>)
    at js/src/frontend/Parser.cpp:3398
#10 0x0000555557db5112 in CompileLazyFunctionToStencilMaybeInstantiate<char16_t> (
    maybeCx=0x7ffff6039100, fc=fc@entry=0x7fffffffb7f8, tempLifoAlloc=..., input=...,
    scopeCache=scopeCache@entry=0x7ffff60388d8, 
    units=0x7ffff60c8036 u"() {\n", ' ' <repeats 12 times>, "function F2() {\n", ' ' <repeats 16 times>, "if (!new.target) { throw 'must be called with new'; }\n", ' ' <repeats 12 times>, "}\n", ' ' <repeats 12 times>, "return F2();\n", ' ' <repeats 12 times>, "return {};\n        },\n    };\n\n    const o7 = {"..., length=174, output=...)
    at js/src/frontend/BytecodeCompiler.cpp:1455
#11 0x0000555557d81701 in DelazifyCanonicalScriptedFunctionImpl<char16_t> (cx=0x7ffff6039100,
    fc=0x7fffffffb7f8, scopeCache=0x7ffff60388d8, fun=..., lazy=..., ss=0x7ffff6042820)
    at js/src/frontend/BytecodeCompiler.cpp:1597
#12 js::frontend::DelazifyCanonicalScriptedFunction (cx=cx@entry=0x7ffff6039100,
    fc=fc@entry=0x7fffffffb7f8, fun=fun@entry=...)
    at js/src/frontend/BytecodeCompiler.cpp:1624
#13 0x00005555575e7201 in JSFunction::delazifyLazilyInterpretedFunction (cx=0x7ffff6039100, fun=...)
    at js/src/vm/JSFunction.cpp:1194
#14 0x00005555571ede76 in JSFunction::getOrCreateScript (cx=0x7ffff6039100, fun=...) at js/src/vm/JSFunction.h:493
#15 0x00005555575e7082 in JSFunction::delazifyLazilyInterpretedFunction (cx=0x7ffff6039100, fun=...)
    at js/src/vm/JSFunction.cpp:1181
#16 0x00005555571ede76 in JSFunction::getOrCreateScript (cx=0x7ffff6039100, fun=...) at js/src/vm/JSFunction.h:493
#17 0x000055555728cf43 in js::InternalCallOrConstruct (cx=0x7ffff6039100, args=..., construct=construct@entry=js::NO_CONSTRUCT,
    reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:584
#18 0x000055555728ef56 in InternalCall (cx=0x7ffff7a008e0 <_IO_stdfile_2_lock>, cx@entry=0x7ffff6039100, args=..., reason=1506616576,
    reason@entry=js::CallReason::Call) at js/src/vm/Interpreter.cpp:640
#19 0x000055555728f13e in js::Call (cx=cx@entry=0x7ffff6039100, fval=fval@entry=..., thisv=thisv@entry=..., args=..., rval=rval@entry=...,
    reason=reason@entry=js::CallReason::Call) at js/src/vm/Interpreter.cpp:672
#20 0x0000555558ae5142 in js::wasm::Instance::callImport (this=0x7ffff4e8fc00, cx=0x7ffff6039100, funcImportIndex=<optimized out>,
    argc=<optimized out>, argv=0x7fffffffbe60) at js/src/wasm/WasmInstance.cpp:298
#21 0x0000555558ae6214 in js::wasm::Instance::callImport_general (instance=0x7ffff7a008e0 <_IO_stdfile_2_lock>, funcImportIndex=-140511421,
    argc=1506616576, argv=0x0) at js/src/wasm/WasmInstance.cpp:349
#22 0x0000131bba3ef199 in ?? ()
#23 0x00007ffff4f84c64 in ?? ()
#24 0xf4f84c0000000000 in ?? ()
#25 0x00007fffffffbea0 in ?? ()
#26 0x0000131bba3ef10f in ?? ()
#27 0x00007ffff4e8fc00 in ?? ()
#28 0x00007ffff4e8fc00 in ?? ()
#29 0x00007fffffffc560 in ?? ()
#30 0x0000138800000000 in ?? ()
#31 0x00007fffffffbec0 in ?? ()
#32 0x0000131bba3ef0a4 in ?? ()
#33 0x0000000000000050 in ?? ()
#34 0x00005555587a0911 in js::gc::MaybeForwarded<JSScript*> (t=0x7ffff4f80300) at js/src/gc/Marking-inl.h:122

Comment 1

[Julian Seward [:jseward]]

Here's a reduced test case:

const v0 = `
    const o6 = {
        f() {
            function F2() {
                if (!new.target) { throw 'must be called with new'; }
            }
            return F2();
            return {}; // This can be anything, but it must be present
        },
    };

    const o7 = {
        "main": o6,
    };

    const v15 = new WebAssembly.Module(wasmTextToBinary(\`
    (module
        (import "main" "f" (func))
        (func (export "go")
            call 0
        )
    )\`));
    const v16 = new WebAssembly.Instance(v15, o7);
    v16.exports.go();
`;

const o27 = {
    // Both "fileName" and null are necessary
    "fileName": null,
};
evaluate(v0, o27);

I can reproduce this without needing --fuzzing-safe.

This is somewhat confusing because of the interleaved compilation and
execution. Basically, execution ends by throwing an exception of some kind
(I'm not sure what), and it seems to me that the assertion happens when trying
to assemble a message describing the exception. I believe the exception is
caused by the extra return {} in v0/o6/f/F2; changing that to (eg)
let x = 1 also causes the failure, but removing that line makes it go away.

Possibly related is o27 (the environment arg for evaluate). Changing
either the field name or the null makes the failure disappear. Changing the
null to "greencheese.js" produces:

greencheese.js:5:36 uncaught exception: must be called with new
Stack:
  [e-mail]:5:36
  [e-mail]:7:20
  @greencheese.js line 16 > WebAssembly.Module:wasm-function[1]:0x2b
  @greencheese.js:24:17
  @/nfs/compx/testcase1887176.js:31:9

Comment 2

[Julian Seward [:jseward]]

After conferring with Arai and Jan, this doesn't need to be s-s. In the worst
case it will cause a column number of 0x80000000 | wasm-function-index to be
baked in to an error message, instead of a valid column number.

Comment 3

[Julian Seward [:jseward]]

Attachment: Bug 1887176 - TokenStreamAnyChars::fillExceptingContext: correctly handle wasm frames. r=arai.

TokenStreamAnyChars::fillExceptingContext assumes, when trying to get location
information from the caller, that the frame isn't a wasm frame. This leads to
an assertion failure in isLimitedColumnNumber.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/782dc76d8de3
TokenStreamAnyChars::fillExceptingContext: correctly handle wasm frames.  r=arai.

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/782dc76d8de3