Closed Bug 1231758 Opened 4 years ago Closed 4 years ago

Assertion failure: emitOption != DefineVars && emitOption != AnnexB, at js/src/frontend/BytecodeEmitter.cpp:4382

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: gkw, Unassigned)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: [fuzzblocker] [jsbugmon:update,ignore])

Attachments

(1 file)

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

{
    function f() {}
}
function f() {}


Backtrace:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008b8c5d js::frontend::BytecodeEmitter::emitVariables(js::frontend::ParseNode*, js::frontend::VarEmitOption) + 621 (BytecodeEmitter.cpp:4382)
1   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008bfa74 js::frontend::BytecodeEmitter::emitFunction(js::frontend::ParseNode*, bool) + 516 (BytecodeEmitter.cpp:6242)
2   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008a4976 js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::BytecodeEmitter::EmitLineNumberNote) + 774 (BytecodeEmitter.cpp:8304)
3   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008a4b83 js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::BytecodeEmitter::EmitLineNumberNote) + 1299 (BytecodeEmitter.cpp:6925)
4   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008bcb1d js::frontend::BytecodeEmitter::emitLexicalScope(js::frontend::ParseNode*) + 221 (BytecodeEmitter.cpp:5359)
5   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008a505b js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::BytecodeEmitter::EmitLineNumberNote) + 2539 (BytecodeEmitter.cpp:8549)
6   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008a4b83 js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::BytecodeEmitter::EmitLineNumberNote) + 1299 (BytecodeEmitter.cpp:6925)
7   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008a4351 BytecodeCompiler::prepareAndEmitTree(js::frontend::ParseNode**) + 177 (BytecodeCompiler.cpp:350)
8   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008a6944 BytecodeCompiler::compileScript(JS::Handle<JSObject*>, JS::Handle<JSScript*>) + 1012 (BytecodeCompiler.cpp:535)
9   js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001008a86ed js::frontend::CompileScript(js::ExclusiveContext*, js::LifoAlloc*, JS::Handle<JSObject*>, JS::Handle<js::ScopeObject*>, JS::Handle<JSScript*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JSString*, js::SourceCompressionTask*, js::ScriptSourceObject**) + 189 (BytecodeCompiler.cpp:738)
10  js-dbg-64-dm-darwin-412e4d7ce98c	0x0000000100509644 Compile(JSContext*, JS::ReadOnlyCompileOptions const&, SyntacticScopeOption, JS::SourceBufferHolder&, JS::MutableHandle<JSScript*>) + 404 (RootingAPI.h:481)
11  js-dbg-64-dm-darwin-412e4d7ce98c	0x00000001005099cb Compile(JSContext*, JS::ReadOnlyCompileOptions const&, SyntacticScopeOption, char const*, unsigned long, JS::MutableHandle<JSScript*>) + 267 (jsapi.cpp:4027)
12  js-dbg-64-dm-darwin-412e4d7ce98c	0x0000000100509b1c JS::Compile(JSContext*, JS::ReadOnlyCompileOptions const&, __sFILE*, JS::MutableHandle<JSScript*>) + 108 (jsapi.cpp:4053)
13  js-dbg-64-dm-darwin-412e4d7ce98c	0x000000010001e70a Process(JSContext*, char const*, bool, FileKind) + 3098 (js.cpp:507)
14  js-dbg-64-dm-darwin-412e4d7ce98c	0x0000000100004fa1 main + 11825 (js.cpp:6205)
15  js-dbg-64-dm-darwin-412e4d7ce98c	0x0000000100001564 start + 52
Setting [fuzzblocker] due to its simplicity.
Whiteboard: [jsbugmon:update,bisect] → [fuzzblocker][jsbugmon:update,bisect]
Comment on attachment 8697353 [details] [diff] [review]
Fix bogus assertion in BCE for Annex B function assignment.

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

r=me. If you could take a second to fix the comments in a second changeset, i'd be grateful...

