Bug 1169731 - [[Call]] on a class constructor should throw. (r=jandem)
authorEric Faust <efaustbmo@mozilla.com>
Wed, 17 Jun 2015 14:37:49 -0700
changeset 249477 f11b7896950f6a708fd295ce21314652cb6e07e2
parent 249476 cfaa6fb7d84edb86c3a940fc645a94b3fcb283f5
child 249478 5c5ab792827a29fb604db02e5e915321310d47d5
push id28927
push usercbook@mozilla.com
push dateThu, 18 Jun 2015 13:13:33 +0000
treeherdermozilla-central@efe86609e776 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1169731
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1169731 - [[Call]] on a class constructor should throw. (r=jandem)
js/public/TrackedOptimizationInfo.h
js/src/jit-test/tests/baseline/classConstructor-AnyScripted.js
js/src/jit/BaselineIC.cpp
js/src/jit/CodeGenerator.cpp
js/src/jit/IonBuilder.cpp
js/src/jit/Lowering.cpp
js/src/jit/MacroAssembler.h
js/src/js.msg
js/src/tests/ecma_6/Class/classConstructorNoCall.js
js/src/tests/ecma_6/Class/methodInstallation.js
js/src/vm/Interpreter.cpp
js/src/vm/Xdr.h
--- a/js/public/TrackedOptimizationInfo.h
+++ b/js/public/TrackedOptimizationInfo.h
@@ -103,16 +103,17 @@ namespace JS {
     _(UnknownSimdProperty)                                              \
                                                                         \
     _(CantInlineGeneric)                                                \
     _(CantInlineNoTarget)                                               \
     _(CantInlineNotInterpreted)                                         \
     _(CantInlineNoBaseline)                                             \
     _(CantInlineLazy)                                                   \
     _(CantInlineNotConstructor)                                         \
+    _(CantInlineClassConstructor)                                       \
     _(CantInlineDisabledIon)                                            \
     _(CantInlineTooManyArgs)                                            \
     _(CantInlineRecursive)                                              \
     _(CantInlineHeavyweight)                                            \
     _(CantInlineNeedsArgsObj)                                           \
     _(CantInlineDebuggee)                                               \
     _(CantInlineUnknownProps)                                           \
     _(CantInlineExceededDepth)                                          \
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/baseline/classConstructor-AnyScripted.js
@@ -0,0 +1,30 @@
+load(libdir + "class.js");
+
+var test = `
+
+function test(fun) {
+    fun();
+}
+
+// Generate a CallAnyScripted stub in test()
+for (let i = 0; i < 20; i++) {
+    test(function() { /* wheeee */ });
+}
+
+class foo {
+    constructor() { }
+}
+
+// Compile foo()
+for (let i = 0; i < 11; i++)
+    new foo();
+
+try {
+    test(foo);
+    throw new Error("Invoking a class constructor without new must throw");
+} catch (e if e instanceof TypeError) { }
+
+`;
+
+if (classesEnabled())
+    eval(test);
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -9555,16 +9555,20 @@ TryAttachCallStub(JSContext* cx, ICCall_
         // MagicArguments may escape the frame through them.
         if (op == JSOP_FUNAPPLY)
             return true;
 
         // If callee is not an interpreted constructor, we have to throw.
         if (constructing && !fun->isConstructor())
             return true;
 
+        // Likewise, if the callee is a class constructor, we have to throw.
+        if (!constructing && fun->isClassConstructor())
+            return true;
+
         if (!fun->hasJITCode()) {
             // Don't treat this as an unoptimizable case, as we'll add a stub
             // when the callee becomes hot.
             *handled = true;
             return true;
         }
 
         // Check if this stub chain has already generalized scripted calls.
@@ -10413,20 +10417,23 @@ ICCallScriptedCompiler::generateStubCode
         masm.branchPtr(Assembler::NotEqual, expectedCallee, callee, &failure);
 
         // Guard against relazification.
         masm.branchIfFunctionHasNoScript(callee, &failure);
     } else {
         // Ensure the object is a function.
         masm.branchTestObjClass(Assembler::NotEqual, callee, regs.getAny(), &JSFunction::class_,
                                 &failure);
-        if (isConstructing_)
+        if (isConstructing_) {
             masm.branchIfNotInterpretedConstructor(callee, regs.getAny(), &failure);
-        else
+        } else {
             masm.branchIfFunctionHasNoScript(callee, &failure);
+            masm.branchFunctionKind(Assembler::Equal, JSFunction::ClassConstructor, callee,
+                                    regs.getAny(), &failure);
+        }
     }
 
     // Load the JSScript.
     masm.loadPtr(Address(callee, JSFunction::offsetOfNativeOrScript()), callee);
 
     // Load the start of the target JitCode.
     Register code;
     if (!isConstructing_) {
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -3023,20 +3023,22 @@ CodeGenerator::visitCallGeneric(LCallGen
     masm.checkStackAlignment();
 
     // Guard that calleereg is actually a function object.
     masm.loadObjClass(calleereg, nargsreg);
     masm.branchPtr(Assembler::NotEqual, nargsreg, ImmPtr(&JSFunction::class_), &invoke);
 
     // Guard that calleereg is an interpreted function with a JSScript.
     // If we are constructing, also ensure the callee is a constructor.
-    if (call->mir()->isConstructing())
+    if (call->mir()->isConstructing()) {
         masm.branchIfNotInterpretedConstructor(calleereg, nargsreg, &invoke);
-    else
+    } else {
         masm.branchIfFunctionHasNoScript(calleereg, &invoke);
+        masm.branchFunctionKind(Assembler::Equal, JSFunction::ClassConstructor, calleereg, objreg, &invoke);
+    }
 
     // Knowing that calleereg is a non-native function, load the JSScript.
     masm.loadPtr(Address(calleereg, JSFunction::offsetOfNativeOrScript()), objreg);
 
     // Load script jitcode.
     masm.loadBaselineOrIonRaw(objreg, objreg, &invoke);
 
     // Nestle the StackPointer up to the argument vector.
@@ -3120,16 +3122,17 @@ CodeGenerator::visitCallKnown(LCallKnown
     Register calleereg = ToRegister(call->getFunction());
     Register objreg    = ToRegister(call->getTempObject());
     uint32_t unusedStack = StackOffsetOfPassedArg(call->argslot());
     JSFunction* target = call->getSingleTarget();
     Label end, uncompiled;
 
     // Native single targets are handled by LCallNative.
     MOZ_ASSERT(!target->isNative());
+    MOZ_ASSERT_IF(target->isClassConstructor(), call->isConstructing());
     // Missing arguments must have been explicitly appended by the IonBuilder.
     DebugOnly<unsigned> numNonArgsOnStack = 1 + call->isConstructing();
     MOZ_ASSERT(target->nargs() <= call->mir()->numStackArgs() - numNonArgsOnStack);
 
     MOZ_ASSERT_IF(call->mir()->isConstructing(), target->isConstructor());
 
     masm.checkStackAlignment();
 
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -433,16 +433,21 @@ IonBuilder::canInlineTarget(JSFunction* 
     }
 
     JSScript* inlineScript = target->nonLazyScript();
     if (callInfo.constructing() && !target->isConstructor()) {
         trackOptimizationOutcome(TrackedOutcome::CantInlineNotConstructor);
         return DontInline(inlineScript, "Callee is not a constructor");
     }
 
+    if (!callInfo.constructing() && target->isClassConstructor()) {
+        trackOptimizationOutcome(TrackedOutcome::CantInlineClassConstructor);
+        return DontInline(inlineScript, "Not constructing class constructor");
+    }
+
     AnalysisMode analysisMode = info().analysisMode();
     if (!CanIonCompile(inlineScript, analysisMode)) {
         trackOptimizationOutcome(TrackedOutcome::CantInlineDisabledIon);
         return DontInline(inlineScript, "Disabled Ion compilation");
     }
 
     // Don't inline functions which don't have baseline scripts.
     if (!inlineScript->hasBaselineScript()) {
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -451,17 +451,17 @@ LIRGenerator::visitCall(MCall* call)
         Register cxReg, objReg, privReg, argsReg;
         GetTempRegForIntArg(0, 0, &cxReg);
         GetTempRegForIntArg(1, 0, &objReg);
         GetTempRegForIntArg(2, 0, &privReg);
         mozilla::DebugOnly<bool> ok = GetTempRegForIntArg(3, 0, &argsReg);
         MOZ_ASSERT(ok, "How can we not have four temp registers?");
         lir = new(alloc()) LCallDOMNative(tempFixed(cxReg), tempFixed(objReg),
                                           tempFixed(privReg), tempFixed(argsReg));
-    } else if (target) {
+    } else if (target && !(target->isClassConstructor() && !call->isConstructing())) {
         // Call known functions.
         if (target->isNative()) {
             Register cxReg, numReg, vpReg, tmpReg;
             GetTempRegForIntArg(0, 0, &cxReg);
             GetTempRegForIntArg(1, 0, &numReg);
             GetTempRegForIntArg(2, 0, &vpReg);
 
             // Even though this is just a temp reg, use the same API to avoid
--- a/js/src/jit/MacroAssembler.h
+++ b/js/src/jit/MacroAssembler.h
@@ -936,16 +936,25 @@ class MacroAssembler : public MacroAssem
     }
 
     void branchTestObjectIsProxy(bool proxy, Register object, Register scratch, Label* label)
     {
         loadObjClass(object, scratch);
         branchTestClassIsProxy(proxy, scratch, label);
     }
 
+    void branchFunctionKind(Condition cond, JSFunction::FunctionKind kind, Register fun,
+                            Register scratch, Label* label)
+    {
+        Address flags(fun, JSFunction::offsetOfFlags());
+        load32(flags, scratch);
+        and32(Imm32(JSFunction::FUNCTION_KIND_MASK), scratch);
+        branch32(cond, scratch, Imm32(kind << JSFunction::FUNCTION_KIND_SHIFT), label);
+    }
+
   public:
 #ifndef JS_CODEGEN_ARM64
     // StackPointer manipulation functions.
     // On ARM64, the StackPointer is implemented as two synchronized registers.
     // Code shared across platforms must use these functions to be valid.
     template <typename T>
     void addToStackPtr(T t) { addPtr(t, getStackPointer()); }
     template <typename T>
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -97,16 +97,17 @@ MSG_DEF(JSMSG_NOT_ITERABLE,            1
 MSG_DEF(JSMSG_ALREADY_HAS_PRAGMA,      2, JSEXN_NONE, "{0} is being assigned a {1}, but already has one")
 MSG_DEF(JSMSG_NEXT_RETURNED_PRIMITIVE, 0, JSEXN_TYPEERR, "iterator.next() returned a non-object value")
 MSG_DEF(JSMSG_CANT_SET_PROTO,          0, JSEXN_TYPEERR, "can't set prototype of this object")
 MSG_DEF(JSMSG_CANT_SET_PROTO_OF,       1, JSEXN_TYPEERR, "can't set prototype of {0}")
 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|")
 
 // JSON
 MSG_DEF(JSMSG_JSON_BAD_PARSE,          3, JSEXN_SYNTAXERR, "JSON.parse: {0} at line {1} column {2} of the JSON data")
 MSG_DEF(JSMSG_JSON_CYCLIC_VALUE,       1, JSEXN_TYPEERR, "cyclic {0} value")
 
 // Runtime errors
 MSG_DEF(JSMSG_BAD_INSTANCEOF_RHS,      1, JSEXN_TYPEERR, "invalid 'instanceof' operand {0}")
 MSG_DEF(JSMSG_BAD_LEFTSIDE_OF_ASS,     0, JSEXN_REFERENCEERR, "invalid assignment left-hand side")
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Class/classConstructorNoCall.js
@@ -0,0 +1,28 @@
+// Class constructors don't have a [[Call]]
+var test = `
+
+class Foo {
+    constructor() { }
+}
+
+assertThrowsInstanceOf(Foo, TypeError);
+
+class Bar extends Foo {
+    constructor() { }
+}
+
+assertThrowsInstanceOf(Bar, TypeError);
+
+assertThrowsInstanceOf(class { constructor() { } }, TypeError);
+assertThrowsInstanceOf(class extends Foo { constructor() { } }, TypeError);
+
+assertThrowsInstanceOf(class foo { constructor() { } }, TypeError);
+assertThrowsInstanceOf(class foo extends Foo { constructor() { } }, TypeError);
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");
--- a/js/src/tests/ecma_6/Class/methodInstallation.js
+++ b/js/src/tests/ecma_6/Class/methodInstallation.js
@@ -39,17 +39,17 @@ for (let a of [testClass,
     staticMethodCalled = false;
     staticGetterCalled = false;
     staticSetterCalled = false;
 
     var aConstDesc = Object.getOwnPropertyDescriptor(a.prototype, \"constructor\");
     assertEq(aConstDesc.writable, true);
     assertEq(aConstDesc.configurable, true);
     assertEq(aConstDesc.enumerable, false);
-    aConstDesc.value();
+    new aConstDesc.value();
     assertEq(constructorCalled, true);
 
     // __proto__ is just an identifier for classes. No prototype changes are made.
     assertEq(Object.getPrototypeOf(a.prototype), Object.prototype);
     var aMethDesc = Object.getOwnPropertyDescriptor(a.prototype, \"__proto__\");
     assertEq(aMethDesc.writable, true);
     assertEq(aMethDesc.configurable, true);
     assertEq(aMethDesc.enumerable, true);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -694,16 +694,21 @@ js::Invoke(JSContext* cx, CallArgs args,
         JSNative call = args.callee().callHook();
         if (!call)
             return ReportIsNotFunction(cx, args.calleev(), skipForCallee, construct);
         return CallJSNative(cx, call, args);
     }
 
     /* Invoke native functions. */
     JSFunction* fun = &args.callee().as<JSFunction>();
+    if (construct != CONSTRUCT && fun->isClassConstructor()) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_CALL_CLASS_CONSTRUCTOR);
+        return false;
+    }
+
     if (fun->isNative()) {
         MOZ_ASSERT_IF(construct, !fun->isConstructor());
         return CallJSNative(cx, fun->native(), args);
     }
 
     if (!fun->getOrCreateScript(cx))
         return false;
 
@@ -2942,17 +2947,19 @@ CASE(JSOP_FUNCALL)
 
     MOZ_ASSERT(REGS.stackDepth() >= 2u + GET_ARGC(REGS.pc));
     CallArgs args = CallArgsFromSp(argStackSlots, REGS.sp, construct);
 
     JSFunction* maybeFun;
     bool isFunction = IsFunctionObject(args.calleev(), &maybeFun);
 
     /* Don't bother trying to fast-path calls to scripted non-constructors. */
-    if (!isFunction || !maybeFun->isInterpreted() || !maybeFun->isConstructor()) {
+    if (!isFunction || !maybeFun->isInterpreted() || !maybeFun->isConstructor() ||
+        (!construct && maybeFun->isClassConstructor()))
+    {
         if (construct) {
             if (!InvokeConstructor(cx, args))
                 goto error;
         } else {
             if (!Invoke(cx, args))
                 goto error;
         }
         Value* newsp = args.spAfterCall();
--- a/js/src/vm/Xdr.h
+++ b/js/src/vm/Xdr.h
@@ -24,21 +24,21 @@ namespace js {
  * versions.  If deserialization fails, the data should be invalidated if
  * possible.
  *
  * When you change this, run make_opcode_doc.py and copy the new output into
  * this wiki page:
  *
  *  https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/Bytecode
  */
-static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 292;
+static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 293;
 static const uint32_t XDR_BYTECODE_VERSION =
     uint32_t(0xb973c0de - XDR_BYTECODE_VERSION_SUBTRAHEND);
 
-static_assert(JSErr_Limit == 401,
+static_assert(JSErr_Limit == 402,
               "GREETINGS, POTENTIAL SUBTRAHEND INCREMENTER! If you added or "
               "removed MSG_DEFs from js.msg, you should increment "
               "XDR_BYTECODE_VERSION_SUBTRAHEND and update this assertion's "
               "expected JSErr_Limit value.");
 
 class XDRBuffer {
   public:
     explicit XDRBuffer(JSContext* cx)