Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Description • 9 years ago

g1 = evalcx('lazy');
// Adapted from randomly chosen test: js/src/jit-test/tests/gc/oomInFormatStackDump.js
oomTest(() => getBacktrace({
    thisprops: true
}));

asserts js debug shell on m-c changeset 4e164269cf88 with --fuzzing-safe --no-threads --baseline-eager at Assertion failure: inited == !getPrototype(key).isUndefined(), at vm/GlobalObject.h

Configure options:

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

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r 4e164269cf88

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9c365490d4ce
user:        Jon Coppeard
date:        Tue Oct 13 13:37:07 2015 +0100
summary:     Bug 1212469 - Make oomTest() into a shell function r=nbp

Jon, is bug 1212469 a likely regressor?

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 1 • 9 years ago

(lldb) bt 5
* thread #1: tid = 0x242757, 0x00000001006c057b js-dbg-64-dm-darwin-4e164269cf88`js::GlobalObject::functionObjectClassesInitialized() const [inlined] js::GlobalObject::classIsInitialized(this=<unavailable>, key=JSProto_Function) const + 120 at GlobalObject.h:199, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001006c057b js-dbg-64-dm-darwin-4e164269cf88`js::GlobalObject::functionObjectClassesInitialized() const [inlined] js::GlobalObject::classIsInitialized(this=<unavailable>, key=JSProto_Function) const + 120 at GlobalObject.h:199
    frame #1: 0x00000001006c0503 js-dbg-64-dm-darwin-4e164269cf88`js::GlobalObject::functionObjectClassesInitialized(this=<unavailable>) const + 339 at GlobalObject.h:204
    frame #2: 0x00000001006abc78 js-dbg-64-dm-darwin-4e164269cf88`js::GlobalObject::getOrCreateObjectPrototype(this=0x0000000102d81060, cx=0x0000000102c45400) + 24 at GlobalObject.h:337
    frame #3: 0x00000001004acd52 js-dbg-64-dm-darwin-4e164269cf88`JS_ResolveStandardClass(cx=0x0000000102c45400, obj=<unavailable>, id=<unavailable>, resolved=<unavailable>) + 562 at jsapi.cpp:1250
    frame #4: 0x0000000100018f02 js-dbg-64-dm-darwin-4e164269cf88`sandbox_resolve(cx=0x0000000102c45400, obj=JS::HandleObject @ r12, id=JS::HandleId @ 0x00007fff5fbfca58, resolvedp=0x00007fff5fbfcad7) + 338 at js.cpp:2449
(lldb)

	Jon Coppeard (:jonco) (PTO until 14th September)
	Comment 2 • 9 years ago

No bug 1212469 is not a likely regressor, it just introduced the oomTest() function to the shell which is helping to highlight these problems.

This looks like something to do with global object initialization.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 3 • 9 years ago

Jan, do you know how to move this forward? (or find someone who is able to?)

	Jan de Mooij [:jandem]
	Comment 4 • 9 years ago

GlobalObject::resolveConstructor first creates the prototype and stashes it in the prototype slot on the global. Then it creates the constructor and stores that in the constructor slot.

Here we OOM while creating the Object constructor, so we end up with the Object.prototype slot initialized but the constructor slot still undefined and later on we assert.

Not sure how to fix this properly. Should we call GlobalObject::resolveConstructor a second time and let it ignore the prototype slot if it has already been initialized?

	Jan de Mooij [:jandem]
	Comment 5 • 9 years ago

(And sorry for forwarding this to you while you're out for the rest of the week, but I don't know who else knows about object/function initialization..)

	Jeff Walden [:Waldo]
	Comment 6 • 9 years ago

