Closed
Bug 1414805
Opened 7 years ago
Closed 6 years ago
Assertion failure: next.type != TOK_DIV && next.type != TOK_REGEXP (next token requires contextual specifier to be parsed unambiguously), at js/src/frontend/TokenStream.h:414
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | fixed |
People
(Reporter: decoder, Assigned: Waldo)
References
Details
(4 keywords, Whiteboard: [jsbugmon:update])
Attachments
(4 files)
17.27 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
1.22 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
3.45 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
5.23 KB,
patch
|
arai
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 4e6df5159df3 (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 --ion-offthread-compile=off): 1 ? (yield) : (yield) ^= (...target) => {} /Q/; Backtrace: received signal SIGSEGV, Segmentation fault. 0x00000000004d6ca8 in js::frontend::TokenStreamAnyChars::addModifierException (this=this@entry=0x7fffffffcd40, modifierException=modifierException@entry=js::frontend::Token::OperandIsNone) at js/src/frontend/TokenStream.h:413 #0 0x00000000004d6ca8 in js::frontend::TokenStreamAnyChars::addModifierException (this=this@entry=0x7fffffffcd40, modifierException=modifierException@entry=js::frontend::Token::OperandIsNone) at js/src/frontend/TokenStream.h:413 #1 0x00000000004f9995 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::assignExpr (this=this@entry=0x7fffffffcd28, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, tripledotHandling=tripledotHandling@entry=js::frontend::TripledotProhibited, possibleError=possibleError@entry=0x0, invoked=invoked@entry=js::frontend::ParserBase::PredictUninvoked) at js/src/frontend/Parser.cpp:8126 #2 0x00000000004f9e1d in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::expr (this=this@entry=0x7fffffffcd28, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, tripledotHandling=tripledotHandling@entry=js::frontend::TripledotProhibited, possibleError=possibleError@entry=0x0, invoked=invoked@entry=js::frontend::ParserBase::PredictUninvoked) at js/src/frontend/Parser.cpp:7715 #3 0x00000000004fab1b in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::expressionStatement (this=this@entry=0x7fffffffcd28, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, invoked=invoked@entry=js::frontend::ParserBase::PredictUninvoked) at js/src/frontend/Parser.cpp:5827 #4 0x0000000000504d35 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::statementListItem (this=this@entry=0x7fffffffcd28, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, canHaveDirectives=<optimized out>) at js/src/frontend/Parser.cpp:7580 #5 0x0000000000505268 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::statementList (this=this@entry=0x7fffffffcd28, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName) at js/src/frontend/Parser.cpp:4140 #6 0x00000000004d507a in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::globalBody (this=0x7fffffffcd28, globalsc=globalsc@entry=0x7fffffffc720) at js/src/frontend/Parser.cpp:2214 #7 0x0000000000af6878 in BytecodeCompiler::compileScript (this=this@entry=0x7fffffffc770, environment=..., environment@entry=..., sc=sc@entry=0x7fffffffc720) at js/src/frontend/BytecodeCompiler.cpp:348 #8 0x0000000000af71e1 in BytecodeCompiler::compileGlobalScript (scopeKind=<optimized out>, this=0x7fffffffc770) at js/src/frontend/BytecodeCompiler.cpp:394 #9 js::frontend::CompileGlobalScript (cx=cx@entry=0x7ffff6948000, alloc=..., scopeKind=scopeKind@entry=js::ScopeKind::Global, options=..., srcBuf=..., sourceObjectOut=sourceObjectOut@entry=0x0) at js/src/frontend/BytecodeCompiler.cpp:592 #10 0x000000000098484e in Compile (cx=cx@entry=0x7ffff6948000, options=..., scopeKind=scopeKind@entry=js::ScopeKind::Global, srcBuf=..., script=...) at js/src/jsapi.cpp:4136 #11 0x0000000000984a1a in Compile (script=..., length=<optimized out>, chars=0x7ffff4360630 u"1 ? (yield) : (yield) ^= (...target) => {} /Q/;\n", scopeKind=js::ScopeKind::Global, options=..., cx=0x7ffff6948000) at js/src/jsapi.cpp:4145 #12 Compile (cx=cx@entry=0x7ffff6948000, options=..., scopeKind=scopeKind@entry=js::ScopeKind::Global, bytes=<optimized out>, length=48, script=..., script@entry=...) at js/src/jsapi.cpp:4160 #13 0x000000000098a07d in Compile (cx=cx@entry=0x7ffff6948000, options=..., scopeKind=scopeKind@entry=js::ScopeKind::Global, fp=fp@entry=0x7ffff4351400, script=...) at js/src/jsapi.cpp:4171 #14 0x000000000098a0e5 in JS::Compile (cx=cx@entry=0x7ffff6948000, options=..., file=file@entry=0x7ffff4351400, script=..., script@entry=...) at js/src/jsapi.cpp:4211 #15 0x0000000000434d8d in RunFile (compileOnly=false, file=0x7ffff4351400, filename=<optimized out>, cx=0x7ffff6948000) at js/src/shell/js.cpp:685 #16 Process (cx=0x7ffff6948000, filename=<optimized out>, forceTTY=forceTTY@entry=false, kind=kind@entry=FileScript) at js/src/shell/js.cpp:1048 #17 0x0000000000445eac in ProcessArgs (op=0x7fffffffd9e0, cx=0x7ffff6948000) at js/src/shell/js.cpp:8184 #18 Shell (envp=<optimized out>, op=0x7fffffffd9e0, cx=0x7ffff6948000) at js/src/shell/js.cpp:8556 #19 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8962 rax 0x0 0 rbx 0x7fffffffcd28 140737488342312 rcx 0x7ffff6c28a2d 140737333332525 rdx 0x0 0 rsi 0x7ffff6ef7770 140737336276848 rdi 0x7ffff6ef6540 140737336272192 rbp 0x7fffffffbcb0 140737488338096 rsp 0x7fffffffbcb0 140737488338096 r8 0x7ffff6ef7770 140737336276848 r9 0x7ffff7fe4740 140737354024768 r10 0x58 88 r11 0x7ffff6b9f750 140737332770640 r12 0x7fffffffcd40 140737488342336 r13 0x7ffff4305200 140737290195456 r14 0x0 0 r15 0x0 0 rip 0x4d6ca8 <js::frontend::TokenStreamAnyChars::addModifierException(js::frontend::Token::ModifierException)+488> => 0x4d6ca8 <js::frontend::TokenStreamAnyChars::addModifierException(js::frontend::Token::ModifierException)+488>: movl $0x0,0x0 0x4d6cb3 <js::frontend::TokenStreamAnyChars::addModifierException(js::frontend::Token::ModifierException)+499>: ud2
Reporter | ||
Comment 1•7 years ago
|
||
I believe Waldo is familiar with this one. Sorry in advance ;)
Flags: needinfo?(jwalden+bmo)
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 2•7 years ago
|
||
JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/fe95681caba3 user: Jeff Walden date: Mon Oct 30 12:47:57 2017 -0700 summary: Bug 1319416 - Substantially rejigger token lookahead and modifier exceptions to function more like the spec. r=arai This iteration took 277.512 seconds to run.
Assignee | ||
Comment 3•6 years ago
|
||
This is not horribly unexpected. :-) The patchwork in bug 1319416 leaves us in a fundamentally better place than before -- but it was likely to regress something in the short run. On it.
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(jwalden+bmo)
Assignee | ||
Comment 4•6 years ago
|
||
I have a patch for this. Unfortunately, in the process of fixing, on a lark I started adding more testcases entirely unrelated to anything pointed out by this. And they broke left and right. So I started auditing everything in the parser, and that picked up more issues, and I'm not even done finding them (or fixing them). The latest issue, that I'm giving up fixing today far too late into it, involves changing modifiers in Parser::functionArguments -- and if that doesn't scare you in the complexity of any changes made there, you don't understand how we parse function arguments at all. (Answer: all in a single function, for every single possible flavor of function syntax in the language. Super-gag.) Back tomorrow to hopefully fix the rest of this. I'm as yet undecided how many separate patches I'm going to split this up into, because it probably could be half a dozen or more for all the distinct issues I've found now. But it's extra work, and testing all the intermediate states is super-slow and arguably not worth it if only the final state's correctness ultimately matters.
Assignee | ||
Comment 5•6 years ago
|
||
This fixes the original problem reported here. Gist of the fix is that it's just wrong for the token after condExpr() in assignExpr() to be gotten as None, so we fix that. But that requires ensuring several places where assignExpr() can appear -- within array/object literals/binding patterns -- also need to get commas/closing-brac(es|kets) as Operand for consistency. And you sometimes need modifier exceptions to accord as well. So this change overall spiraled large. :-( Unfortunately, I don't think I can trim away any of it (and I tried).
Attachment #8926162 -
Flags: review?(arai.unmht)
Assignee | ||
Comment 6•6 years ago
|
||
This does not *appear* to fix any observable bugs, or at least I couldn't find one that triggered. But it is consistent with how tokens after AssignmentExpression should be accessed generally.
Attachment #8926164 -
Flags: review?(arai.unmht)
Assignee | ||
Comment 7•6 years ago
|
||
Some of these, I think I broke in the intermediate state of writing this patch. But I don't think the patches you're seeing break/fix any of these tests. It's just good hygiene for us to have more of these.
Attachment #8926165 -
Flags: review?(arai.unmht)
Assignee | ||
Comment 8•6 years ago
|
||
The usual thing in this bug of ensuring that the token after assignExpr() is gotten as Operand, and thus that closing-parens/commas are consistently gotten that way. Parser::functionArguments() is a royal mess. Fortunately I was able to patch this without having to fundamentally change it. But we *really* need to fix function parsing so that there's some sort of helper class you can use to parse the umpteen different varieties of function, without commingling all the myriad ways their arguments present (and require particular modifiers as well).
Attachment #8926168 -
Flags: review?(arai.unmht)
Assignee | ||
Comment 9•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2d1f253e95f537b215711951a3a0145c44ca24e is all these patches (minus two or so little tweaks that seem obvious in very-minimal context to be correct -- specifically, removing a couple unnecessary addModifierExceptions followed by breaks to MUST_MATCH_TOKEN_MODs that don't require the exception any more). I'm pretty optimistic, at least enough to get a review now. But I think this is going to have to demand one final all-push given how gnarl it is. Also, I audited all the assignExpr()/expr() callers to ensure the next token is always gotten with the correct modifier, and I think everything is correct now. Odds on that analysis being correct are...not great, but they seem like they should be better than before this bug, or before bug 1319416. :-|
Blocks: 1319416
Assignee | ||
Comment 10•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f8462f8650211c80f4cf530f5763ff4764fc51d is a rebased version of these patches, atop inbound ~5h ago. Everything appears good to go.
Comment 11•6 years ago
|
||
I'll review this tonight.
Comment 12•6 years ago
|
||
Comment on attachment 8926162 [details] [diff] [review] Don't mishandle ASI after a ternary expression ending in an arrow function with block body, or such ternary appearing in a computed property name expression Review of attachment 8926162 [details] [diff] [review]: ----------------------------------------------------------------- Looks good :) I'm wondering if we should force using same/fixed modifier for some token kinds (like, TOK_COMMA/TOK_ASSIGN always need Operand), inside matchToken, consumeKnownToken, and some helper macros that receives expected token.
Attachment #8926162 -
Flags: review?(arai.unmht) → review+
Updated•6 years ago
|
Attachment #8926164 -
Flags: review?(arai.unmht) → review+
Updated•6 years ago
|
Attachment #8926165 -
Flags: review?(arai.unmht) → review+
Updated•6 years ago
|
Attachment #8926168 -
Flags: review?(arai.unmht) → review+
Comment 13•6 years ago
|
||
Pushed by jwalden@mit.edu: https://hg.mozilla.org/integration/mozilla-inbound/rev/8b99335365c5 Don't mishandle ASI after a ternary expression ending in an arrow function with block body, or such ternary appearing in a computed property name expression. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/d8441c69c083 Parse the colon in a ?: expression using the Operand modifier, because it follows after an AssignmentExpression. This does not *appear* to fix any observable bugs, but it is consistent with how tokens after AssignmentExpression should be accessed generally. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/eed73ebe0266 Add more tests for other cases of parsing ternary expressions ending in an arrow function with block body, that aren't already covered in the existing test. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/af596055be21 Use proper modifiers when parsing default expressions that end in an arrow function with block body, in function parameters. r=arai
Comment 14•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8b99335365c5 https://hg.mozilla.org/mozilla-central/rev/d8441c69c083 https://hg.mozilla.org/mozilla-central/rev/eed73ebe0266 https://hg.mozilla.org/mozilla-central/rev/af596055be21
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•6 years ago
|
status-firefox56:
--- → unaffected
status-firefox57:
--- → unaffected
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•