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

RESOLVED FIXED in Firefox 54

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: decoder, Assigned: arai)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla55
x86_64
Linux
assertion, jsbugmon, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 fixed, firefox55 fixed)

Details

(Whiteboard: [jsbugmon:update,bisect])

Attachments

(2 attachments)

(Reporter)

Description

a year ago
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
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?
(Assignee)

Comment 2

a year ago
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.
Blocks: 1317400
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox55: --- → affected
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → unaffected
(Assignee)

Comment 3

a year ago
Created attachment 8844750 [details] [diff] [review]
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.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8844750 - Flags: review?(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
Attachment #8844750 - Flags: review?(till) → review+
(Assignee)

Comment 5

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

Comment 6

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

thank you for reviewing :)

I'll ask approval later.
Attachment #8844894 - Flags: review+

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/dc0814eb54f8
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 8

a year ago
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
Attachment #8844894 - Flags: approval-mozilla-aurora?
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+.
Attachment #8844894 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 10

a year ago
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
Hi :arai,
Thanks for clarifying the user impact. This is still worth uplifting to Aurora54.

Comment 12

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc2ab961df0d
status-firefox54: affected → fixed
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.