Pārlūkot izejas kodu

Removing some marshalling code for bound function

In order to fix OS14392550, I was trying to make the changes in the bound function. However with few testing it is determined that we don't need to do this Unnecessary marshalling code.
Added few test-cases to see if the marshalling does work as expected.
Akrosh Gandhi 8 gadi atpakaļ
vecāks
revīzija
a17f84bb4e

+ 4 - 39
lib/Runtime/Library/BoundFunction.cpp

@@ -31,6 +31,8 @@ namespace Js
         ScriptContext *scriptContext = this->GetScriptContext();
         targetFunction = RecyclableObject::FromVar(args[0]);
 
+        Assert(!CrossSite::NeedMarshalVar(targetFunction, scriptContext));
+
         // Let proto be targetFunction.[[GetPrototypeOf]]().
         RecyclableObject* proto = JavascriptOperators::GetPrototype(targetFunction);
         if (proto != type->GetPrototype())
@@ -150,7 +152,6 @@ namespace Js
 
         if (boundFunction->count > 0)
         {
-            BOOL isCrossSiteObject = boundFunction->IsCrossSiteObject();
             // OACR thinks that this can change between here and the check in the for loop below
             const unsigned int argCount = args.Info.Count;
 
@@ -176,24 +177,9 @@ namespace Js
                 newValues[index++] = boundFunction->boundThis;
             }
 
-            // Copy the bound args
-            if (!isCrossSiteObject)
+            for (uint i = 0; i < boundFunction->count; i++)
             {
-                for (uint i = 0; i < boundFunction->count; i++)
-                {
-                    newValues[index++] = boundFunction->boundArgs[i];
-                }
-            }
-            else
-            {
-                // it is possible that the bound arguments are not marshalled yet.
-                for (uint i = 0; i < boundFunction->count; i++)
-                {
-                    //warning C6386: Buffer overrun while writing to 'newValues':  the writable size is 'boundFunction->count+argCount*8' bytes, but '40' bytes might be written.
-                    // there's throw with args.Info.Count == 0, so here won't hit buffer overrun, and __analyze_assume(argCount>0) does not work
-#pragma warning(suppress: 6386)
-                    newValues[index++] = CrossSite::MarshalVar(scriptContext, boundFunction->boundArgs[i]);
-                }
+                newValues[index++] = boundFunction->boundArgs[i];
             }
 
             // Copy the extra args
@@ -229,27 +215,6 @@ namespace Js
         return aReturnValue;
     }
 
