1546232 - Assertion failure: shape, at js/src/vm/SelfHosting.cpp:3139

Status: RESOLVED FIXED

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision b783cd5203ea (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off --more-compartments):

var g = newGlobal();
var ga = g["Int8Array"].from([1, 2, 3]);
gczeal(14);
var target = newGlobal({newCompartment: true});
g.eval(`
  var p = new Promise(() => {});
  var q = p.then();
  var s = Promise.all([p, q]);
`);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  GetUnclonedValue (cx=cx@entry=0x7ffff5f19000, selfHostedObject=..., selfHostedObject@entry=..., id=..., id@entry=..., vp=...) at js/src/vm/SelfHosting.cpp:3139
#1  0x0000555555cb0a05 in JSRuntime::getUnclonedSelfHostedValue (this=this@entry=0x7ffff5f1b000, cx=cx@entry=0x7ffff5f19000, name=..., vp=vp@entry=...) at js/src/vm/SelfHosting.cpp:3469
#2  0x0000555555cb0b1c in JSRuntime::getUnclonedSelfHostedFunction (this=0x7ffff5f1b000, cx=0x7ffff5f19000, name=...) at js/src/vm/SelfHosting.cpp:3475
#3  0x0000555555cc5a44 in JSRuntime::cloneSelfHostedFunctionScript (this=<optimized out>, cx=<optimized out>, name=..., name@entry=..., targetFun=targetFun@entry=...) at js/src/vm/SelfHosting.cpp:3418
#4  0x0000555555b2be58 in JSFunction::createScriptForLazilyInterpretedFunction (cx=<optimized out>, fun=...) at js/src/vm/JSFunction.cpp:1694
#5  0x0000555555864930 in JSFunction::getOrCreateScript (cx=<optimized out>, fun=...) at js/src/vm/JSFunction.h:545
#6  0x00005555558e6972 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:537
#7  0x00005555558e703d in InternalCall (cx=cx@entry=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:589
#8  0x00005555558e71b0 in js::Call (cx=cx@entry=0x7ffff5f19000, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:605
#9  0x0000555555aab816 in js::Call (rval=..., thisv=..., fval=..., cx=0x7ffff5f19000) at js/src/vm/Interpreter.h:84
#10 JS::ForOfIterator::init (this=this@entry=0x7fffffffb360, iterable=iterable@entry=..., nonIterableBehavior=nonIterableBehavior@entry=JS::ForOfIterator::AllowNonIterable) at js/src/vm/ForOfIterator.cpp:82
#11 0x00005555559870a8 in CommonStaticAllRace (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., mode=mode@entry=IterationMode::All) at js/src/builtin/Promise.cpp:2335
#12 0x0000555555989b9b in Promise_static_all (cx=cx@entry=0x7ffff5f19000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Promise.cpp:2393
#13 0x00005555558ef719 in CallJSNative (cx=0x7ffff5f19000, native=native@entry=0x555555989b60 <Promise_static_all(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:442
#14 0x00005555558e68f9 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:534
#15 0x00005555558e703d in InternalCall (cx=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:589
#16 0x00005555558d81ab in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:593
#17 Interpret (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:3079
#18 0x00005555558e6376 in js::RunScript (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:422
#19 0x00005555558e9a94 in js::ExecuteKernel (cx=<optimized out>, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=0x7fffffffc528) at js/src/vm/Interpreter.cpp:781
#20 0x0000555555927399 in EvalKernel (cx=<optimized out>, cx@entry=0x7ffff5f19000, v=..., evalType=evalType@entry=INDIRECT_EVAL, caller=..., env=..., pc=pc@entry=0x0, vp=...) at js/src/builtin/Eval.cpp:327
#21 0x0000555555927c15 in js::IndirectEval (cx=cx@entry=0x7ffff5f19000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Eval.cpp:425
#22 0x00005555558ef719 in CallJSNative (cx=0x7ffff5f19000, native=native@entry=0x555555927b60 <js::IndirectEval(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:442
#23 0x00005555558e68f9 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:534
#24 0x00005555558e703d in InternalCall (cx=cx@entry=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:589
#25 0x00005555558e71b0 in js::Call (cx=cx@entry=0x7ffff5f19000, fval=..., fval@entry=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:605
#26 0x0000555555e75520 in js::ForwardingProxyHandler::call (this=<optimized out>, cx=0x7ffff5f19000, proxy=..., args=...) at js/src/proxy/Wrapper.cpp:162
#27 0x0000555555e5f293 in js::CrossCompartmentWrapper::call (this=0x555557c3e0a0 <js::CrossCompartmentWrapper::singleton>, cx=<optimized out>, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:237
#28 0x0000555555e6b9f5 in js::Proxy::call (cx=0x7ffff5f19000, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:504
#29 0x00005555558e6e06 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f19000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:508
#30 0x00005555558e703d in InternalCall (cx=0x7ffff5f19000, args=...) at js/src/vm/Interpreter.cpp:589
#31 0x00005555558d81ab in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:593
#32 Interpret (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:3079
#33 0x00005555558e6376 in js::RunScript (cx=0x7ffff5f19000, state=...) at js/src/vm/Interpreter.cpp:422
[...]
#41 0x000055555582fd15 in Shell (envp=<optimized out>, op=0x7fffffffd980, cx=<optimized out>) at js/src/shell/js.cpp:10754
#42 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:11369
rax	0x555557c87340	93825033335616
rbx	0x555556af961a	93825014928922
rcx	0x7ffff6c1c2dd	140737333281501
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffffffaaf0	140737488333552
rsp	0x7fffffffaa90	140737488333456
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6cc0	140737354034368
r10	0x58	88
r11	0x7ffff6b927a0	140737332717472
r12	0x7fffffffaac0	140737488333504
r13	0x7ffff5f19000	140737319636992
r14	0x7fffffffab60	140737488333664
r15	0x7fffffffad20	140737488334112
rip	0x555555ca6fa9 <GetUnclonedValue(JSContext*, js::HandleNativeObject, JS::HandleId, JS::MutableHandleValue)+409>
=> 0x555555ca6fa9 <GetUnclonedValue(JSContext*, js::HandleNativeObject, JS::HandleId, JS::MutableHandleValue)+409>:	movl   $0x0,0x0
   0x555555ca6fb4 <GetUnclonedValue(JSContext*, js::HandleNativeObject, JS::HandleId, JS::MutableHandleValue)+420>:	ud2

