[turbofan] Properly check new.target parameter in inlined Reflect.construct.

The ConstructFunctionForwardVarargs and ConstructForwardVarargs
builtins, which are used when inlining the Reflect.construct
builtin into TurboFan optimized code, didn't properly check the
new.target parameter whether it's a constructor.

Bug: chromium:752481
Change-Id: I9b8f8c429d6eaed0ff8d27fc3f6b52eb906766a2
Reviewed-on: https://chromium-review.googlesource.com/604187
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47206}

src/builtins/arm/builtins-arm.cc

@@ -2098,6 +2098,7 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
 
 // static
 void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
+                                                      CallOrConstructMode mode,
                                                       Handle<Code> code) {
   // ----------- S t a t e -------------
   //  -- r0 : the number of arguments (not including the receiver)
@@ -2108,6 +2109,24 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
 
   Register scratch = r6;
 
+  // Check if new.target has a [[Construct]] internal method.
+  if (mode == CallOrConstructMode::kConstruct) {
+    Label new_target_constructor, new_target_not_constructor;
+    __ JumpIfSmi(r3, &new_target_not_constructor);
+    __ ldr(scratch, FieldMemOperand(r3, HeapObject::kMapOffset));
+    __ ldrb(scratch, FieldMemOperand(scratch, Map::kBitFieldOffset));
+    __ tst(scratch, Operand(1 << Map::kIsConstructor));
+    __ b(ne, &new_target_constructor);
+    __ bind(&new_target_not_constructor);
+    {
+      FrameScope scope(masm, StackFrame::MANUAL);
+      __ EnterFrame(StackFrame::INTERNAL);
+      __ Push(r3);
+      __ CallRuntime(Runtime::kThrowNotConstructor);
+    }
+    __ bind(&new_target_constructor);
+  }
+
   // Check if we have an arguments adaptor frame below the function frame.
   Label arguments_adaptor, arguments_done;
   __ ldr(r4, MemOperand(fp, StandardFrameConstants::kCallerFPOffset));

src/builtins/arm64/builtins-arm64.cc

@@ -2200,6 +2200,7 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
 
 // static
 void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
+                                                      CallOrConstructMode mode,
                                                       Handle<Code> code) {
   // ----------- S t a t e -------------
   //  -- x0 : the number of arguments (not including the receiver)
@@ -2208,6 +2209,24 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   //  -- x2 : start index (to support rest parameters)
   // -----------------------------------
 
+  // Check if new.target has a [[Construct]] internal method.
+  if (mode == CallOrConstructMode::kConstruct) {
+    Label new_target_constructor, new_target_not_constructor;
+    __ JumpIfSmi(x3, &new_target_not_constructor);
+    __ Ldr(x5, FieldMemOperand(x3, HeapObject::kMapOffset));
+    __ Ldrb(x5, FieldMemOperand(x5, Map::kBitFieldOffset));
+    __ TestAndBranchIfAnySet(x5, 1 << Map::kIsConstructor,
+                             &new_target_constructor);
+    __ Bind(&new_target_not_constructor);
+    {
+      FrameScope scope(masm, StackFrame::MANUAL);
+      __ EnterFrame(StackFrame::INTERNAL);
+      __ Push(x3);
+      __ CallRuntime(Runtime::kThrowNotConstructor);
+    }
+    __ Bind(&new_target_constructor);
+  }
+
   // Check if we have an arguments adaptor frame below the function frame.
   Label arguments_adaptor, arguments_done;
   __ Ldr(x5, MemOperand(fp, StandardFrameConstants::kCallerFPOffset));

src/builtins/builtins-call-gen.cc

@@ -49,13 +49,14 @@ void Builtins::Generate_CallVarargs(MacroAssembler* masm) {
 }
 
 void Builtins::Generate_CallForwardVarargs(MacroAssembler* masm) {
-  Generate_CallOrConstructForwardVarargs(masm,
+  Generate_CallOrConstructForwardVarargs(masm, CallOrConstructMode::kCall,
                                          masm->isolate()->builtins()->Call());
 }
 
 void Builtins::Generate_CallFunctionForwardVarargs(MacroAssembler* masm) {
   Generate_CallOrConstructForwardVarargs(
-      masm, masm->isolate()->builtins()->CallFunction());
+      masm, CallOrConstructMode::kCall,
+      masm->isolate()->builtins()->CallFunction());
 }
 
 void CallOrConstructBuiltinsAssembler::CallOrConstructWithArrayLike(

src/builtins/builtins-constructor-gen.cc

@@ -25,12 +25,14 @@ void Builtins::Generate_ConstructVarargs(MacroAssembler* masm) {
 
 void Builtins::Generate_ConstructForwardVarargs(MacroAssembler* masm) {
   Generate_CallOrConstructForwardVarargs(
-      masm, BUILTIN_CODE(masm->isolate(), Construct));
+      masm, CallOrConstructMode::kConstruct,
+      BUILTIN_CODE(masm->isolate(), Construct));
 }
 
 void Builtins::Generate_ConstructFunctionForwardVarargs(MacroAssembler* masm) {
   Generate_CallOrConstructForwardVarargs(
-      masm, BUILTIN_CODE(masm->isolate(), ConstructFunction));
+      masm, CallOrConstructMode::kConstruct,
+      BUILTIN_CODE(masm->isolate(), ConstructFunction));
 }
 
 TF_BUILTIN(ConstructWithArrayLike, CallOrConstructBuiltinsAssembler) {

src/builtins/builtins.h

@@ -126,9 +126,11 @@ class Builtins {
 
   static void Generate_Call(MacroAssembler* masm, ConvertReceiverMode mode);
 
+  enum class CallOrConstructMode { kCall, kConstruct };
   static void Generate_CallOrConstructVarargs(MacroAssembler* masm,
                                               Handle<Code> code);
   static void Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
+                                                     CallOrConstructMode mode,
                                                      Handle<Code> code);
 
   static void Generate_InterpreterPushArgsThenCallImpl(

src/builtins/ia32/builtins-ia32.cc

@@ -2238,6 +2238,7 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
 
 // static
 void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
+                                                      CallOrConstructMode mode,
                                                       Handle<Code> code) {
   // ----------- S t a t e -------------
   //  -- eax : the number of arguments (not including the receiver)
@@ -2246,6 +2247,24 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   //  -- ecx : start index (to support rest parameters)
   // -----------------------------------
 
+  // Check if new.target has a [[Construct]] internal method.
+  if (mode == CallOrConstructMode::kConstruct) {
+    Label new_target_constructor, new_target_not_constructor;
+    __ JumpIfSmi(edx, &new_target_not_constructor, Label::kNear);
+    __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset));
+    __ test_b(FieldOperand(ebx, Map::kBitFieldOffset),
+              Immediate(1 << Map::kIsConstructor));
+    __ j(not_zero, &new_target_constructor, Label::kNear);
+    __ bind(&new_target_not_constructor);
+    {
+      FrameScope scope(masm, StackFrame::MANUAL);
+      __ EnterFrame(StackFrame::INTERNAL);
+      __ Push(edx);
+      __ CallRuntime(Runtime::kThrowNotConstructor);
+    }
+    __ bind(&new_target_constructor);
+  }
+
   // Preserve new.target (in case of [[Construct]]).
   __ movd(xmm0, edx);
 

src/builtins/mips/builtins-mips.cc

@@ -2095,6 +2095,7 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
 
 // static
 void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
+                                                      CallOrConstructMode mode,
                                                       Handle<Code> code) {
   // ----------- S t a t e -------------
   //  -- a0 : the number of arguments (not including the receiver)
@@ -2103,6 +2104,24 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   //  -- a2 : start index (to support rest parameters)
   // -----------------------------------
 
+  // Check if new.target has a [[Construct]] internal method.
+  if (mode == CallOrConstructMode::kConstruct) {
+    Label new_target_constructor, new_target_not_constructor;
+    __ JumpIfSmi(a3, &new_target_not_constructor);
+    __ lw(t1, FieldMemOperand(a3, HeapObject::kMapOffset));
+    __ lbu(t1, FieldMemOperand(t1, Map::kBitFieldOffset));
+    __ And(t1, t1, Operand(1 << Map::kIsConstructor));
+    __ Branch(&new_target_constructor, ne, t1, Operand(zero_reg));
+    __ bind(&new_target_not_constructor);
+    {
+      FrameScope scope(masm, StackFrame::MANUAL);
+      __ EnterFrame(StackFrame::INTERNAL);
+      __ Push(a3);
+      __ CallRuntime(Runtime::kThrowNotConstructor);
+    }
+    __ bind(&new_target_constructor);
+  }
+
   // Check if we have an arguments adaptor frame below the function frame.
   Label arguments_adaptor, arguments_done;
   __ lw(t3, MemOperand(fp, StandardFrameConstants::kCallerFPOffset));

src/builtins/mips64/builtins-mips64.cc

@@ -2125,6 +2125,7 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
 
 // static
 void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
+                                                      CallOrConstructMode mode,
                                                       Handle<Code> code) {
   // ----------- S t a t e -------------
   //  -- a0 : the number of arguments (not including the receiver)
@@ -2133,6 +2134,24 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   //  -- a2 : start index (to support rest parameters)
   // -----------------------------------
 
+  // Check if new.target has a [[Construct]] internal method.
+  if (mode == CallOrConstructMode::kConstruct) {
+    Label new_target_constructor, new_target_not_constructor;
+    __ JumpIfSmi(a3, &new_target_not_constructor);
+    __ ld(t1, FieldMemOperand(a3, HeapObject::kMapOffset));
+    __ lbu(t1, FieldMemOperand(t1, Map::kBitFieldOffset));
+    __ And(t1, t1, Operand(1 << Map::kIsConstructor));
+    __ Branch(&new_target_constructor, ne, t1, Operand(zero_reg));
+    __ bind(&new_target_not_constructor);
+    {
+      FrameScope scope(masm, StackFrame::MANUAL);
+      __ EnterFrame(StackFrame::INTERNAL);
+      __ Push(a3);
+      __ CallRuntime(Runtime::kThrowNotConstructor);
+    }
+    __ bind(&new_target_constructor);
+  }
+
   // Check if we have an arguments adaptor frame below the function frame.
   Label arguments_adaptor, arguments_done;
   __ Ld(a6, MemOperand(fp, StandardFrameConstants::kCallerFPOffset));

src/builtins/x64/builtins-x64.cc

@@ -2343,6 +2343,7 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
 
 // static
 void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
+                                                      CallOrConstructMode mode,
                                                       Handle<Code> code) {
   // ----------- S t a t e -------------
   //  -- rax : the number of arguments (not including the receiver)
@@ -2351,6 +2352,24 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   //  -- rcx : start index (to support rest parameters)
   // -----------------------------------
 
+  // Check if new.target has a [[Construct]] internal method.
+  if (mode == CallOrConstructMode::kConstruct) {
+    Label new_target_constructor, new_target_not_constructor;
+    __ JumpIfSmi(rdx, &new_target_not_constructor, Label::kNear);
+    __ movp(rbx, FieldOperand(rdx, HeapObject::kMapOffset));
+    __ testb(FieldOperand(rbx, Map::kBitFieldOffset),
+             Immediate(1 << Map::kIsConstructor));
+    __ j(not_zero, &new_target_constructor, Label::kNear);
+    __ bind(&new_target_not_constructor);
+    {
+      FrameScope scope(masm, StackFrame::MANUAL);
+      __ EnterFrame(StackFrame::INTERNAL);
+      __ Push(rdx);
+      __ CallRuntime(Runtime::kThrowNotConstructor);
+    }
+    __ bind(&new_target_constructor);
+  }
+
   // Check if we have an arguments adaptor frame below the function frame.
   Label arguments_adaptor, arguments_done;
   __ movp(rbx, Operand(rbp, StandardFrameConstants::kCallerFPOffset));

test/mjsunit/regress/regress-crbug-752481.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+const A = class A {}
+
+function test(foo) {
+  assertThrows(foo);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+}
+
+// Test combinations of rest parameters and primitive new.targets
+test((...args) => Reflect.construct(A, args, 0));
+test((...args) => Reflect.construct(A, args, true));
+test((...args) => Reflect.construct(A, args, false));
+test((...args) => Reflect.construct(A, args, ""));
+test((...args) => Reflect.construct(A, args, null));
+test((...args) => Reflect.construct(A, args, undefined));
+test((...args) => Reflect.construct(A, args, Symbol.species));
+
+// Test combinations of arguments object and primitive new.targets.
+test(function() { Reflect.construct(A, arguments, 0); });
+test(function() { Reflect.construct(A, arguments, true); });
+test(function() { Reflect.construct(A, arguments, false); });
+test(function() { Reflect.construct(A, arguments, ""); });
+test(function() { Reflect.construct(A, arguments, null); });
+test(function() { Reflect.construct(A, arguments, undefined); });
+test(function() { Reflect.construct(A, arguments, Symbol.species); });