Assertion failure: !possibleError->hasError(), at js/src/frontend/Parser.cpp:7831

RESOLVED FIXED in Firefox 47

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gkw, Assigned: mrrrgn)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla48
x86_64
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 unaffected, firefox47 fixed, firefox48 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
The following testcase crashes on mozilla-central revision be593a64d7c6 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

({e=[]}==(

Backtrace:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005d6d3 js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 1843 (Parser.cpp:7831)
1   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005ccc5 js::frontend::Parser<js::frontend::FullParseHandler>::expr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 37 (Parser.cpp:7540)
2   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005c435 js::frontend::Parser<js::frontend::FullParseHandler>::primaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::TokenKind, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 1637 (Parser.cpp:9576)
3   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005f2d3 js::frontend::Parser<js::frontend::FullParseHandler>::memberExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::TokenKind, bool, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 803 (Parser.cpp:8684)
4   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005ebbd js::frontend::Parser<js::frontend::FullParseHandler>::unaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 749 (Parser.cpp:8212)
5   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005e55b js::frontend::Parser<js::frontend::FullParseHandler>::orExpr1(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 107 (Parser.cpp:7701)
6   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005e312 js::frontend::Parser<js::frontend::FullParseHandler>::condExpr1(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 34 (Parser.cpp:7755)
7   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005d29d js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 765 (Parser.cpp:7882)
8   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005ccc5 js::frontend::Parser<js::frontend::FullParseHandler>::expr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 37 (Parser.cpp:7540)
9   js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005c435 js::frontend::Parser<js::frontend::FullParseHandler>::primaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::TokenKind, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 1637 (Parser.cpp:9576)
10  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005f2d3 js::frontend::Parser<js::frontend::FullParseHandler>::memberExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::TokenKind, bool, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 803 (Parser.cpp:8684)
11  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005ebbd js::frontend::Parser<js::frontend::FullParseHandler>::unaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 749 (Parser.cpp:8212)
12  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005e55b js::frontend::Parser<js::frontend::FullParseHandler>::orExpr1(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 107 (Parser.cpp:7701)
13  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005e312 js::frontend::Parser<js::frontend::FullParseHandler>::condExpr1(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 34 (Parser.cpp:7755)
14  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005d29d js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 765 (Parser.cpp:7882)
15  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010005ccc5 js::frontend::Parser<js::frontend::FullParseHandler>::expr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 37 (Parser.cpp:7540)
16  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x00000001000505d3 js::frontend::Parser<js::frontend::FullParseHandler>::expressionStatement(js::frontend::YieldHandling, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) + 99 (Parser.cpp:7593)
17  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010004fc2c js::frontend::Parser<js::frontend::FullParseHandler>::statement(js::frontend::YieldHandling, bool) + 1580 (Parser.cpp:7378)
18  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010004d91c js::frontend::Parser<js::frontend::FullParseHandler>::statements(js::frontend::YieldHandling) + 572 (Parser.cpp:3530)
19  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010004715d js::frontend::Parser<js::frontend::FullParseHandler>::globalBody() + 77 (Parser.cpp:1105)
20  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x0000000100956bed BytecodeCompiler::compileScript(JS::Handle<JSObject*>, JS::Handle<JSScript*>) + 733 (BytecodeCompiler.cpp:527)
21  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010095896d js::frontend::CompileScript(js::ExclusiveContext*, js::LifoAlloc*, JS::Handle<JSObject*>, JS::Handle<js::StaticScope*>, JS::Handle<JSScript*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JSString*, js::SourceCompressionTask*, js::ScriptSourceObject**) + 189 (BytecodeCompiler.cpp:738)
22  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010054a754 Compile(JSContext*, JS::ReadOnlyCompileOptions const&, SyntacticScopeOption, JS::SourceBufferHolder&, JS::MutableHandle<JSScript*>) + 404 (RootingAPI.h:481)
23  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010054aadb Compile(JSContext*, JS::ReadOnlyCompileOptions const&, SyntacticScopeOption, char const*, unsigned long, JS::MutableHandle<JSScript*>) + 267 (jsapi.cpp:3981)
24  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x000000010054ac2c JS::Compile(JSContext*, JS::ReadOnlyCompileOptions const&, __sFILE*, JS::MutableHandle<JSScript*>) + 108 (jsapi.cpp:4007)
25  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x00000001000208b6 Process(JSContext*, char const*, bool, FileKind) + 3286 (js.cpp:514)
26  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x0000000100006742 main + 11778 (js.cpp:6563)
27  js-dbg-64-dm-clang-darwin-be593a64d7c6	0x00000001000018b4 start + 52
(Reporter)

Comment 1

2 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/bbab21ac3b8c
user:        Morgan Phillips
date:        Sat Mar 05 12:51:38 2016 -0800
summary:     Bug 932080 - Support default values in destructuring; r=jorendorff

Morgan, is bug 932080 a likely regressor?
Blocks: 932080
Flags: needinfo?(winter2718)
Comment hidden (obsolete)
(Assignee)

Comment 3

2 years ago
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #1)
> autoBisect shows this is probably related to the following changeset:
> 
> The first bad revision is:
> changeset:   https://hg.mozilla.org/mozilla-central/rev/bbab21ac3b8c
> user:        Morgan Phillips
> date:        Sat Mar 05 12:51:38 2016 -0800
> summary:     Bug 932080 - Support default values in destructuring;
> r=jorendorff
> 
> Morgan, is bug 932080 a likely regressor?

It is, taking the bug. I'd been hesitant about this assertion from the start. Will consider removing it altogether after a short investigation. Thanks !
Assignee: nobody → winter2718
Flags: needinfo?(winter2718)
Comment hidden (obsolete)
(Assignee)

Comment 5

2 years ago
So, not having a pending error when we enter an assignExpr is not an invariant. Instead, it's a sign that we're in a bad state and we should raise the pending error right away. I'll attempt to distill this into a convincing comment. Patch coming soon.
(Assignee)

Comment 6

2 years ago
Created attachment 8728323 [details] [diff] [review]
assertion.diff

Sorry for the delay here. So, now we know that !pendingError->hasError() isn't an invariant (in assignExpr) but I do believe its violation will always accompany an existing syntax error. That said, after going back and forth about this all day, I decided to remove the assertion altogether rather than replace it with an error check.

My primary reason for this is that the intent of the original assertion was to guarantee that pending errors are not overwritten. I believe a better way of checking this is to just assert inside of PossibleError::setPending. The intent is more clear and violations of the original assertion will still produce errors.

I may disagree with myself after a nights sleep, but for now it's my position.
Attachment #8728323 - Flags: review?(jorendorff)
(Assignee)

Updated

2 years ago
Attachment #8728323 - Flags: review?(jorendorff)
(Assignee)

Comment 7

2 years ago
Created attachment 8728346 [details] [diff] [review]
assertion.diff
Attachment #8728323 - Attachment is obsolete: true
Attachment #8728346 - Flags: review?(jorendorff)
(Assignee)

Comment 8

2 years ago
I moved from an assertion to just failing to set a pending error (and returning false). This seems more sensible.
Comment on attachment 8728346 [details] [diff] [review]
assertion.diff

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

All right, land it - but I don't feel very confident that it's correct this time. If any more bugs are found, it'll be time to take a step back and figure out a way to cover all the cases systematically. Hopefully something simple that we can trust.
Attachment #8728346 - Flags: review?(jorendorff) → review+

Comment 10

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/73f748c3b24b

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/73f748c3b24b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Reporter)

Comment 12

2 years ago
Let's get this onto Aurora as well. A small testcase is pretty easy to trigger the issue on that branch.
status-firefox46: --- → unaffected
Flags: needinfo?(winter2718)
(Assignee)

Comment 13

2 years ago
Comment on attachment 8728346 [details] [diff] [review]
assertion.diff

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: May cause crashes.
[Describe test coverage new/current, TreeHerder]: A unit test case is included in the patch.
[Risks and why]: 
[String/UUID change made/needed]:
Flags: needinfo?(winter2718)
Attachment #8728346 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 14

2 years ago
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #12)
> Let's get this onto Aurora as well. A small testcase is pretty easy to
> trigger the issue on that branch.

Agreed. There was another fuzzbug that we may need to flag. Checking my history.
(Assignee)

Comment 15

2 years ago
s/fuzzbug/similar bug/
(Reporter)

Comment 16

2 years ago
I do have crashes that occur at js::jit::MConstant::toObjectOrNull which reduce to this assertion on mozilla-aurora.
Comment on attachment 8728346 [details] [diff] [review]
assertion.diff

Potential Crash fix, Aurora47+
Attachment #8728346 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 18

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/9799515f95fc
status-firefox47: affected → fixed
You need to log in before you can comment on or make changes to this bug.