From 37b0f0cbacd80d2c67b0ddcfaa99a50e06c94c8f Mon Sep 17 00:00:00 2001 From: Ted Campbell Date: Tue, 23 May 2017 16:23:53 -0400 Subject: [PATCH] Bug 1169743 - Rework JSOP_CLASSHERITAGE to be jit-friendly. r=shu Initial patch by Eric Faust [:efaust]. MozReview-Commit-ID: 52I0IY0cMJn --HG-- extra : rebase_source : 41f02d04de222fe512e87fb758ebb0284615ccf8 --- js/src/frontend/BytecodeEmitter.cpp | 125 +++++++++++++++++++++++----- js/src/js.msg | 3 +- js/src/jsopcode.cpp | 7 +- js/src/vm/Interpreter.cpp | 89 ++++++++++++-------- js/src/vm/Interpreter.h | 9 ++ js/src/vm/Opcodes.h | 17 ++-- 6 files changed, 181 insertions(+), 69 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index a45bfc6ba53c..2651d4f4c092 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -10665,33 +10665,110 @@ BytecodeEmitter::emitClass(ParseNode* pn) return false; } + // Pseudocode for class declarations: + // + // class extends BaseExpression { + // constructor() { ... } + // ... + // } + // + // + // if defined { + // let heritage = BaseExpression; + // + // if (heritage !== null) { + // funProto = heritage; + // objProto = heritage.prototype; + // } else { + // funProto = %FunctionPrototype%; + // objProto = null; + // } + // } else { + // objProto = %ObjectPrototype%; + // } + // + // let homeObject = ObjectCreate(objProto); + // + // if defined { + // if defined { + // cons = DefineMethod(, proto=homeObject, funProto=funProto); + // } else { + // cons = DefineMethod(, proto=homeObject); + // } + // } else { + // if defined { + // cons = DefaultDerivedConstructor(proto=homeObject, funProto=funProto); + // } else { + // cons = DefaultConstructor(proto=homeObject); + // } + // } + // + // cons.prototype = homeObject; + // homeObject.constructor = cons; + // + // EmitPropertyList(...) + // This is kind of silly. In order to the get the home object defined on // the constructor, we have to make it second, but we want the prototype // on top for EmitPropertyList, because we expect static properties to be // rarer. The result is a few more swaps than we would like. Such is life. if (heritageExpression) { - if (!emitTree(heritageExpression)) - return false; - if (!emit1(JSOP_CLASSHERITAGE)) - return false; - if (!emit1(JSOP_OBJWITHPROTO)) + IfThenElseEmitter ifThenElse(this); + + if (!emitTree(heritageExpression)) // ... HERITAGE return false; - // JSOP_CLASSHERITAGE leaves both protos on the stack. After - // creating the prototype, swap it to the bottom to make the - // constructor. - if (!emit1(JSOP_SWAP)) + // Heritage must be null or a non-generator constructor + if (!emit1(JSOP_CHECKCLASSHERITAGE)) // ... HERITAGE + return false; + + // [IF] (heritage !== null) + if (!emit1(JSOP_DUP)) // ... HERITAGE HERITAGE + return false; + if (!emit1(JSOP_NULL)) // ... HERITAGE HERITAGE NULL + return false; + if (!emit1(JSOP_STRICTNE)) // ... HERITAGE NE + return false; + + // [THEN] funProto = heritage, objProto = heritage.prototype + if (!ifThenElse.emitIfElse()) + return false; + if (!emit1(JSOP_DUP)) // ... HERITAGE HERITAGE + return false; + if (!emitAtomOp(cx->names().prototype, JSOP_GETPROP)) // ... HERITAGE PROTO + return false; + + // [ELSE] funProto = %FunctionPrototype%, objProto = null + if (!ifThenElse.emitElse()) + return false; + if (!emit1(JSOP_POP)) // ... + return false; + if (!emit2(JSOP_BUILTINPROTO, JSProto_Function)) // ... PROTO + return false; + if (!emit1(JSOP_NULL)) // ... PROTO NULL + return false; + + // [ENDIF] + if (!ifThenElse.emitEnd()) + return false; + + if (!emit1(JSOP_OBJWITHPROTO)) // ... HERITAGE HOMEOBJ + return false; + if (!emit1(JSOP_SWAP)) // ... HOMEOBJ HERITAGE return false; } else { - if (!emitNewInit(JSProto_Object)) + if (!emitNewInit(JSProto_Object)) // ... HOMEOBJ return false; } + // Stack currently has HOMEOBJ followed by optional HERITAGE. When HERITAGE + // is not used, an implicit value of %FunctionPrototype% is implied. + if (constructor) { - if (!emitFunction(constructor, !!heritageExpression)) + if (!emitFunction(constructor, !!heritageExpression)) // ... HOMEOBJ CONSTRUCTOR return false; if (constructor->pn_funbox->needsHomeObject()) { - if (!emit2(JSOP_INITHOMEOBJECT, 0)) + if (!emit2(JSOP_INITHOMEOBJECT, 0)) // ... HOMEOBJ CONSTRUCTOR return false; } } else { @@ -10712,34 +10789,34 @@ BytecodeEmitter::emitClass(ParseNode* pn) JSAtom *name = names ? names->innerBinding()->pn_atom : cx->names().empty; if (heritageExpression) { - if (!emitAtomOp(name, JSOP_DERIVEDCONSTRUCTOR)) + if (!emitAtomOp(name, JSOP_DERIVEDCONSTRUCTOR)) // ... HOMEOBJ CONSTRUCTOR return false; } else { - if (!emitAtomOp(name, JSOP_CLASSCONSTRUCTOR)) + if (!emitAtomOp(name, JSOP_CLASSCONSTRUCTOR)) // ... HOMEOBJ CONSTRUCTOR return false; } } - if (!emit1(JSOP_SWAP)) + if (!emit1(JSOP_SWAP)) // ... CONSTRUCTOR HOMEOBJ return false; - if (!emit1(JSOP_DUP2)) + if (!emit1(JSOP_DUP2)) // ... CONSTRUCTOR HOMEOBJ CONSTRUCTOR HOMEOBJ return false; - if (!emitAtomOp(cx->names().prototype, JSOP_INITLOCKEDPROP)) + if (!emitAtomOp(cx->names().prototype, JSOP_INITLOCKEDPROP)) // ... CONSTRUCTOR HOMEOBJ CONSTRUCTOR return false; - if (!emitAtomOp(cx->names().constructor, JSOP_INITHIDDENPROP)) + if (!emitAtomOp(cx->names().constructor, JSOP_INITHIDDENPROP)) // ... CONSTRUCTOR HOMEOBJ return false; RootedPlainObject obj(cx); - if (!emitPropertyList(classMethods, &obj, ClassBody)) + if (!emitPropertyList(classMethods, &obj, ClassBody)) // ... CONSTRUCTOR HOMEOBJ return false; - if (!emit1(JSOP_POP)) + if (!emit1(JSOP_POP)) // ... CONSTRUCTOR return false; if (names) { ParseNode* innerName = names->innerBinding(); - if (!emitLexicalInitialization(innerName)) + if (!emitLexicalInitialization(innerName)) // ... CONSTRUCTOR return false; // Pop the inner scope. @@ -10749,15 +10826,17 @@ BytecodeEmitter::emitClass(ParseNode* pn) ParseNode* outerName = names->outerBinding(); if (outerName) { - if (!emitLexicalInitialization(outerName)) + if (!emitLexicalInitialization(outerName)) // ... CONSTRUCTOR return false; // Only class statements make outer bindings, and they do not leave // themselves on the stack. - if (!emit1(JSOP_POP)) + if (!emit1(JSOP_POP)) // ... return false; } } + // The CONSTRUCTOR is left on stack if this is an expression. + MOZ_ALWAYS_TRUE(sc->setLocalStrictMode(savedStrictness)); return true; diff --git a/js/src/js.msg b/js/src/js.msg index 0a555d4cc49c..67480b91e1a7 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -102,11 +102,12 @@ MSG_DEF(JSMSG_CANT_SET_PROTO_OF, 1, JSEXN_TYPEERR, "can't set prototype of MSG_DEF(JSMSG_CANT_SET_PROTO_CYCLE, 0, JSEXN_TYPEERR, "can't set prototype: it would cause a prototype chain cycle") MSG_DEF(JSMSG_INVALID_ARG_TYPE, 3, JSEXN_TYPEERR, "Invalid type: {0} can't be a{1} {2}") MSG_DEF(JSMSG_TERMINATED, 1, JSEXN_ERR, "Script terminated by timeout at:\n{0}") -MSG_DEF(JSMSG_PROTO_NOT_OBJORNULL, 1, JSEXN_TYPEERR, "{0}.prototype is not an object or null") MSG_DEF(JSMSG_CANT_CALL_CLASS_CONSTRUCTOR, 0, JSEXN_TYPEERR, "class constructors must be invoked with |new|") MSG_DEF(JSMSG_UNINITIALIZED_THIS, 1, JSEXN_REFERENCEERR, "|this| used uninitialized in {0} class constructor") MSG_DEF(JSMSG_UNINITIALIZED_THIS_ARROW, 0, JSEXN_REFERENCEERR, "|this| used uninitialized in arrow function in class constructor") MSG_DEF(JSMSG_BAD_DERIVED_RETURN, 1, JSEXN_TYPEERR, "derived class constructor returned invalid value {0}") +MSG_DEF(JSMSG_BAD_HERITAGE, 2, JSEXN_TYPEERR, "class heritage {0} is {1}") +MSG_DEF(JSMSG_NOT_OBJORNULL, 1, JSEXN_TYPEERR, "{0} is not an object or null") // JSON MSG_DEF(JSMSG_JSON_BAD_PARSE, 3, JSEXN_SYNTAXERR, "JSON.parse: {0} at line {1} column {2} of the JSON data") diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index 84f227e10901..2f5c596dd2fd 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -729,6 +729,7 @@ BytecodeParser::simulateOp(JSOp op, uint32_t offset, OffsetAndDefIndex* offsetSt case JSOP_CHECKOBJCOERCIBLE: case JSOP_CHECKTHIS: case JSOP_CHECKTHISREINIT: + case JSOP_CHECKCLASSHERITAGE: case JSOP_DEBUGCHECKSELFHOSTED: case JSOP_INITGLEXICAL: case JSOP_INITLEXICAL: @@ -1970,12 +1971,6 @@ ExpressionDecompiler::decompilePC(jsbytecode* pc, uint8_t defIndex) case JSOP_DERIVEDCONSTRUCTOR: return write("CONSTRUCTOR"); - case JSOP_CLASSHERITAGE: - if (defIndex == 0) - return write("FUNCPROTO"); - MOZ_ASSERT(defIndex == 1); - return write("OBJPROTO"); - case JSOP_DOUBLE: return sprinter.printf("%lf", script->getConst(GET_UINT32_INDEX(pc)).toDouble()); diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 44741881140b..021fc6807327 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -971,6 +971,43 @@ js::TypeOfValue(const Value& v) return JSTYPE_SYMBOL; } +bool +js::CheckClassHeritageOperation(JSContext* cx, HandleValue heritage) +{ + if (IsConstructor(heritage)) + return true; + + if (heritage.isNull()) + return true; + + if (heritage.isObject()) { + ReportIsNotFunction(cx, heritage, 0, CONSTRUCT); + return false; + } + + ReportValueError2(cx, JSMSG_BAD_HERITAGE, -1, heritage, nullptr, "not an object or null"); + return false; +} + +JSObject* +js::ObjectWithProtoOperation(JSContext* cx, HandleValue val) +{ + if (!val.isObjectOrNull()) { + ReportValueError(cx, JSMSG_NOT_OBJORNULL, -1, val, nullptr); + return nullptr; + } + + RootedObject proto(cx, val.toObjectOrNull()); + return NewObjectWithGivenProto(cx, proto); +} + +JSObject* +js::FunWithProtoOperation(JSContext* cx, HandleFunction fun, HandleObject parent, + HandleObject proto) +{ + return CloneFunctionObjectIfNotSingleton(cx, fun, parent, proto); +} + /* * Enter the new with environment using an object at sp[-1] and associate the * depth of the with block with sp + stackIndex. @@ -1942,7 +1979,6 @@ CASE(EnableInterruptsPseudoOpcode) CASE(JSOP_NOP) CASE(JSOP_NOP_DESTRUCTURING) CASE(JSOP_TRY_DESTRUCTURING_ITERCLOSE) -CASE(JSOP_UNUSED221) CASE(JSOP_UNUSED222) CASE(JSOP_UNUSED223) CASE(JSOP_CONDSWITCH) @@ -4099,37 +4135,25 @@ CASE(JSOP_ARRAYPUSH) } END_CASE(JSOP_ARRAYPUSH) -CASE(JSOP_CLASSHERITAGE) +CASE(JSOP_CHECKCLASSHERITAGE) { - ReservedRooted val(&rootValue0, REGS.sp[-1]); + HandleValue heritage = REGS.stackHandleAt(-1); - ReservedRooted objProto(&rootValue1); - ReservedRooted funcProto(&rootObject0); - if (val.isNull()) { - objProto = NullValue(); - if (!GetBuiltinPrototype(cx, JSProto_Function, &funcProto)) - goto error; - } else { - if (!IsConstructor(val)) { - ReportIsNotFunction(cx, val, 0, CONSTRUCT); - goto error; - } - - funcProto = &val.toObject(); - - if (!GetProperty(cx, funcProto, funcProto, cx->names().prototype, &objProto)) - goto error; - - if (!objProto.isObjectOrNull()) { - ReportValueError(cx, JSMSG_PROTO_NOT_OBJORNULL, -1, objProto, nullptr); - goto error; - } - } - - REGS.sp[-1].setObject(*funcProto); - PUSH_COPY(objProto); + if (!CheckClassHeritageOperation(cx, heritage)) + goto error; } -END_CASE(JSOP_CLASSHERITAGE) +END_CASE(JSOP_CHECKCLASSHERITAGE) + +CASE(JSOP_BUILTINPROTO) +{ + ReservedRooted builtin(&rootObject0); + MOZ_ASSERT(GET_UINT8(REGS.pc) < JSProto_LIMIT); + JSProtoKey key = static_cast(GET_UINT8(REGS.pc)); + if (!GetBuiltinPrototype(cx, key, &builtin)) + goto error; + PUSH_OBJECT(*builtin); +} +END_CASE(JSOP_BUILTINPROTO) CASE(JSOP_FUNWITHPROTO) { @@ -4138,8 +4162,7 @@ CASE(JSOP_FUNWITHPROTO) /* Load the specified function object literal. */ ReservedRooted fun(&rootFunction0, script->getFunction(GET_UINT32_INDEX(REGS.pc))); - JSObject* obj = CloneFunctionObjectIfNotSingleton(cx, fun, REGS.fp()->environmentChain(), - proto, GenericObject); + JSObject* obj = FunWithProtoOperation(cx, fun, REGS.fp()->environmentChain(), proto); if (!obj) goto error; @@ -4149,9 +4172,7 @@ END_CASE(JSOP_FUNWITHPROTO) CASE(JSOP_OBJWITHPROTO) { - ReservedRooted proto(&rootObject0, REGS.sp[-1].toObjectOrNull()); - - JSObject* obj = NewObjectWithGivenProto(cx, proto); + JSObject* obj = ObjectWithProtoOperation(cx, REGS.stackHandleAt(-1)); if (!obj) goto error; diff --git a/js/src/vm/Interpreter.h b/js/src/vm/Interpreter.h index a5860bf947cd..7ce92ddb4411 100644 --- a/js/src/vm/Interpreter.h +++ b/js/src/vm/Interpreter.h @@ -577,6 +577,15 @@ DefaultClassConstructor(JSContext* cx, unsigned argc, Value* vp); bool Debug_CheckSelfHosted(JSContext* cx, HandleValue v); +bool +CheckClassHeritageOperation(JSContext* cx, HandleValue heritage); + +JSObject* +ObjectWithProtoOperation(JSContext* cx, HandleValue proto); + +JSObject* +FunWithProtoOperation(JSContext* cx, HandleFunction fun, HandleObject parent, HandleObject proto); + } /* namespace js */ #endif /* vm_Interpreter_h */ diff --git a/js/src/vm/Opcodes.h b/js/src/vm/Opcodes.h index ab0c78613719..8e8b18364ee2 100644 --- a/js/src/vm/Opcodes.h +++ b/js/src/vm/Opcodes.h @@ -491,14 +491,13 @@ */ \ macro(JSOP_STRICTSPREADEVAL, 50, "strict-spreadeval", NULL, 1, 3, 1, JOF_BYTE|JOF_INVOKE|JOF_TYPESET|JOF_CHECKSTRICT) \ /* - * Writes the [[Prototype]] objects for both a class and its .prototype to - * the stack, given the result of a heritage expression. + * Ensures the result of a class's heritage expression is either null or a constructor. * Category: Literals * Type: Object * Operands: - * Stack: heritage => funcProto, objProto + * Stack: heritage => heritage */ \ - macro(JSOP_CLASSHERITAGE, 51, "classheritage", NULL, 1, 1, 2, JOF_BYTE) \ + macro(JSOP_CHECKCLASSHERITAGE, 51, "checkclassheritage", NULL, 1, 1, 1, JOF_BYTE) \ /* * Pushes a clone of a function with a given [[Prototype]] onto the stack. * Category: Statements @@ -2261,7 +2260,15 @@ * Stack: => */ \ macro(JSOP_TRY_DESTRUCTURING_ITERCLOSE, 220, "try-destructuring-iterclose", NULL, 1, 0, 0, JOF_BYTE) \ - macro(JSOP_UNUSED221, 221,"unused221", NULL, 1, 0, 0, JOF_BYTE) \ + \ + /* + * Pushes the current global's builtin prototype for a given proto key + * Category: Literals + * Type: Constants + * Operands: uint8_t kind + * Stack: => %BuiltinPrototype% + */ \ + macro(JSOP_BUILTINPROTO, 221, "builtinproto", NULL, 2, 0, 1, JOF_UINT8) \ macro(JSOP_UNUSED222, 222,"unused222", NULL, 1, 0, 0, JOF_BYTE) \ macro(JSOP_UNUSED223, 223,"unused223", NULL, 1, 0, 0, JOF_BYTE) \ \