1142844 - Crash [@ js::LazyScript::scriptSource]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

// Randomly chosen test: js/src/jit-test/tests/basic/bug1057571.js
(function() {
    code = cacheEntry("function f(){}f()")
    code.compileAndGo = true;
    evaluate(code, Object.create(code, {
        saveBytecode: {
            value: true
        }
    }))
    evaluate(code, {
        loadBytecode: true
    })
})()
// jsfunfuzz-generated code
relazifyFunctions()
this instanceof this

crashes js debug shell on m-c changeset 30916c9ca768 with --fuzzing-safe --no-threads --no-baseline --no-ion at js::LazyScript::scriptSource.

Configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh ~/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build" -r 30916c9ca768

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/eb6e90404b76
parent:      224399:b86864fd9d60
user:        Jan de Mooij
date:        Sat Jan 17 12:54:03 2015 +0100
summary:     Bug 1116760 - Add a shell function to test function relazification. r=till

Jan, did bug 1116760 expose the issue?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x5e5ca, 0x000000010084f693 js-dbg-64-dm-nsprBuild-darwin-30916c9ca768`js::LazyScript::scriptSource() const [inlined] JSObject::getClass() const at jsobj.h:130, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010084f693 js-dbg-64-dm-nsprBuild-darwin-30916c9ca768`js::LazyScript::scriptSource() const [inlined] JSObject::getClass() const at jsobj.h:130
    frame #1: 0x000000010084f693 js-dbg-64-dm-nsprBuild-darwin-30916c9ca768`js::LazyScript::scriptSource() const [inlined] js::NativeObject::getReservedSlot(this=<unavailable>, index=<unavailable>) const at NativeObject.h:824
    frame #2: 0x000000010084f693 js-dbg-64-dm-nsprBuild-darwin-30916c9ca768`js::LazyScript::scriptSource() const [inlined] js::LazyScript::sourceObject(this=<unavailable>, this=0x0000000000000000) const + 31 at jsscript.h:706
    frame #3: 0x000000010084f674 js-dbg-64-dm-nsprBuild-darwin-30916c9ca768`js::LazyScript::scriptSource(this=<unavailable>) const + 4 at jsscript.h:1910
    frame #4: 0x00000001007d2ca3 js-dbg-64-dm-nsprBuild-darwin-30916c9ca768`JSFunction::createScriptForLazilyInterpretedFunction(cx=0x0000000101e028a0, fun=<unavailable>) + 1539 at jsfun.cpp:1470
(lldb)

Comment 2

[Jan de Mooij [:jandem]]

Here's a simpler test, does not require --enable-more-deterministic:

(function() {
    var code = cacheEntry("function f(){}; f();");
    code.compileAndGo = true;
    evaluate(code, Object.create(code, {saveBytecode: {value: true}}));
    evaluate(code, {loadBytecode: true});
})();
relazifyFunctions();
print(f.toSource());

And here's one without relazifyFunctions:

var g = newGlobal();
g.eval(`
(function() {
    var code = cacheEntry("function f(){}; f();");
    code.compileAndGo = true;
    evaluate(code, Object.create(code, {saveBytecode: {value: true}}));
    evaluate(code, {loadBytecode: true});
})();
`);
gc();
print(g.f.toString());

Gary, can you try autoBisect on this one? :)

Comment 3

[Jan de Mooij [:jandem]]

I just realized template strings are fairly new and that may confuse autoBisect. Try this one:

var g = newGlobal();
g.eval("(" + (function() {
    var code = cacheEntry("function f(){}; f();");
    code.compileAndGo = true;
    evaluate(code, Object.create(code, {saveBytecode: {value: true}}));
    evaluate(code, {loadBytecode: true});
}) + ")();");
gc();
print(g.f.toString());

Comment 4

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

Thanks Jan!

With the testcase in comment 3:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/f317d2cb357b
user:        Nicolas B. Pierron
date:        Thu Feb 20 07:09:34 2014 -0800
summary:     Bug 900789 - Instrument evaluate function to save/load the bytecode. r=luke

Nicolas, is bug 900789 a likely regressor?

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #4)
> summary:     Bug 900789 - Instrument evaluate function to save/load the
> bytecode. r=luke
> 
> Nicolas, is bug 900789 a likely regressor?

Yes, it is.  And now that I modified the test case, I realized that this is an issue that I thought about for a long time for Gecko but I never thought about making a JS test case about it :/

A simple test case which highlight better what is the issue is the following, and it does not require any JS shell argument any more:


var g1 = newGlobal();
var g2 = newGlobal();
var res = "function f(){}";
var code = cacheEntry(res + "; f();");
evaluate(code, {global:g1, compileAndGo: true, saveBytecode: {value: true}});
evaluate(code, {global:g2, loadBytecode: true});
gc();
assertEq(g2.f.toString(), res);

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: When xdr-decoding a non-lazy but relazifiable function, don't forget to set up the source object on the LazyScript we create for it

Comment 7

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8583894 [details] [diff] [review]
When xdr-decoding a non-lazy but relazifiable function, don't forget to set up the source object on the LazyScript we create for it

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

Sounds good to me, maybe move it inside XDRRelazificationInfo to keep the symetry in XDRScript.

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 8583894 [details] [diff] [review]
When xdr-decoding a non-lazy but relazifiable function, don't forget to set up the source object on the LazyScript we create for it

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

Nice job tracking this down!

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Per IRC discussion, with nbp's suggestion

Comment 10

[Boris Zbarsky [:bzbarsky]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[David]

An crash on RelazifyFunctions has occured that may be related to this? (unsure, please confirm)
see https://crash-stats.mozilla.com/report/index/11808ec6-947e-410c-acb1-674222160203

Comment 13

[Boris Zbarsky [:bzbarsky]]

Unless this crash started happening 10 months ago, unlikely...