Marking s-s because the test involves gczeal.

Comment 1

[Jan de Mooij [:jandem]]

We're looking for a "values" property on the self-hosting global but we can't find it. This happens when looking up the iterator symbol:

JS_SELF_HOSTED_SYM_FN(iterator, "ArrayValues", 0, 0),

In Array.js:

function ArrayValues() {
    return CreateArrayIterator(this, ITEM_KIND_VALUE);
}
_SetCanonicalName(ArrayValues, "values");

It looks like we're conflating the function's name, "values", with the property name on the global. I think this happens in CloneObject where we set LAZY_FUNCTION_NAME_SLOT to "values": https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/js/src/vm/SelfHosting.cpp#3274-3301

However I don't know yet why this isn't a problem elsewhere.

Slightly simpler test:

var g = newGlobal();
var ga = g.Int8Array.from([1, 2, 3]);
relazifyFunctions();
var target = newGlobal({newCompartment: true});
g.eval(`
  var p = new Promise(() => {});
  var q = p.then();
  var s = Promise.all([p, q]);
`);

Comment 2

[Jan de Mooij [:jandem]]

André, maybe you know off-hand what's going on here? If not I can take a closer look.

Comment 3

[Jan de Mooij [:jandem]]

Also NI'ing arai because of bug 1235656.

Comment 4

[Jan de Mooij [:jandem]]

Simpler test:

var g = newGlobal();
g.Int8Array.from([]);
relazifyFunctions();
g.eval(`Promise.all([]);`);

Comment 5

[André Bargull [:anba]]

There's no assertion with https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/js/src/builtin/TypedArray.js#1554-1588 commented out. So maybe there's some issue when cloning a function without creating its script (because the comparisonsTypedArrayStaticFrom doesn't call ArrayValues, but only observes its equality)? So when we later actually call ArrayValues, the "name" property can no longer be used to get the correct script from the self-hosting global.

Comment 6

[André Bargull [:anba]]

FWIW g.eval("Promise.all([]);"); can be further simplified to g.eval("[].values()"); in the test case.

Comment 7

[Tooru Fujisawa [:arai]]

We have extended slot in self-hosted global's functions, that is currently used only in debug build.

js/src/builtin/SelfHostingDefines.h

// The extended slot which contains a boolean value that indicates whether
// that the canonical name of the self-hosted builtins is set in self-hosted
// global. This slot is used only in debug build.
#define HAS_SELFHOSTED_CANONICAL_NAME_SLOT 0

We can change the slot to store the original function name (the global binding name) and copy it to cloned functions.
that simplifies how extended slot is used in self-hosted functions.

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1546232 - Simplify value in extended slot of self-hosted functions. r?anba

Comment 9

[Jan de Mooij [:jandem]]

arai do you agree we can open this up? shape->slot() should be a near-null crash in opt builds. I don't think there are other risks?

I noticed we have similar crashes in GetUnclonedValue in the wild: https://crash-stats.mozilla.com/signature/?signature=GetUnclonedValue

Comment 10

[Tooru Fujisawa [:arai]]

I agree. this should result only in safe crash.

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1546232 - Simplify value in extended slot of self-hosted functions. r?anba

Comment 12

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 13

[Patricia Lawless]

Hi Steven, is this ready to land? (tracking for 68)

Comment 14

[Steven DeTar [:sdetar]]

Arai, could you help answer Patricia's question around status of this fix?

Comment 15

[Tooru Fujisawa [:arai]]

thank you for the reminder.
I'll land this shortly after try run.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd2ad6987449
Simplify value in extended slot of self-hosted functions. r=anba

Comment 17

[Oana Pop-Rus]

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