1308744 - Crash [@ js::frontend::BytecodeEmitter::emitFunction] or [@ js::frontend::BytecodeEmitter::emitTree]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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

[{
    a: b = (function() {
        return {
            f() {},
            g() {}
        }
    })
}] = c


Backtrace:

0   js-dbg-64-dm-clang-darwin-313a2d049350	0x000000010ad1bc45 js::frontend::BytecodeEmitter::emitFunction(js::frontend::ParseNode*, bool) + 2565 (BytecodeEmitter.cpp:9847)
1   js-dbg-64-dm-clang-darwin-313a2d049350	0x000000010ad0c126 js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::BytecodeEmitter::EmitLineNumberNote) + 1302 (BytecodeEmitter.cpp:9179)
2   js-dbg-64-dm-clang-darwin-313a2d049350	0x000000010ad2224f js::frontend::BytecodeEmitter::emitPropertyList(js::frontend::ParseNode*, JS::MutableHandle<js::PlainObject*>, js::frontend::PropListType) + 671 (BytecodeEmitter.cpp:8382)
3   js-dbg-64-dm-clang-darwin-313a2d049350	0x000000010ad22ae5 js::frontend::BytecodeEmitter::emitObject(js::frontend::ParseNode*) + 373 (BytecodeEmitter.cpp:8487)
4   js-dbg-64-dm-clang-darwin-313a2d049350	0x000000010ad0c556 js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::BytecodeEmitter::EmitLineNumberNote) + 2374 (BytecodeEmitter.cpp:9492)
/snip

For detailed crash information, see attachment.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9716bcfed35d
user:        Tooru Fujisawa
date:        Tue Sep 27 13:57:00 2016 +0900
summary:     Bug 1184922 - Part 1: Do not call iter.next() if the previous iter.next().done was true in array destructuring. r=shu

Arai-san, is bug 1184922 a likely regressor?

Comment 3

[Tooru Fujisawa [:arai]]

Yes, and it's already backed out in m-i.

the issue is that we try to emit an object twice for different script for the same function, by different emitter (sadly, the pointer is same, as it's on stack...)
emitLink is created while emitting first time, and we used the same emitLink while emitting second time, that's wrong.
so, even if emitLink is non-null, it doesn't mean that the object is in the objectList of the current emitter.

possible solutions:
  * clear emitLink before or after emitting a function, if emitting twice (maybe need RAII)
  * search through objectList elements everytime
  * search through objectList elements everytime, if emitting twice (needs RAII)

Comment 4

[Tooru Fujisawa [:arai]]

another solution:
  * add unique ID to BytecodeEmitter or ObjectList and store it to ObjectBox, and ignore the emitLink value if the ID is different

Comment 5

[Tooru Fujisawa [:arai]]

We can clear emitLink in CGObjectList::finish.
Patch is ready, will fix in bug 1184922

Comment 6

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 90d8afaddf91).

Comment 7

[Tooru Fujisawa [:arai]]

fixed in bug 1184922

Comment 8

[Marco Castelluccio [:marco]]

Should we try to uplift the fix?

Comment 9

[Tooru Fujisawa [:arai]]

This bug is a regression from bug 1184922 comment #33 (firefox 52) and soon backed out there (bug 1184922 comment #34).
So this issue itself doesn't affect firefox 50 or 51.
Have you spotted similar issue on older branches?

Comment 10

[Marco Castelluccio [:marco]]

For some reason I thought 1184922 landed earlier than 52.