1657557 - Assertion failure: obj->staticPrototypeIsImmutable(), at gc/GC.cpp:7871

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200806-6e35e01646d7 (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

function runNormalTests(global) {
  nestedTarget = runNormalTests.__proto__;
  var nestedProxy = new global.Proxy(new Proxy(nestedTarget, {}), {});
  setImmutablePrototype(nestedProxy);
}
var subsumingEverything = newGlobal({ principal: ~0 });
runNormalTests(subsumingEverything);
let t49 = cacheEntry(`
  function f() {};
`);
evaluate(t49, { sourceIsLazy: true, saveIncrementalBytecode: true });
offThreadDecodeScript(t49);
runOffThreadDecodedScript();

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x00005555575bbd04 in js::gc::GCRuntime::mergeRealms(JS::Realm*, JS::Realm*) ()
#0  0x00005555575bbd04 in js::gc::GCRuntime::mergeRealms(JS::Realm*, JS::Realm*) ()
#1  0x00005555575bab45 in js::gc::MergeRealms(JS::Realm*, JS::Realm*) ()
#2  0x0000555556ef849d in js::GlobalHelperThreadState::mergeParseTaskRealm(JSContext*, js::ParseTask*, JS::Realm*) ()
#3  0x0000555556ef77b4 in js::GlobalHelperThreadState::finishParseTaskCommon(JSContext*, js::ParseTaskKind, JS::OffThreadToken*) ()
#4  0x0000555556ef8595 in js::GlobalHelperThreadState::finishSingleParseTask(JSContext*, js::ParseTaskKind, JS::OffThreadToken*) ()
#5  0x0000555556ef8e13 in js::GlobalHelperThreadState::finishScriptDecodeTask(JSContext*, JS::OffThreadToken*) ()
#6  0x0000555556b56409 in runOffThreadDecodedScript(JSContext*, unsigned int, JS::Value*) ()
#7  0x0000555556caf7e2 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
[...]
#18 0x0000555556b2e609 in Shell(JSContext*, js::cli::OptionParser*, char**) ()
#19 0x0000555556b270c0 in main ()
rax	0x5555557fad19	93824995011865
rbx	0x7ffff56e1000	140737311019008
rcx	0x555558473a80	93825041644160
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffb6c0	140737488336576
rsp	0x7fffffffb580	140737488336256
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f9ddc0	140737353735616
r10	0x58	88
r11	0x7ffff6dac7a0	140737334921120
r12	0x7fffffffb638	140737488336440
r13	0xb93821b8070	12728170938480
r14	0xb9382185040	12728170729536
r15	0x7ffff56f0c00	140737311083520
rip	0x5555575bbd04 <js::gc::GCRuntime::mergeRealms(JS::Realm*, JS::Realm*)+4388>
=> 0x5555575bbd04 <_ZN2js2gc9GCRuntime11mergeRealmsEPN2JS5RealmES4_+4388>:	movl   $0x1ebf,0x0
   0x5555575bbd0f <_ZN2js2gc9GCRuntime11mergeRealmsEPN2JS5RealmES4_+4399>:	callq  0x555556bb5e3e <abort>

GC assert, marking s-s until investigated.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200806033456-6e35e01646d7.
Failed to bisect testcase (Start build crashes!):
> Start: e8b7c48d4e7ed1b63aeedff379b51e566ea499d9 (20191107015224)
> End: 6e35e01646d7c465893a172a0b4fb116c2293d2a (20200806033456)
> BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=False, coverage=False, valgrind=False)

Comment 3

[Jon Coppeard (:jonco)]

This is more a problem with off-thread parsing than GC per se. This code may also be removed by the stencil work.

Comment 4

[Nicolas B. Pierron [:nbp]]

Ted, would you know when this code can be removed?
However, if it is still in older versions we might still have to work on a patch to backport.

Comment 5

[Daniel Veditz [:dveditz]]

What are the implications of this in a security sense? Does it affect web content? If not we may not need to backport.

Comment 6

[Ted Campbell [:tcampbell]]

Reduced test case:

setImmutablePrototype(Function.__proto__);

if (helperThreadCount() > 0) {
    offThreadCompileScript("function f() {}");
    runOffThreadScript();
}

The setImmutablePrototype is only a testing function, and I think the assert is also incorrect. Still confirming but this is likely not a security issue at all.

Comment 7

[Ted Campbell [:tcampbell]]

This assert was added in Bug 1449033 as a way to assure that the code (here)[https://searchfox.org/mozilla-central/rev/26b13464c2beb26e0d864d561c30e817a85c348a/js/src/vm/GlobalObject.cpp#528-531] was called. The assert assumed that this flag could not be set arbitrarily, but that is not true with the setImmutablePrototype testing function.

Not a security issue. Patch incoming.

Comment 8

[Ted Campbell [:tcampbell]]

Attachment: Bug 1657557 - Remove erroneous assert in GCRuntime::mergeRealms r?jonco

This assert was intended to assure that off-thread parse realm sets the same
special prototype flags off-thread as on-thread. In practice though, the
setImmutablePrototype testing function allows us to set the flag on a
common prototype which confuses the assert.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57b473b0e71a
Remove erroneous assert in GCRuntime::mergeRealms r=jonco

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/57b473b0e71a

Comment 11

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20200807213618-768023eab227.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.