Changeset 286069 in webkit

Timestamp:

Nov 19, 2021, 11:17:04 AM (5 years ago)

Author:

commit-queue@webkit.org

Message:

[JSC] Shadow realms: set correct Function prototype on wrapped functions
​https://bugs.webkit.org/show_bug.cgi?id=233143

Patch by Joseph Griego <​jgriego@igalia.com> on 2021-11-19
Reviewed by Yusuke Suzuki.

At present, the Function prototype set on each of the returned wrapped
functions will be the Function object from the realm the shadow realm
builtin is from--to comply with the latest draft of the shadow realms
spec [1], wrapped function objects should have the Function prototype
from the realm the wrapper object is destined for, instead.

At present, this requires tracking both the calling (destination) and
target (source) realm and switching between the two as function
arguments are wrapped (when the notion of source and destination realm
also flips)

Adds a simple builtin (moveFunctionToRealm) that can switch the Function
prototype given only the Shadow Realm object corresponding to the
correct global object.

Also marks the corresponding part of test262 as passing.

JSTests:

• test262/expectations.yaml:

Source/JavaScriptCore:

[1] ​https://tc39.es/proposal-shadowrealm/ sections 2.1, 2.2

• builtins/BuiltinNames.h:
• builtins/ShadowRealmPrototype.js:

(wrapped):
(globalPrivate.wrap):
(evaluate):
(importValue):
(globalPrivate.wrap.wrapped): Deleted.

• bytecode/LinkTimeConstant.h:
• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

• runtime/ShadowRealmPrototype.cpp:

(JSC::JSC_DEFINE_HOST_FUNCTION):

• runtime/ShadowRealmPrototype.h:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/shadow-realm-evaluate.js (modified) (4 diffs)
• JSTests/test262/expectations.yaml (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/builtins/BuiltinNames.h (modified) (1 diff)
• Source/JavaScriptCore/builtins/ShadowRealmPrototype.js (modified) (4 diffs)
• Source/JavaScriptCore/bytecode/LinkTimeConstant.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSGlobalObject.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/ShadowRealmPrototype.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/ShadowRealmPrototype.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-11-19  Joseph Griego  <jgriego@igalia.com>
+
+        [JSC] Shadow realms: set correct Function prototype on wrapped functions
+        https://bugs.webkit.org/show_bug.cgi?id=233143
+
+        Reviewed by Yusuke Suzuki.
+
+        At present, the Function prototype set on each of the returned wrapped
+        functions will be the Function object from the realm the shadow realm
+        builtin is from--to comply with the latest draft of the shadow realms
+        spec [1], wrapped function objects should have the Function prototype
+        from the realm the wrapper object is destined for, instead.
+
+        At present, this requires tracking both the calling (destination) and
+        target (source) realm and switching between the two as function
+        arguments are wrapped (when the notion of source and destination realm
+        also flips)
+
+        Adds a simple builtin (moveFunctionToRealm) that can switch the Function
+        prototype given only the Shadow Realm object corresponding to the
+        correct global object.
+
+        Also marks the corresponding part of test262 as passing.
+
+        * test262/expectations.yaml:
+
 2021-11-19  Angelos Oikonomopoulos  <angelos@igalia.com>
 

trunk/JSTests/stress/shadow-realm-evaluate.js

@@ -78 +78 @@
     shouldBe(wrappedInvokeAndAdd(() => { return 1 }, () => { return 2 }), 3);
     shouldBe($.globalObjectFor(wrappedInvokeAndAdd), globalThis);
+    shouldBe(Object.getPrototypeOf(wrappedInvokeAndAdd), Function.prototype);
 
     // name and length properties from wrapped function are absent
@@ -123 +124 @@
         shouldBe($.globalObjectFor(f), globalThis);
         shouldBe(f(() => { return 41; }), 42);
+        shouldBe(Object.getPrototypeOf(f), Function.prototype);
     }
     // (potential) inlining of wrapped function uses correct global object
@@ -129 +131 @@
         shouldBe($.globalObjectFor(f), globalThis);
         shouldBe(f(() => { return 41; }), 42);
