Skip to content

Commit a0c7d51

Browse files
committed
Bug 1703469: Add FunctionHasStableBaseScript r=jandem
Some Marionette tests failed on the assertion [here](https://searchfox.org/mozilla-central/rev/8b52ebe3cddf0e63bb42e8b51618bc3ab8dcb12d/js/src/jit/MIR.cpp#7102) because we had a getter calling a non-lambda self-hosted function. The fix is to reuse the checks we already have in emitCalleeGuard. Differential Revision: https://phabricator.services.mozilla.com/D252878
1 parent ac74992 commit a0c7d51

File tree

2 files changed

+35
-9
lines changed

2 files changed

+35
-9
lines changed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// |jit-test| --fast-warmup; --no-threads
2+
3+
function makeObjWithSelfHostedGetter() {
4+
var o = {};
5+
Object.defineProperty(o, "x", {
6+
get: Number.isFinite
7+
});
8+
return o;
9+
}
10+
11+
for (var i = 0; i < 100; i++) {
12+
let o = makeObjWithSelfHostedGetter();
13+
o.x;
14+
}

js/src/jit/CacheIR.cpp

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,25 @@ void IRGenerator::emitCallGetterResultNoGuards(NativeGetPropKind kind,
976976
}
977977
}
978978

979+
static bool FunctionHasStableBaseScript(JSFunction* fun) {
980+
// When guarding a callee, guarding on the JSFunction* is most efficient,
981+
// but doesn't work well for lambda clones (multiple functions with the
982+
// same BaseScript). We can instead guard on the BaseScript itself.
983+
if (!fun->hasBaseScript()) {
984+
return false;
985+
}
986+
// Self-hosted functions are more complicated: top-level functions can be
987+
// relazified using SelfHostedLazyScript and this means they don't have a
988+
// stable BaseScript pointer. These functions are never lambda clones, though,
989+
// so we can just always guard on the JSFunction*. Self-hosted lambdas are
990+
// never relazified so there we use the normal heuristics.
991+
if (fun->isSelfHostedBuiltin() && !fun->isLambda()) {
992+
return false;
993+
}
994+
return true;
995+
}
996+
997+
979998
// See the SMDOC comment in vm/GetterSetter.h for more info on Getter/Setter
980999
// properties
9811000
void IRGenerator::emitGuardGetterSetterSlot(NativeObject* holder,
@@ -1000,7 +1019,7 @@ void IRGenerator::emitGuardGetterSetterSlot(NativeObject* holder,
10001019
JSObject* accessor = isGetter ? holder->getGetter(prop)
10011020
: holder->getSetter(prop);
10021021
JSFunction* fun = &accessor->as<JSFunction>();
1003-
if (fun->hasBaseScript()) {
1022+
if (FunctionHasStableBaseScript(fun)) {
10041023
ValOperandId getterSetterId = EmitLoadSlot(writer, holder, holderId, slot);
10051024
ObjOperandId functionId = writer.loadGetterSetterFunction(getterSetterId,
10061025
isGetter);
@@ -6649,14 +6668,7 @@ void IRGenerator::emitCalleeGuard(ObjOperandId calleeId, JSFunction* callee) {
66496668
// for lambda clones (multiple functions with the same BaseScript). We guard
66506669
// on the function's BaseScript if the callee is scripted and this isn't the
66516670
// first IC stub.
6652-
//
6653-
// Self-hosted functions are more complicated: top-level functions can be
6654-
// relazified using SelfHostedLazyScript and this means they don't have a
6655-
// stable BaseScript pointer. These functions are never lambda clones, though,
6656-
// so we can just always guard on the JSFunction*. Self-hosted lambdas are
6657-
// never relazified so there we use the normal heuristics.
6658-
if (isFirstStub_ || !callee->hasBaseScript() ||
6659-
(callee->isSelfHostedBuiltin() && !callee->isLambda())) {
6671+
if (isFirstStub_ || !FunctionHasStableBaseScript(callee)) {
66606672
writer.guardSpecificFunction(calleeId, callee);
66616673
} else {
66626674
MOZ_ASSERT_IF(callee->isSelfHostedBuiltin(),

0 commit comments

Comments
 (0)