1196041 - Assertion failure: false (MOZ_ASSERT_UNREACHABLE: this token was previously looked up with a different modifier, potentially making tokenization non-deterministic), at js/src/frontend/TokenStream.h:531

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 90d9b7c391d3 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2):

Reflect.parse("(function() { class a { constructor() { } static get p() it: missing or incorrect StopIteration");



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x000000000042919c in js::frontend::TokenStream::verifyConsistentModifier (this=<optimized out>, modifier=js::frontend::Token::KeywordIsName, lookaheadToken=...) at js/src/frontend/TokenStream.h:529
#0  0x000000000042919c in js::frontend::TokenStream::verifyConsistentModifier (this=<optimized out>, modifier=js::frontend::Token::KeywordIsName, lookaheadToken=...) at js/src/frontend/TokenStream.h:529
#1  0x00000000004c22e0 in verifyConsistentModifier (this=0x7fffffffc710, modifier=js::frontend::Token::KeywordIsName, lookaheadToken=...) at js/src/frontend/TokenStream.h:506
#2  js::frontend::TokenStream::getToken (this=this@entry=0x7fffffffc710, ttp=ttp@entry=0x7fffffffa760, modifier=js::frontend::Token::KeywordIsName) at js/src/frontend/TokenStream.h:545
#3  0x00000000004f87ff in js::frontend::Parser<js::frontend::FullParseHandler>::propertyList (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, type=<optimized out>) at js/src/frontend/Parser.cpp:8479
#4  0x00000000004ce42a in js::frontend::Parser<js::frontend::FullParseHandler>::classDefinition (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, classContext=classContext@entry=js::frontend::Parser<js::frontend::FullParseHandler>::ClassStatement, defaultHandling=defaultHandling@entry=js::frontend::NameRequired) at js/src/frontend/Parser.cpp:6186
#5  0x00000000004f433a in js::frontend::Parser<js::frontend::FullParseHandler>::statement (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, canHaveDirectives=<optimized out>) at js/src/frontend/Parser.cpp:6374
#6  0x00000000004f47c2 in js::frontend::Parser<js::frontend::FullParseHandler>::statements (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName) at js/src/frontend/Parser.cpp:2991
#7  0x00000000004f4b6b in js::frontend::Parser<js::frontend::FullParseHandler>::functionBody (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, kind=kind@entry=js::frontend::Expression, type=type@entry=js::frontend::Parser<js::frontend::FullParseHandler>::StatementListBody) at js/src/frontend/Parser.cpp:1040
#8  0x00000000004f51e4 in js::frontend::Parser<js::frontend::FullParseHandler>::functionArgsAndBodyGeneric (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, pn=pn@entry=0x7ffff6994058, fun=fun@entry=..., kind=kind@entry=js::frontend::Expression) at js/src/frontend/Parser.cpp:2694
#9  0x00000000004cc609 in js::frontend::Parser<js::frontend::FullParseHandler>::functionArgsAndBody (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, pn=0x7ffff6994058, fun=..., fun@entry=..., kind=kind@entry=js::frontend::Expression, generatorKind=generatorKind@entry=js::NotGenerator, inheritedDirectives=..., newDirectives=newDirectives@entry=0x7fffffffb020) at js/src/frontend/Parser.cpp:2500
#10 0x00000000004f562a in js::frontend::Parser<js::frontend::FullParseHandler>::functionDef (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, funName=..., funName@entry=..., kind=kind@entry=js::frontend::Expression, generatorKind=generatorKind@entry=js::NotGenerator, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:2332
#11 0x00000000004f5b17 in js::frontend::Parser<js::frontend::FullParseHandler>::functionExpr (this=this@entry=0x7fffffffc6e0, invoked=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:2810
#12 0x00000000004f9dfa in js::frontend::Parser<js::frontend::FullParseHandler>::primaryExpr (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, tt=<optimized out>, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:8832
#13 0x00000000004fbc61 in js::frontend::Parser<js::frontend::FullParseHandler>::memberExpr (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, tt=js::frontend::TOK_FUNCTION, allowCallSyntax=allowCallSyntax@entry=true, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:8062
#14 0x00000000004fc944 in js::frontend::Parser<js::frontend::FullParseHandler>::unaryExpr (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:6986
#15 0x00000000004fcbb6 in js::frontend::Parser<js::frontend::FullParseHandler>::orExpr1 (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:6545
#16 0x00000000004fce0e in js::frontend::Parser<js::frontend::FullParseHandler>::condExpr1 (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:6597
#17 0x00000000004f604b in js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:6712
#18 0x00000000004f663f in js::frontend::Parser<js::frontend::FullParseHandler>::expr (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictInvoked) at js/src/frontend/Parser.cpp:6413
#19 0x00000000004fd215 in js::frontend::Parser<js::frontend::FullParseHandler>::parenExprOrGeneratorComprehension (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName) at js/src/frontend/Parser.cpp:8964
#20 0x00000000004f9db8 in js::frontend::Parser<js::frontend::FullParseHandler>::primaryExpr (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, tt=<optimized out>, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:8848
#21 0x00000000004fbc61 in js::frontend::Parser<js::frontend::FullParseHandler>::memberExpr (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, tt=js::frontend::TOK_LP, allowCallSyntax=allowCallSyntax@entry=true, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:8062
#22 0x00000000004fc944 in js::frontend::Parser<js::frontend::FullParseHandler>::unaryExpr (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:6986
#23 0x00000000004fcbb6 in js::frontend::Parser<js::frontend::FullParseHandler>::orExpr1 (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:6545
#24 0x00000000004fce0e in js::frontend::Parser<js::frontend::FullParseHandler>::condExpr1 (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:6597
#25 0x00000000004f604b in js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:6712
#26 0x00000000004f663f in js::frontend::Parser<js::frontend::FullParseHandler>::expr (this=this@entry=0x7fffffffc6e0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:6413
#27 0x00000000004f7253 in js::frontend::Parser<js::frontend::FullParseHandler>::expressionStatement (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:4733
#28 0x00000000004f416d in js::frontend::Parser<js::frontend::FullParseHandler>::statement (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, canHaveDirectives=<optimized out>) at js/src/frontend/Parser.cpp:6305
#29 0x00000000004f47c2 in js::frontend::Parser<js::frontend::FullParseHandler>::statements (this=this@entry=0x7fffffffc6e0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName) at js/src/frontend/Parser.cpp:2991
#30 0x00000000004fe82a in js::frontend::Parser<js::frontend::FullParseHandler>::parse (this=this@entry=0x7fffffffc6e0) at js/src/frontend/Parser.cpp:743
#31 0x0000000000581877 in reflect_parse (cx=0x7ffff6907000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/ReflectParse.cpp:3787
#32 0x00000000006d2812 in js::CallJSNative (cx=0x7ffff6907000, native=0x5806d0 <reflect_parse(JSContext*, uint32_t, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
#33 0x00000000006b9d12 in js::Invoke (cx=cx@entry=0x7ffff6907000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:773
#34 0x00000000006aca9a in Interpret (cx=cx@entry=0x7ffff6907000, state=...) at js/src/vm/Interpreter.cpp:3035
#35 0x00000000006b962b in js::RunScript (cx=cx@entry=0x7ffff6907000, state=...) at js/src/vm/Interpreter.cpp:714
#36 0x00000000006cc8b9 in js::ExecuteKernel (cx=cx@entry=0x7ffff6907000, script=..., script@entry=..., scopeChainArg=..., thisv=..., newTargetValue=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:955
#37 0x00000000006ccc33 in js::Execute (cx=cx@entry=0x7ffff6907000, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:989
#38 0x0000000000ad1feb in ExecuteScript (cx=cx@entry=0x7ffff6907000, scope=..., script=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4352
#39 0x0000000000ad210b in JS_ExecuteScript (cx=cx@entry=0x7ffff6907000, scriptArg=..., scriptArg@entry=...) at js/src/jsapi.cpp:4383
#40 0x0000000000428547 in RunFile (compileOnly=false, file=0x7ffff699e400, filename=0x7fffffffe050 "min.js", cx=0x7ffff6907000) at js/src/shell/js.cpp:459
#41 Process (cx=cx@entry=0x7ffff6907000, filename=0x7fffffffe050 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:577
#42 0x00000000004787cc in ProcessArgs (op=0x7fffffffdaf0, cx=0x7ffff6907000) at js/src/shell/js.cpp:5772
#43 Shell (envp=<optimized out>, op=0x7fffffffdaf0, cx=0x7ffff6907000) at js/src/shell/js.cpp:6050
#44 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6396
rax	0x0	0
rbx	0x1	1
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffa6c0	140737488332480
rsp	0x7fffffffa6c0	140737488332480
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffa480	140737488331904
r11	0x7ffff6c27960	140737333328224
r12	0x61	97
r13	0x7fffffffc6e0	140737488340704
r14	0x1	1
r15	0x7fffffffc710	140737488340752
rip	0x42919c <js::frontend::TokenStream::verifyConsistentModifier(js::frontend::Token::Modifier, js::frontend::Token)+28>
=> 0x42919c <js::frontend::TokenStream::verifyConsistentModifier(js::frontend::Token::Modifier, js::frontend::Token)+28>:	movl   $0x213,0x0
   0x4291a7 <js::frontend::TokenStream::verifyConsistentModifier(js::frontend::Token::Modifier, js::frontend::Token)+39>:	callq  0x499bc0 <abort()>

