From fd89cca730ca5e040b27c7da2f8a9753be6a126f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Thu, 30 Jan 2020 16:37:02 +0000 Subject: [PATCH] Bug 1608809 - Part 1: Don't throw for non-constructors in JSOp::SuperFun. r=jandem Modify JSOp::SuperFun to only retrieve the prototype without any further checks. Drive-by change: Take advantage that the object whose prototype gets retrieved is guaranteed to be a JSFunction, so we neither have to worry about proxy objects nor lazy prototypes. Differential Revision: https://phabricator.services.mozilla.com/D60677 --HG-- extra : moz-landing-system : lando --- js/src/jit/BaselineCodeGen.cpp | 52 ++++++++++----------- js/src/jit/VMFunctionList-inl.h | 1 - js/src/tests/jstests.list | 8 ++-- js/src/tests/non262/class/superCallOrder.js | 12 +++-- js/src/vm/Interpreter.cpp | 39 +++++----------- js/src/vm/Interpreter.h | 2 - js/src/vm/Opcodes.h | 1 - 7 files changed, 49 insertions(+), 66 deletions(-) diff --git a/js/src/jit/BaselineCodeGen.cpp b/js/src/jit/BaselineCodeGen.cpp index dd3486ccfa0d..28e53faa2e73 100644 --- a/js/src/jit/BaselineCodeGen.cpp +++ b/js/src/jit/BaselineCodeGen.cpp @@ -5706,46 +5706,46 @@ bool BaselineCodeGen::emit_SuperFun() { Register callee = R0.scratchReg(); Register proto = R1.scratchReg(); +#ifdef DEBUG Register scratch = R2.scratchReg(); +#endif // Unbox callee. masm.unboxObject(R0, callee); +#ifdef DEBUG + Label classCheckDone; + masm.branchTestObjClass(Assembler::Equal, callee, &JSFunction::class_, + scratch, callee, &classCheckDone); + masm.assumeUnreachable("Unexpected non-JSFunction callee in JSOp::SuperFun"); + masm.bind(&classCheckDone); +#endif + // Load prototype of callee masm.loadObjProto(callee, proto); - // Use VMCall for missing or lazy proto - Label needVMCall; +#ifdef DEBUG + // We won't encounter a lazy proto, because |callee| is guaranteed to be a + // JSFunction and only proxy objects can have a lazy proto. MOZ_ASSERT(uintptr_t(TaggedProto::LazyProto) == 1); - masm.branchPtr(Assembler::BelowOrEqual, proto, ImmWord(1), &needVMCall); - // Use VMCall for non-JSFunction objects (eg. Proxy) - masm.branchTestObjClass(Assembler::NotEqual, proto, &JSFunction::class_, - scratch, proto, &needVMCall); + Label proxyCheckDone; + masm.branchPtr(Assembler::NotEqual, proto, ImmWord(1), &proxyCheckDone); + masm.assumeUnreachable("Unexpected lazy proto in JSOp::SuperFun"); + masm.bind(&proxyCheckDone); +#endif - // Use VMCall if not constructor - masm.load16ZeroExtend(Address(proto, JSFunction::offsetOfFlags()), scratch); - masm.branchTest32(Assembler::Zero, scratch, Imm32(FunctionFlags::CONSTRUCTOR), - &needVMCall); - - // Valid constructor - Label hasSuperFun; - masm.jump(&hasSuperFun); - - // Slow path VM Call - masm.bind(&needVMCall); - prepareVMCall(); - pushArg(callee); - - using Fn = JSObject* (*)(JSContext*, HandleObject); - if (!callVM()) { - return false; - } - masm.movePtr(ReturnReg, proto); + Label nullProto, done; + masm.branchPtr(Assembler::Equal, proto, ImmWord(0), &nullProto); // Box prototype and return - masm.bind(&hasSuperFun); masm.tagValue(JSVAL_TYPE_OBJECT, proto, R1); + masm.jump(&done); + + masm.bind(&nullProto); + masm.moveValue(NullValue(), R1); + + masm.bind(&done); frame.push(R1); return true; } diff --git a/js/src/jit/VMFunctionList-inl.h b/js/src/jit/VMFunctionList-inl.h index 3c37ddf3434f..bd1519c58ba2 100644 --- a/js/src/jit/VMFunctionList-inl.h +++ b/js/src/jit/VMFunctionList-inl.h @@ -273,7 +273,6 @@ namespace jit { _(StringsNotEqual, js::jit::StringsEqual) \ _(SubValues, js::SubValues) \ _(SubstringKernel, js::SubstringKernel) \ - _(SuperFunOperation, js::SuperFunOperation) \ _(ThrowBadDerivedReturn, js::jit::ThrowBadDerivedReturn) \ _(ThrowCheckIsObject, js::ThrowCheckIsObject) \ _(ThrowInitializedThis, js::ThrowInitializedThis) \ diff --git a/js/src/tests/jstests.list b/js/src/tests/jstests.list index 239aa0c161c9..345da197e7a3 100644 --- a/js/src/tests/jstests.list +++ b/js/src/tests/jstests.list @@ -469,9 +469,6 @@ skip script test262/intl402/NumberFormat/prototype/format/signDisplay-ko-KR.js skip script test262/intl402/NumberFormat/prototype/format/signDisplay-de-DE.js skip script test262/intl402/NumberFormat/prototype/format/signDisplay-rounding.js -# https://bugzilla.mozilla.org/show_bug.cgi?id=1608809 -skip script test262/language/expressions/class/super-evaluation-order.js - ########################################################### # Tests disabled due to issues in test262 importer script # @@ -504,5 +501,10 @@ skip script test262/built-ins/AggregateError/newtarget-proto-fallback.js # hour-cycle to "h24", which isn't allowed per the above CLDR data. skip script test262/intl402/DateTimeFormat/prototype/resolvedOptions/hourCycle-default.js +# https://github.com/tc39/test262/pull/2464 only added a new test, but didn't +# update the existing tests. +skip script test262/language/statements/class/subclass/class-definition-null-proto-super.js +skip script test262/language/expressions/super/call-proto-not-ctor.js + # Tests expects RangeError, but should instead expect TypeError. skip script test262/intl402/NumberFormat/constructor-order.js diff --git a/js/src/tests/non262/class/superCallOrder.js b/js/src/tests/non262/class/superCallOrder.js index 051658eef344..46a1b0e57c78 100644 --- a/js/src/tests/non262/class/superCallOrder.js +++ b/js/src/tests/non262/class/superCallOrder.js @@ -8,19 +8,21 @@ class beforeSwizzle extends base { new beforeSwizzle(); -// Again, testing both dynamic prototype dispatch, and that we get the function -// before evaluating args +function MyError() {} + +// Again, testing both dynamic prototype dispatch, and that we verify the function +// is a constructor after evaluating args class beforeThrow extends base { constructor() { - function thrower() { throw new Error(); } + function thrower() { throw new MyError(); } super(thrower()); } } Object.setPrototypeOf(beforeThrow, Math.sin); -// Will throw that Math.sin is not a constructor before evaluating the args -assertThrowsInstanceOf(() => new beforeThrow(), TypeError); +// Won't throw that Math.sin is not a constructor before evaluating the args +assertThrowsInstanceOf(() => new beforeThrow(), MyError); if (typeof reportCompare === 'function') reportCompare(0,0,"OK"); diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 1f8c4d00e614..3343f20b9825 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -326,6 +326,14 @@ JSFunction* js::MakeDefaultConstructor(JSContext* cx, HandleScript script, return ctor; } +static JSObject* SuperFunOperation(JSObject* callee) { + MOZ_ASSERT(callee->as().isClassConstructor()); + MOZ_ASSERT( + callee->as().baseScript()->isDerivedClassConstructor()); + + return callee->as().staticPrototype(); +} + bool js::ReportIsNotFunction(JSContext* cx, HandleValue v, int numToSkip, MaybeConstruct construct) { unsigned error = construct ? JSMSG_NOT_CONSTRUCTOR : JSMSG_NOT_FUNCTION; @@ -4182,14 +4190,9 @@ static MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_CALLER bool Interpret(JSContext* cx, END_CASE(EnvCallee) CASE(SuperFun) { - ReservedRooted superEnvFunc(&rootObject0, - ®S.sp[-1].toObject()); - JSObject* superFun = SuperFunOperation(cx, superEnvFunc); - if (!superFun) { - goto error; - } - - REGS.sp[-1].setObject(*superFun); + JSObject* superEnvFunc = ®S.sp[-1].toObject(); + JSObject* superFun = SuperFunOperation(superEnvFunc); + REGS.sp[-1].setObjectOrNull(superFun); } END_CASE(SuperFun) @@ -5357,26 +5360,6 @@ JSObject* js::HomeObjectSuperBase(JSContext* cx, HandleObject homeObj) { return superBase; } -JSObject* js::SuperFunOperation(JSContext* cx, HandleObject callee) { - MOZ_ASSERT(callee->as().isClassConstructor()); - MOZ_ASSERT( - callee->as().baseScript()->isDerivedClassConstructor()); - - RootedObject superFun(cx); - - if (!GetPrototype(cx, callee, &superFun)) { - return nullptr; - } - - if (!superFun || !superFun->isConstructor()) { - RootedValue superFunVal(cx, ObjectOrNullValue(superFun)); - ReportIsNotFunction(cx, superFunVal, JSDVG_IGNORE_STACK, CONSTRUCT); - return nullptr; - } - - return superFun; -} - bool js::ThrowInitializedThis(JSContext* cx) { JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_REINIT_THIS); return false; diff --git a/js/src/vm/Interpreter.h b/js/src/vm/Interpreter.h index b44ba120c889..1ca23506c096 100644 --- a/js/src/vm/Interpreter.h +++ b/js/src/vm/Interpreter.h @@ -659,8 +659,6 @@ JSFunction* MakeDefaultConstructor(JSContext* cx, HandleScript script, JSObject* HomeObjectSuperBase(JSContext* cx, HandleObject homeObj); -JSObject* SuperFunOperation(JSContext* cx, HandleObject callee); - bool SetPropertySuper(JSContext* cx, HandleObject obj, HandleValue receiver, HandlePropertyName id, HandleValue rval, bool strict); diff --git a/js/src/vm/Opcodes.h b/js/src/vm/Opcodes.h index 104a85d2a38f..ea6f68b39c55 100644 --- a/js/src/vm/Opcodes.h +++ b/js/src/vm/Opcodes.h @@ -1932,7 +1932,6 @@ MACRO(SpreadSuperCall, spread_super_call, NULL, 1, 4, 1, JOF_BYTE|JOF_INVOKE|JOF_CONSTRUCT|JOF_SPREAD|JOF_TYPESET|JOF_IC) \ /* * Push the prototype of `callee` in preparation for calling `super()`. - * Throw a TypeError if that value is not a constructor. * * `callee` must be a derived class constructor. *