-    void BoundFunction::MarshalToScriptContext(Js::ScriptContext * scriptContext)
-    {
-        Assert(this->GetScriptContext() != scriptContext);
-        AssertMsg(VirtualTableInfo<BoundFunction>::HasVirtualTable(this), "Derived class need to define marshal to script context");
-        VirtualTableInfo<Js::CrossSiteObject<BoundFunction>>::SetVirtualTable(this);
-        this->targetFunction = (RecyclableObject*)CrossSite::MarshalVar(scriptContext, this->targetFunction);
-        this->boundThis = (RecyclableObject*)CrossSite::MarshalVar(this->GetScriptContext(), this->boundThis);
-        for (uint i = 0; i < count; i++)
-        {
-            this->boundArgs[i] = CrossSite::MarshalVar(this->GetScriptContext(), this->boundArgs[i]);
-        }
-    }
-
-#if ENABLE_TTD
-    void BoundFunction::MarshalCrossSite_TTDInflate()
-    {
-        AssertMsg(VirtualTableInfo<BoundFunction>::HasVirtualTable(this), "Derived class need to define marshal");
-        VirtualTableInfo<Js::CrossSiteObject<BoundFunction>>::SetVirtualTable(this);
-    }
-#endif
-
     JavascriptFunction * BoundFunction::GetTargetFunction() const
     {
         if (targetFunction != nullptr)

+ 1 - 5
lib/Runtime/Library/BoundFunction.h

@@ -10,11 +10,7 @@ namespace Js
     {
     protected:
         DEFINE_VTABLE_CTOR(BoundFunction, JavascriptFunction);
-        virtual void MarshalToScriptContext(Js::ScriptContext * scriptContext) override;
-
-#if ENABLE_TTD
-        virtual void MarshalCrossSite_TTDInflate() override;
-#endif
+        DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(BoundFunction);
 
     private:
         bool GetPropertyBuiltIns(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext, BOOL* result);

+ 182 - 0
test/Bugs/cross_context_test.js

@@ -0,0 +1,182 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+
+var tests = [
+  {
+    name: "Assigning a bound function to the proxy's prototype should not fire the assert",
+    body: function () {
+        var pr = new Proxy({}, {
+                getPrototypeOf: function() {
+                    return;
+                }
+            }).__proto__ = Float64Array.bind()
+    }
+  },
+  {
+    name: "a function and it's bind function are one context and invoked from a different context",
+    body: function () {
+        var sc1 = WScript.LoadScript(`
+        function assertAreEqual(a, b) { if (a != b) { throw new Error('expected : ' + a + ', actual : ' + b) } };
+        function foo(a, b, c) {
+            assertAreEqual(undefined, a);
+            assertAreEqual(1, b.x);
+            assertAreEqual('three', c);
+            return {d:10};
+        };
+    
+        function test() {
+            var bf = foo.bind(undefined, undefined, {x:1}, 'three');
+            assertAreEqual(10, bf().d);
+        
+            var bf1 = foo.bind(undefined, undefined);
+            assertAreEqual(10, bf1({x:1}, 'three').d);
+            }
+        `,
+        "samethread");
+    
+        sc1.test();
+    }
+  },
+  {
+    name: "a bound function is passed to first context and called from there",
+    body: function () {
+        var sc1 = WScript.LoadScript(`
+        function assertAreEqual(a, b) { if (a != b) { throw new Error('expected : ' + b + ', actual : ' + a) } };
+        function foo(a, b, c) {
+            assertAreEqual(undefined, a);
+            assertAreEqual(1, b.x);
+            assertAreEqual('three', c);
+            return {d:10};
+        };
+    
+        var bf1 = foo.bind(undefined, undefined, {x:1}, 'three');
+        var bf2 = foo.bind(undefined, undefined);
+    
+        function test() {
+            return foo.bind(undefined, undefined, {x:1}, 'three');
+        }
+        function test1() {
+            return foo.bind(undefined, undefined);
+        }
+        `,
+        "samethread");
+    
+        assert.areEqual(10, sc1.bf1().d);
+        assert.areEqual(10, sc1.bf2({x:1}, 'three').d);
+        assert.areEqual(10, sc1.test()().d);
+        assert.areEqual(10, sc1.test1()({x:1}, 'three').d);
+        }
+  },
+  {
+    name: "bound function is created on second context on the function passed from the first context",
+    body: function () {
+        function foo(a, b, c) {
+            assert.areEqual(undefined, a);
+            assert.areEqual(1, b.x);
+            assert.areEqual('three', c);
+            return {d:10};
+        };
+
+        var sc1 = WScript.LoadScript(`
+            var bf1;
+            var bf2;
+            function setup(func) {
+                bf1 = func.bind(undefined, undefined, {x:1}, 'three');
+            }
+            function setup1(func, a) {
+                bf2 = func.bind(func, a);
+            }
+        
+            function test() {
+                return bf1();
+            }
+            function test1(a, b) {
+                return bf2(a, b);
+            }   
+        `,
+        "samethread");
+
+        sc1.setup(foo);
+        sc1.setup1(foo, undefined);
+    
+        assert.areEqual(10, sc1.test().d);
+        assert.areEqual(10, sc1.test({x:1}, 'three').d);
+    
+    }
+  },
+  {
+    name: "bound function is created on second context on the function passed from the first context and invoked directly from first context",
+    body: function () {
+        function foo(a, b, c) {
+            assert.areEqual(undefined, a);
+            assert.areEqual(1, b.x);
+            assert.areEqual('three', c);
+            return {d:10};
+        };
+
+        var sc1 = WScript.LoadScript(`
+            function test(func) {
+                return func.bind(undefined, undefined, {x:1}, 'three');
+            }
+            function test1(func, a) {
+                return func.bind(func, a);
+            }
+            `,
+        "samethread");
+
+        assert.areEqual(10, sc1.test(foo)().d);
+        assert.areEqual(10, sc1.test1(foo, undefined)({x:1}, 'three').d);
+        
+    }
+  },
+  {
+    name: "bound function is created on second context on the function passed from the third context",
+    body: function () {
+        var sc1 = WScript.LoadScript(`
+            function assertAreEqual(a, b) { if (a != b) { throw new Error('expected : ' + b + ', actual : ' + a) } };
+            function foo(a, b, c) {
+                assertAreEqual(undefined, a);
+                assertAreEqual(1, b.x);
+                assertAreEqual('three', c);
+                return {d:10};
+            };
+        `,
+        "samethread");
+    
+        function foo(a, b, c) {
+            assert.areEqual(undefined, a);
+            assert.areEqual(1, b.x);
+            assert.areEqual('three', c);
+            return {d:10};
+        };
+
+        var sc2 = WScript.LoadScript(`
+            var bf1, bf2;
+        
+            function setup(obj, a) {
+                bf1 = obj.foo.bind(undefined, undefined, {x:1}, 'three');
+                bf2 = obj.foo.bind(obj.foo, a);
+            }
+        
+            function test() {
+                return bf1();
+            }
+            function test1(a, b) {
+                return bf2(a, b);
+            }
+        `,
+        "samethread");
+    
+        sc2.setup(sc1, undefined);
+        assert.areEqual(10, sc2.test().d);
+        assert.areEqual(10, sc2.test1({x:1}, 'three').d);
+    }
+  },
+  
+];
+
+testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

+ 6 - 0
test/Bugs/rlexe.xml

@@ -317,6 +317,12 @@
       <compile-flags>-args summary -endargs</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>cross_context_test.js</files>
+      <compile-flags>-args summary -endargs</compile-flags>
+    </default>
+  </test>
   <test>
     <default>
       <files>json_bugs.js</files>