::: js/src/frontend/BytecodeEmitter.cpp
@@ +4376,5 @@
>               * A destructuring initialiser assignment preceded by var will
>               * never occur to the left of 'in' in a for-in loop.  As with 'for
>               * (var x = i in o)...', this will cause the entire 'var [a, b] =
>               * i' to be hoisted out of the loop.
>               */

This comment looks stale to me - if it looks stale to you too, please fix it?

@@ +4384,5 @@
>  
>              /*
>               * To allow the front end to rewrite var f = x; as f = x; when a
>               * function f(){} precedes the var, detect simple name assignment
>               * here and initialize the name.

This one could be improved too, while you're here. It seems to not be really correct about what "rewrite" we are talking about here; and is there a reason to give `function f(){}` as an example, rather than just a previous declaration `var f;`?
Attachment #8697353 - Flags: review?(jorendorff) → review+
(In reply to Jason Orendorff [:jorendorff] from comment #3)
> Comment on attachment 8697353 [details] [diff] [review]
> Fix bogus assertion in BCE for Annex B function assignment.
> 
> Review of attachment 8697353 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me. If you could take a second to fix the comments in a second changeset,
> i'd be grateful...
> 
> ::: js/src/frontend/BytecodeEmitter.cpp
> @@ +4376,5 @@
> >               * A destructuring initialiser assignment preceded by var will
> >               * never occur to the left of 'in' in a for-in loop.  As with 'for
> >               * (var x = i in o)...', this will cause the entire 'var [a, b] =
> >               * i' to be hoisted out of the loop.
> >               */
> 
> This comment looks stale to me - if it looks stale to you too, please fix it?
> 

I don't know enough to say if this is stale. I'll leave as is for now. Waldo should update this with the work he's doing.

> @@ +4384,5 @@
> >  
> >              /*
> >               * To allow the front end to rewrite var f = x; as f = x; when a
> >               * function f(){} precedes the var, detect simple name assignment
> >               * here and initialize the name.
> 
> This one could be improved too, while you're here. It seems to not be really
> correct about what "rewrite" we are talking about here; and is there a
> reason to give `function f(){}` as an example, rather than just a previous
> declaration `var f;`?

Sure, will give explanation for why Annex B can reach here.
Whiteboard: [fuzzblocker][jsbugmon:update,bisect] → [fuzzblocker] [jsbugmon:update]
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/a44b841c33fb
user:        Shu-yu Guo
date:        Wed Dec 09 07:52:58 2015 -0800
summary:     Bug 1071646 - Introduce JSOP_BINDVAR to support Annex B.3.3.3. (r=jorendorff)

This iteration took 261.028 seconds to run.
Blocks: 1071646
Has Regression Range: --- → yes
Has STR: --- → yes
https://hg.mozilla.org/mozilla-central/rev/b6901bdf88e1
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
https://hg.mozilla.org/mozilla-central/rev/6ea85c756076 - landed yesterday
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Why is this reopened, I'd think that this is still FIXED? The landing in comment 9 is just a bump in the XDR_BYTECODE_VERSION_SUBTRAHEND value.
Flags: needinfo?(shu)
Flags: needinfo?(guijoselito)
I reopened it because of http://hg.mozilla.org/mozilla-central/rev/d2bec6ed7b30 - Backout bug 1071646, bug 1231758 for breaking the web. (r=woe)
Flags: needinfo?(guijoselito)
Whiteboard: [fuzzblocker] [jsbugmon:update] → [fuzzblocker] [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 0babaa3edcf9).
It should be reopened, yeah. Thanks.
Flags: needinfo?(shu)
(In reply to Guilherme Lima from comment #11)
> I reopened it because of
> http://hg.mozilla.org/mozilla-central/rev/d2bec6ed7b30 - Backout bug
> 1071646, bug 1231758 for breaking the web. (r=woe)

Great, thanks for the clarification!
https://hg.mozilla.org/mozilla-central/rev/c7a3d4a1a2f8
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Updating target milestone to avoid confusion; this was backed out (bug 1231568) and relanded in 46.
Target Milestone: mozilla45 → mozilla46
You need to log in before you can comment on or make changes to this bug.