From addd6b8f23366c645a902fd8d38e4ab08507e965 Mon Sep 17 00:00:00 2001 From: Marc Horowitz Date: Sat, 26 Sep 2020 11:25:14 -0700 Subject: [PATCH] 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 | 11 +++++++---- test/hermes/function-constructor.js | 29 ++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/lib/VM/JSLib/JSLibInternal.cpp b/lib/VM/JSLib/JSLibInternal.cpp index 2e920158cd1..1077a1759c5 100644 --- a/lib/VM/JSLib/JSLibInternal.cpp +++ b/lib/VM/JSLib/JSLibInternal.cpp @@ -327,10 +327,13 @@ CallResult 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 parent = vmisa(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 parent = !args.getNewTarget().isUndefined() ? runtime->makeHandle( vmcast(args.getThisArg())->getParent(runtime)) : (isGeneratorFunction diff --git a/test/hermes/function-constructor.js b/test/hermes/function-constructor.js index 595640e4662..219d26bc293 100644 --- a/test/hermes/function-constructor.js +++ b/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"); }}), + [])