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

RESOLVED FIXED in Firefox 68

Status

()

defect
P1
critical
RESOLVED FIXED
2 months ago
13 days ago

People

(Reporter: decoder, Assigned: arai)

Tracking

(Blocks 1 bug, 5 keywords)

Trunk
mozilla68
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox67 wontfix, firefox68 fixed)

Details

(Whiteboard: [jsbugmon:])

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

2 months ago

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.

Flags: needinfo?(jdemooij)

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]);
`);
Flags: needinfo?(jdemooij)

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

Flags: needinfo?(andrebargull)

Also NI'ing arai because of bug 1235656.

Flags: needinfo?(arai.unmht)

Simpler test:

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

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.

Flags: needinfo?(andrebargull)

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

Assignee

Comment 7

2 months ago

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.

Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Flags: needinfo?(arai.unmht)

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

Assignee

Comment 10

2 months ago

I agree. this should result only in safe crash.

Group: javascript-core-security
Attachment #9060647 - Attachment is obsolete: true
Attachment #9060607 - Attachment description: Bug 1546232 - Simplify value in extended slot of self-hosted functions. r?jandem → Bug 1546232 - Simplify value in extended slot of self-hosted functions. r?anba

Updated

2 months ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]

Comment 12

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

Updated

2 months ago
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
Priority: -- → P1

Comment 13

2 months ago

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

Flags: needinfo?(sdetar)

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

Flags: needinfo?(sdetar) → needinfo?(arai.unmht)
Assignee

Comment 15

2 months ago

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

Flags: needinfo?(arai.unmht)

Comment 16

2 months ago
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/bd2ad6987449
Simplify value in extended slot of self-hosted functions. r=anba

Comment 17

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.