+        shouldBe(Object.getPrototypeOf(f), Function.prototype);
     }
     // (potential) inlining inside a realm uses correct global object
@@ -152 +155 @@
     shouldBe(evaluateLength.configurable, true);
 }
+
+// Enclosing realm is hidden from shaodw realm even when playing Function prototype tricks
+{
+    let realm = new ShadowRealm();
+    foo = 42;
+
+    realm.evaluate("foo = false");
+
+    let realmFn = realm.evaluate(`(f) => {
+      let ourFn = Object.getPrototypeOf(f).constructor;
+      return (new ourFn("return this"))().foo
+    }`);
+
+    let retrievedFoo = realmFn(() => {});
+    let aFunction = Object.getPrototypeOf(realmFn).constructor;
+    let anotherFoo = (new aFunction("return this"))().foo;
+
+    shouldBe(retrievedFoo, false);
+    shouldBe(anotherFoo, 42);
+}

trunk/JSTests/test262/expectations.yaml

@@ -1138 +1138 @@
   default: 'SyntaxError: Invalid regular expression: number too large in {} quantifier'
   strict mode: 'SyntaxError: Invalid regular expression: number too large in {} quantifier'
-test/built-ins/ShadowRealm/prototype/evaluate/wrapped-function-proto-from-caller-realm.js:
-  default: 'Test262Error: callable arguments passed into WrappedFunction should be wrapped in target realm Expected SameValue(«false», «true») to be true'
-  strict mode: 'Test262Error: callable arguments passed into WrappedFunction should be wrapped in target realm Expected SameValue(«false», «true») to be true'
 test/built-ins/Temporal/Instant/prototype/toString/timezone-offset.js:
   default: 'Test262Error: offset of UTC is +00:00 Expected SameValue(«1970-01-01T00:00:00Z», «1970-01-01T00:00:00+00:00») to be true'

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-11-19  Joseph Griego  <jgriego@igalia.com>
+
+        [JSC] Shadow realms: set correct Function prototype on wrapped functions
+        https://bugs.webkit.org/show_bug.cgi?id=233143
+
+        Reviewed by Yusuke Suzuki.
+
+        At present, the Function prototype set on each of the returned wrapped
+        functions will be the Function object from the realm the shadow realm
+        builtin is from--to comply with the latest draft of the shadow realms
+        spec [1], wrapped function objects should have the Function prototype
+        from the realm the wrapper object is destined for, instead.
+
+        At present, this requires tracking both the calling (destination) and
+        target (source) realm and switching between the two as function
+        arguments are wrapped (when the notion of source and destination realm
+        also flips)
+
+        Adds a simple builtin (moveFunctionToRealm) that can switch the Function
+        prototype given only the Shadow Realm object corresponding to the
+        correct global object.
+
+        Also marks the corresponding part of test262 as passing.
+
+        [1] https://tc39.es/proposal-shadowrealm/ sections 2.1, 2.2
+
+
+
+        * builtins/BuiltinNames.h:
+        * builtins/ShadowRealmPrototype.js:
+        (wrapped):
+        (globalPrivate.wrap):
+        (evaluate):
+        (importValue):
+        (globalPrivate.wrap.wrapped): Deleted.
+        * bytecode/LinkTimeConstant.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init):
+        * runtime/ShadowRealmPrototype.cpp:
+        (JSC::JSC_DEFINE_HOST_FUNCTION):
+        * runtime/ShadowRealmPrototype.h:
+
 2021-11-19  Robin Morisset  <rmorisset@apple.com>
 

trunk/Source/JavaScriptCore/builtins/BuiltinNames.h

@@ -119 +119 @@
     macro(importInRealm) \
     macro(evalInRealm) \
+    macro(moveFunctionToRealm) \
     macro(thisTimeValue) \
     macro(newTargetLocal) \

trunk/Source/JavaScriptCore/builtins/ShadowRealmPrototype.js

@@ -24 +24 @@
  */
 
+// Wrap a value at the boundary between the incubating realm and `shadowRealm`:
+// if `fromShadowRealm` is false, we are wrapping an object from the incubating
+// realm; if true, we are wrapping an object from the shadow realm
 @globalPrivate