Sigh.  Bug 686900 fixed this, back in the day, but then the class-spec gunk re-gunkified it all up and regressed this.  :-(  Stab.

The right thing to do, most likely, is to revert back to a one-off system like happened before, somehow.  If that requires special-casing JSProto_Object and JSProto_Function in constructor-resolving and prototype-resolving code, so be it.  And whatever has to happen to make scumbag xrays "work", can be equally as gunky.

	Jeff Walden [:Waldo]
	Comment 7 • 9 years ago

Specifically, this revision was what made this bug go away previously, I expect:

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

Note that setObjectClassDetails initializes the two Object class slots all at once (and then objects generally can be created without special effort) and setFunctionClassDetails does likewise for Function (and then functions generally can be created without special effort).

	Fuzzing Team
	Comment 8 • 9 years ago

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

	Fuzzing Team
	Comment 9 • 9 years ago

JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/52d7c9292ecf
user:        Jan de Mooij
date:        Sat Nov 21 14:33:13 2015 +0100
summary:     Bug 1132183 - Make |this| a real binding, remove lazy this computation. r=efaust,shu

This iteration took 255.409 seconds to run.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 10 • 9 years ago

Jan, is bug 1132183 a likely fix?

	Jan de Mooij [:jandem]
	Comment 11 • 9 years ago

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #10)
> Jan, is bug 1132183 a likely fix?

Unlikely. Do you have newer tests with this signature?

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 12 • 9 years ago

No, I don't. Are the discussions in comment 4, comment 6 and comment 7 enough to help push this towards a fix?

	Jan de Mooij [:jandem]
	Comment 13 • 9 years ago

I'm not sure what's the best way to fix this. Waldo, could we just always initialize the Object/Function prototypes when we create the global, and return a nullptr global if this fails?

	Christian Holler (:decoder)
	Comment 14 • 8 years ago

This is probably a duplicate of bug 1037685 which now has a newer test in comment 8.

	Jason Orendorff [:jorendorff]
	Comment 16 • 8 years ago

(In reply to Jan de Mooij [:jandem] from comment #13)
> I'm not sure what's the best way to fix this. Waldo, could we just always
> initialize the Object/Function prototypes when we create the global, and
> return a nullptr global if this fails?

Certainly from a correctness standpoint this is entirely doable. It's a browser performance question. So you tell me.

The number of objects is something like 43: Object, Function, Object.prototype, Function.prototype, and all their methods (kind of a lot of them, unfortunately). Plus as many Shapes. Some of the functions are self-hosted.

We create these anyway as soon as any JS actually runs in the global, because you can't wipe your nose in JS without needing O.p or F.p; but I've been told that in many windows nothing ever happens.

	Jason Orendorff [:jorendorff]
	Comment 17 • 8 years ago

The new test case is in bug 1037685 comment 8, and it reproduces for me with:

 $JS --fuzzing-safe --ion-offthread-compile=off -e 'a = evalcx("lazy"); oomTest(() => a.toString)'

	Jason Orendorff [:jorendorff]
	Comment 21 • 7 years ago

Jan, this keeps gathering dups; what do you think about comment 16?

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 22 • 7 years ago

Was advised to set this [fuzzblocker] due to the dupes that keep appearing.

	Jan de Mooij [:jandem]
	Comment 23 • 7 years ago

Gary, do you have newer test cases for this bug's signature and (duplicate) bug 1348407?

I want to see if we can get away with a more targeted fix. Else we should consider initializing Object/Function eagerly, as discussed in this bug.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 24 • 7 years ago

I don't think so. Whatever testcases we have we will report, even as dupes.

	Jan de Mooij [:jandem]
	Comment 25 • 7 years ago

Clearing NI for now. After bug 1348407 lands we can see if there are other signatures.

	Jason Orendorff [:jorendorff]
	Comment 26 • 7 years ago

OK, this isn't really a fuzzblocker...

	Christian Holler (:decoder)
	Comment 32 • 5 years ago

This is an automated crash issue comment:

Summary: Assertion failure: inited == !getPrototype(key).isUndefined(), at js/src/vm/GlobalObject.h:227
Build version: mozilla-central revision 3eb7623b5e63
Build flags: --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize
Runtime options: --fuzzing-safe --cpu-count=2

Testcase:

    setLazyParsingDisabled(true);
    evaluate(`
      x = evalcx("lazy");
      oomTest(function () {
        x.start("1");
      });
    `);

Backtrace:

    received signal SIGSEGV, Segmentation fault.
    #0  js::GlobalObject::classIsInitialized (this=this@entry=0x78172385240, key=key@entry=JSProto_Object) at js/src/vm/GlobalObject.h:227
    #1  0x0000555555e1f9fc in js::GlobalObject::functionObjectClassesInitialized (this=0x78172385240) at js/src/vm/GlobalObject.h:233
    #2  js::GlobalObject::getOrCreateObjectPrototype (global=..., cx=0x7ffff5f19000) at js/src/vm/GlobalObject.h:291
    #3  JS_ResolveStandardClass (cx=0x7ffff5f19000, obj=..., id=..., resolved=0x7fffffffa96f) at js/src/jsapi.cpp:883
    #4  0x0000555555839375 in sandbox_resolve (cx=0x7ffff5f19000, obj=..., id=..., resolvedp=0x7fffffffa96f) at js/src/shell/js.cpp:3787
    #5  0x0000555555b7e093 in js::CallResolveOp (recursedp=<synthetic pointer>, propp=..., id=..., obj=..., cx=<optimized out>) at js/src/vm/NativeObject-inl.h:741
    #6  js::LookupOwnPropertyInline<(js::AllowGC)1> (cx=<optimized out>, obj=..., id=..., propp=..., donep=0x7fffffffaa7f) at js/src/vm/NativeObject-inl.h:816
    #7  0x0000555555b7f83e in NativeGetPropertyInline<(js::AllowGC)1> (cx=<optimized out>, cx@entry=0x7ffff5f19000, obj=..., receiver=..., id=..., nameLookup=nameLookup@entry=NotNameLookup, vp=...) at js/src/vm/NativeObject.cpp:2504
    #8  0x0000555555b80040 in js::NativeGetProperty (cx=cx@entry=0x7ffff5f19000, obj=..., receiver=..., receiver@entry=..., id=..., id@entry=..., vp=..., vp@entry=...) at js/src/vm/NativeObject.cpp:2555
    #9  0x00005555558f5364 in js::GetProperty (cx=0x7ffff5f19000, obj=..., receiver=..., id=..., vp=...) at js/src/vm/ObjectOperations-inl.h:117
    #10 0x00005555558d27b0 in js::GetProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=<optimized out>) at js/src/vm/ObjectOperations-inl.h:124
    #11 js::GetProperty (cx=<optimized out>, v=..., name=..., vp=...) at js/src/vm/Interpreter.cpp:4487
    #12 0x00005555558db688 in GetPropertyOperation (vp=..., lval=..., pc=<optimized out>, script=..., fp=<optimized out>, cx=0x7ffff5f19000) at js/src/vm/Interpreter.cpp:216
    #13 Interpret (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:2767
    #14 0x00005555558e9146 in js::RunScript (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:423
    #15 0x00005555558e998f in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:563
    #16 0x00005555558e9e0d in InternalCall (cx=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:590
    #17 0x00005555558e9f80 in js::Call (cx=<optimized out>, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
    #18 0x0000555555e3973e in JS_CallFunction (cx=<optimized out>, obj=..., fun=..., fun@entry=..., args=..., rval=..., rval@entry=...) at js/src/jsapi.cpp:2592
    #19 0x0000555555c42f85 in RunIterativeFailureTest (cx=<optimized out>, cx@entry=0x7ffff5f19000, params=..., simulator=...) at js/src/builtin/TestingFunctions.cpp:1928
    #20 0x0000555555c4373a in OOMTest (cx=cx@entry=0x7ffff5f19000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:2109
    #21 0x00005555558f2539 in CallJSNative (cx=0x7ffff5f19000, native=native@entry=0x555555c43670 <OOMTest(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:443
    #22 0x00005555558e96c9 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:535
    #23 0x00005555558e9e0d in InternalCall (cx=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:590
    #24 0x00005555558daf7b in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:594
    #25 Interpret (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:3080
    #26 0x00005555558e9146 in js::RunScript (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:423
    #27 0x00005555558ec864 in js::ExecuteKernel (cx=<optimized out>, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7ffff59560a0) at js/src/vm/Interpreter.cpp:782
    #28 0x00005555558eceac in js::Execute (cx=<optimized out>, cx@entry=0x7ffff5f19000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7ffff59560a0) at js/src/vm/Interpreter.cpp:816
    #29 0x0000555555a06fb0 in ExecuteScript (cx=0x7ffff5f19000, scope=scope@entry=..., script=script@entry=..., rval=rval@entry=0x7ffff59560a0) at js/src/vm/CompilationAndEvaluation.cpp:435
    #30 0x0000555555a5859c in ExecuteScript (cx=<optimized out>, envChain=..., scriptArg=..., rval=0x7ffff59560a0) at js/src/vm/CompilationAndEvaluation.cpp:455
    #31 0x000055555585b7d5 in Evaluate (cx=<optimized out>, cx@entry=0x7ffff5f19000, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:2288
    #32 0x00005555558f2539 in CallJSNative (cx=0x7ffff5f19000, native=native@entry=0x55555585ab30 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:443
    [...]
    #46 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:11391
    rax	0x555557c969a0	93825033398688
    rbx	0x38	56
    rcx	0x555556b5d178	93825015337336
    rdx	0x0	0
    rsi	0x7ffff6eeb770	140737336227696
    rdi	0x7ffff6eea540	140737336223040
    rbp	0x7fffffffa860	140737488332896
    rsp	0x7fffffffa840	140737488332864
    r8	0x7ffff6eeb770	140737336227696
    r9	0x7ffff7fe6cc0	140737354034368
    r10	0x58	88
    r11	0x7ffff6b927a0	140737332717472
    r12	0x78172385240	8252548469312
    r13	0x0	0
    r14	0x0	0
    r15	0x7fffffffa96f	140737488333167
    rip	0x555555e3a8b9 <js::GlobalObject::classIsInitialized(JSProtoKey) const+377>
    => 0x555555e3a8b9 <js::GlobalObject::classIsInitialized(JSProtoKey) const+377>:	movl   $0x0,0x0
       0x555555e3a8c4 <js::GlobalObject::classIsInitialized(JSProtoKey) const+388>:	ud2

	Pulsebot
	Comment 48 • 2 years ago

Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5d4bbe51337a
Initialize Object and Function constructors when creating globals. r=jandem