Make createDynamicFunction use NewTarget.prototype for Function parent

Summary:
The old code was wrong in two ways: it was conditioned on this, not
NewTarget, and used getParent, not prototype property.

The fix is not obvious, because we need to avoid getting the prototype
property twice.  We condition on NewTarget, but instead of getting the
prototype property, get the parent of 'this', which has already had
its parent installed with the correct object.  'this' itself continues
to be otherwise ignored, as the Function ctor does not formally
reference it.

Reviewed By: avp

Differential Revision: D23834088

fbshipit-source-id: 057ce6f7a1b71220228a88a8160b82ba93c40baf

lib/VM/JSLib/JSLibInternal.cpp

@@ -327,10 +327,13 @@ CallResult<HermesValue> createDynamicFunction(
   // If at least two arguments to the function (3 in total), there's a comma.
   SafeUInt32 size{paramCount > 0 ? paramCount - 1 : 0};
 
-  // Use the parent of the 'this' value passed by the caller as the parent.
-  // This will usually be the functionPrototype or generatorFunctionPrototype,
-  // but if this is called from reflectConstruct, it might be something else.
-  Handle<JSObject> parent = vmisa<JSObject>(args.getThisArg())
+  // es2020 19.2.1.1.1 Runtime Semantics: CreateDynamicFunction: If
+  // NewTarget is given, such as with Reflect.construct, use
+  // NewParent.prototype as the parent.  The prototype on NewParent
+  // has already been looked up to use as the parent of 'this', so
+  // instead of looking it up again, just use this's parent.  If
+  // NewTarget isn't given, fall back to a default.
+  Handle<JSObject> parent = !args.getNewTarget().isUndefined()
       ? runtime->makeHandle(
             vmcast<JSObject>(args.getThisArg())->getParent(runtime))
       : (isGeneratorFunction

test/hermes/function-constructor.js

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-// RUN: %hermes -O -Wno-direct-eval %s | %FileCheck --match-full-lines %s
+// RUN: %hermes -Xes6-proxy -O -Wno-direct-eval %s | %FileCheck --match-full-lines %s
 "use strict";
 
 print('Function');
@@ -152,3 +152,30 @@ print(Function('x', 'return x // comment')(1));
 // Hashbang comments are not supported in function bodies
 try {Function('#! comment')} catch (e) {print('caught', e.name, e.message)}
 // CHECK-NEXT: caught SyntaxError {{.*}}
+
+// There was a bug where Function's ctor would set __proto__ of the result
+// to this's parent, instead of NewTarget's prototype property.  Make sure
+// no version of this still happens.
+// Check for this's parent not being used as __proto__
+print(Reflect.apply(Function, Object.create(Date.prototype), []).__proto__ !==
+      Date.prototype);
+// CHECK-NEXT: true
+
+// Check prototype of parent, not __proto__ of parent, is used.
+print(Reflect.construct(Function, [], Date).__proto__ ===
+      Date.prototype);
+// CHECK-NEXT: true
+
+// Check invalid prototype is not used.
+var F = function() {}
+F.prototype = 17;
+print(Reflect.construct(Function, [], F).__proto__ === Function.prototype);
+// CHECK-NEXT: true
+
+// Check that this's prototype and __proto__ are not referenced
+Reflect.apply(Function, {prototype() { throw new Error("die"); }}, []);
+Reflect.apply(Function,
+              new Proxy({}, {
+                  getPrototypeOf() { throw new Error("getPrototypeOf"); },
+                  get() { throw new Error("die"); }}),
+              [])