-function wrap(target)
+function wrap(fromShadowRealm, shadowRealm, target)
 {
     "use strict";
@@ -34 +37 @@
             var wrappedArgs = @newArrayWithSize(length);
             for (var index = 0; index < length; ++index)
-                @putByValDirect(wrappedArgs, index, @wrap(arguments[index]));
+                // Note that for arguments, we flip `fromShadowRealm` since to
+                // wrap a function from realm A to work in realm B, we need to
+                // wrap the arguments (from realm B) to work in realm A before
+                // calling the wrapped function
+                @putByValDirect(wrappedArgs, index, @wrap(!fromShadowRealm, shadowRealm, arguments[index]));
 
             var result = target.@apply(@undefined, wrappedArgs);
-            return @wrap(result);
+            return @wrap(fromShadowRealm, shadowRealm, result);
         };
         delete wrapped['name'];
         delete wrapped['length'];
+
+        // Because this function (wrap) will run with the incubating realm
+        // active, we only need to fix the prototype on `wrapped` if we are
+        // moving the function from the incubating realm to the shadow realm
+        if (!fromShadowRealm)
+            @moveFunctionToRealm(wrapped, shadowRealm);
         return wrapped;
     } else if (@isObject(target)) {
@@ -59 +72 @@
 
     var result = @evalInRealm(this, sourceText)
-    return @wrap(result);
+    return @wrap(true, this, result);
 }
 
@@ -77 +90 @@
             @throwTypeError("%ShadowRealm%.importValue requires |exportName| to exist in the |specifier|");
 
-        return @wrap(lookup);
+        return @wrap(true, this, lookup);
     };
 

trunk/Source/JavaScriptCore/bytecode/LinkTimeConstant.h

@@ -81 +81 @@
     v(importInRealm, nullptr) \
     v(evalInRealm, nullptr) \
+    v(moveFunctionToRealm, nullptr) \
     v(isConstructor, nullptr) \
     v(sameValue, nullptr) \

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -1461 +1461 @@
             init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 0, String(), evalInRealm));
         });
+    m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::moveFunctionToRealm)].initLater([] (const Initializer<JSCell>& init) {
+            init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 0, String(), moveFunctionToRealm));
+        });
     m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::thisTimeValue)].initLater([] (const Initializer<JSCell>& init) {
             init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 0, String(), dateProtoFuncGetTime, DatePrototypeGetTimeIntrinsic));

trunk/Source/JavaScriptCore/runtime/ShadowRealmPrototype.cpp

@@ -125 +125 @@
 }
 
+JSC_DEFINE_HOST_FUNCTION(moveFunctionToRealm, (JSGlobalObject* globalObject, CallFrame* callFrame))
+{
+    VM& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    JSValue wrappedFnArg = callFrame->argument(0);
+    JSFunction* wrappedFn = jsDynamicCast<JSFunction*>(vm, wrappedFnArg);
+    JSValue targetRealmArg = callFrame->argument(1);
+    ShadowRealmObject* targetRealm = jsDynamicCast<ShadowRealmObject*>(vm, targetRealmArg);
+    ASSERT(targetRealm);
+    RETURN_IF_EXCEPTION(scope, { });
+
+    bool isBuiltin = false;
+    JSGlobalObject* targetGlobalObj = targetRealm->globalObject();
+    wrappedFn->setPrototype(vm, targetGlobalObj, targetGlobalObj->strictFunctionStructure(isBuiltin)->storedPrototype());
+    RELEASE_AND_RETURN(scope, JSValue::encode(jsUndefined()));
+}
+
 } // namespace JSC

trunk/Source/JavaScriptCore/runtime/ShadowRealmPrototype.h

@@ -64 +64 @@
 JSC_DECLARE_HOST_FUNCTION(importInRealm);
 JSC_DECLARE_HOST_FUNCTION(evalInRealm);
+JSC_DECLARE_HOST_FUNCTION(moveFunctionToRealm);
 
 } // namespace JSC