Comment 1

[Tooru Fujisawa [:arai]]

The TOK_COLON is gotten with None modifier while parsing expression closure, and getToken with KeywordIsName modifier fails in propertyList.

Following case will be related:

  js> class a { constructor() { } get p() 20 static get q() 10}
  typein:1:39 SyntaxError: static is a reserved identifier:
  typein:1:39 class a { constructor() { } get p() 20 static get q() 10}
  typein:1:39 .......................................^

The "static" token shouldn't be gotten while parsing previous method definition.  in this case SyntaxError is thrown while parsing the previous method definition tho.

Can we just drop expression closure support for getter/setter inside class?
So the next token won't be gotten by the previous method definition parsing, and this assertion failure won't happen.
(object literal should have TOK_COMMA between properties, so it won't be the case)
I think that change won't break any website nor public add-ons, since class is nightly-only.

Comment 2

[Tooru Fujisawa [:arai]]

there's WIP patch for refactoring Parser::propertyList in bug 1192412, I'll work on this bug after that.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Disallow getter/setter with expression closure in class declaration.

Added new 2 types GetterNoExpressionClosure and SetterNoExpressionClosure into FunctionSyntaxKind and PropertyType, which are used in getter/setter in class declaration.  It rejects expression closure.  No other difference between Getter/Setter types.

green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=88a68a02fbee

Comment 4

[Eric Faust [:efaust]]

Comment on attachment 8656511 [details] [diff] [review]
Disallow getter/setter with expression closure in class declaration.

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

This is blech. We should land this to quiet the fuzzers, but look seriously (and immediately) at removing support for expression closures for all getters and setters. It's a spec issue.

{
   get foo() this.bar;
}

is not valid JS.

r=me with an explicitly filed (non security) followup.

::: js/src/frontend/Parser.cpp
@@ +1849,5 @@
>          bool hasDefaults = false;
>          Node duplicatedArg = null();
>          bool disallowDuplicateArgs = kind == Arrow || kind == Method || kind == ClassConstructor;
>  
> +        if (kind == Getter || kind == GetterNoExpressionClosure) {

Please change this to Is{Getter,Setter}Kind(), which is more in line with IsConstructorKind() already scattered around.

Comment 5

[Tooru Fujisawa [:arai]]

Thank you for reviewing :)
filed as bug 1203742

Comment 6

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150812050310" and the hash "2c3c8a927e3fcc3c7d63c2ecf5ff9a189d1cbd96".
The "bad" changeset has the timestamp "20150812054307" and the hash "a8493abd3c62b77e4741da181239af5549d84f80".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2c3c8a927e3fcc3c7d63c2ecf5ff9a189d1cbd96&tochange=a8493abd3c62b77e4741da181239af5549d84f80

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/caec7964e8f7

Comment 8

[Phil Ringnalda (:philor)]

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