1345162 - Assertion failure: fun->explicitName() == nullptr, at js/src/frontend/Parser.cpp:2469 with asm.js

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 966464a68a2c (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

function set(stdlib, foreign, heap) {
    "use asm";
    var ffi = foreign.t;
    return {};
}
set(15, 10);


Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x00000000004aa45b in js::frontend::Parser<js::frontend::FullParseHandler>::standaloneFunction (this=this@entry=0x7fffffffcc88, fun=fun@entry=..., enclosingScope=..., enclosingScope@entry=..., parameterListEnd=..., generatorKind=generatorKind@entry=js::NotGenerator, asyncKind=asyncKind@entry=js::SyncFunction, inheritedDirectives=..., newDirectives=0x7fffffffc330) at js/src/frontend/Parser.cpp:2469
#0  0x00000000004aa45b in js::frontend::Parser<js::frontend::FullParseHandler>::standaloneFunction (this=this@entry=0x7fffffffcc88, fun=fun@entry=..., enclosingScope=..., enclosingScope@entry=..., parameterListEnd=..., generatorKind=generatorKind@entry=js::NotGenerator, asyncKind=asyncKind@entry=js::SyncFunction, inheritedDirectives=..., newDirectives=0x7fffffffc330) at js/src/frontend/Parser.cpp:2469
#1  0x0000000000a82420 in BytecodeCompiler::compileStandaloneFunction (this=this@entry=0x7fffffffc690, fun=fun@entry=..., generatorKind=generatorKind@entry=js::NotGenerator, asyncKind=asyncKind@entry=js::SyncFunction, parameterListEnd=...) at js/src/frontend/BytecodeCompiler.cpp:492
#2  0x0000000000a8274b in js::frontend::CompileStandaloneFunction (cx=cx@entry=0x7ffff6950000, fun=fun@entry=..., options=..., srcBuf=..., parameterListEnd=..., enclosingScope=..., enclosingScope@entry=...) at js/src/frontend/BytecodeCompiler.cpp:733
#3  0x0000000000c98a7a in HandleInstantiationFailure (metadata=..., args=..., cx=0x7ffff6950000) at js/src/wasm/AsmJS.cpp:8099
#4  InstantiateAsmJS (cx=0x7ffff6950000, argc=<optimized out>, vp=<optimized out>) at js/src/wasm/AsmJS.cpp:8131
#5  0x0000000000540e30 in js::CallJSNative (cx=cx@entry=0x7ffff6950000, native=0xc964e0 <InstantiateAsmJS(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:282
[...]
#18 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8452
rax	0x0	0
rbx	0x7fffffffcc88	140737488342152
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffc2f0	140737488339696
rsp	0x7fffffffc080	140737488339072
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7fffffffcca0	140737488342176
r13	0x0	0
r14	0x7fffffffc0c0	140737488339136
r15	0x7fffffffd490	140737488344208
rip	0x4aa45b <js::frontend::Parser<js::frontend::FullParseHandler>::standaloneFunction(JS::Handle<JSFunction*>, JS::Handle<js::Scope*>, mozilla::Maybe<unsigned int> const&, js::GeneratorKind, js::FunctionAsyncKind, js::frontend::Directives, js::frontend::Directives*)+843>
=> 0x4aa45b <js::frontend::Parser<js::frontend::FullParseHandler>::standaloneFunction(JS::Handle<JSFunction*>, JS::Handle<js::Scope*>, mozilla::Maybe<unsigned int> const&, js::GeneratorKind, js::FunctionAsyncKind, js::frontend::Directives, js::frontend::Directives*)+843>:	movl   $0x0,0x0
   0x4aa466 <js::frontend::Parser<js::frontend::FullParseHandler>::standaloneFunction(JS::Handle<JSFunction*>, JS::Handle<js::Scope*>, mozilla::Maybe<unsigned int> const&, js::GeneratorKind, js::FunctionAsyncKind, js::frontend::Directives, js::frontend::Directives*)+854>:	ud2

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

asm.js compilation succeeds, but linking fails (because the given `foreign` argument is not an object), so we recompile the script as usual. The token stream yields a "SET" token instead of the function name, causing this error. Arai, since you've introduced this new assertion and landed this code recently, maybe you know what's going on here?

Comment 2

[Tooru Fujisawa [:arai]]

thanks!

it's a conflict between my 2 patches (bug 1317400 and bug 1336783)
that I forgot to apply the change from bug 1336783 to bug 1317400 patch.
I'll fix it shortly.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Fix identifier handling in Parser::standaloneFunction to follow new token kinds.

Bug 1336783 changed the identifier handling, and now we generate more token kinds than TOK_NAME/TOK_YIELD for identifier (like, TOK_SET for this case), and now we need to check with TokenKindIsPossibleIdentifierName.

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 8844750 [details] [diff] [review]
Fix identifier handling in Parser::standaloneFunction to follow new token kinds.

Review of attachment 8844750 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: js/src/jit-test/tests/parser/standalone-function-name.js
@@ +13,5 @@
> +test("async");
> +test("await");
> +test("each");
> +test("from");
> +test("from");

Nit: duplicate

Comment 5

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc0814eb54f8e9507572cc80c58c5ceb344b8907
Bug 1345162 - Fix identifier handling in Parser::standaloneFunction to follow new token kinds. r=till

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: (mozilla-aurora) Fix identifier handling in Parser::standaloneFunction to follow new token kinds. r=till

thank you for reviewing :)

I'll ask approval later.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/dc0814eb54f8

Comment 8

[Tooru Fujisawa [:arai]]

Comment on attachment 8844894 [details] [diff] [review]
(mozilla-aurora) Fix identifier handling in Parser::standaloneFunction to follow new token kinds. r=till

Approval Request Comment
> [Feature/Bug causing the regression]:
bug 1317400

> [User impact if declined]:
a bit of unnecessary memory consumption

> [Is this code covered by automated tests?]:
not by dedicated test for this, but this path is used by many tests

> [Has the fix been verified in Nightly?]:
yes

> [Needs manual test from QE? If yes, steps to reproduce]:
no

> [List of other uplifts needed for the feature/fix]:
none

> [Is the change risky?]:
no

> [Why is the change risky/not risky?]:
removed the allocation of unnecessary object

> [String changes made/needed]:
none

Comment 9

[Gerry Chang [:gchang]]

Comment on attachment 8844894 [details] [diff] [review]
(mozilla-aurora) Fix identifier handling in Parser::standaloneFunction to follow new token kinds. r=till

Fix a memory consumption issue. Aurora54+.

Comment 10

[Tooru Fujisawa [:arai]]

I'm so sorry that the approval comment is for wrong patch.

Approval Request Comment
> [Feature/Bug causing the regression]:
bug 1317400

> [User impact if declined]:
wrong behavior (rejects correct syntax) in JavaScript in opt build, and assertion failure in debug build.

> [Is this code covered by automated tests?]:
yes

> [Has the fix been verified in Nightly?]:
yes

> [Needs manual test from QE? If yes, steps to reproduce]:
no

> [List of other uplifts needed for the feature/fix]:
none

> [Is the change risky?]:
no

> [Why is the change risky/not risky?]:
this patch changes the code to accept more correct syntaxes, that was mis-handled by the bug.

> [String changes made/needed]:
none

Comment 11

[Gerry Chang [:gchang]]

Hi :arai,
Thanks for clarifying the user impact. This is still worth uplifting to Aurora54.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dc2ab961df0d