validateStackAccess should not validate if the offset is within the stack bounds
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Feb 2018 20:42:39 +0000 (20:42 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Feb 2018 20:42:39 +0000 (20:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183067
<rdar://problem/37749988>

Reviewed by Mark Lam.

JSTests:

* stress/dont-validate-stack-offset-in-b3-because-it-might-be-guarded-by-control-flow.js: Added.
(assert):
(test.a):
(test.b):
(test):

Source/JavaScriptCore:

The validation rule was saying that any load from the stack must be
within the stack bounds of the frame. However, it's natural for a user
of B3 to emit code that may be outside of B3's stack bounds, but guard
such a load with a branch. The FTL does exactly this with GetMyArgumentByVal.
B3 is wrong to assert that this is a static property about all stack loads.

* b3/B3Validate.cpp:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229036 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/dont-validate-stack-offset-in-b3-because-it-might-be-guarded-by-control-flow.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3Validate.cpp

index 60eecf4..3bcc03e 100644 (file)
@@ -1,3 +1,17 @@
+2018-02-26  Saam Barati  <sbarati@apple.com>
+
+        validateStackAccess should not validate if the offset is within the stack bounds
+        https://bugs.webkit.org/show_bug.cgi?id=183067
+        <rdar://problem/37749988>
+
+        Reviewed by Mark Lam.
+
+        * stress/dont-validate-stack-offset-in-b3-because-it-might-be-guarded-by-control-flow.js: Added.
+        (assert):
+        (test.a):
+        (test.b):
+        (test):
+
 2018-02-26  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Unreviewed, skip FTL tests if FTL is disabled
diff --git a/JSTests/stress/dont-validate-stack-offset-in-b3-because-it-might-be-guarded-by-control-flow.js b/JSTests/stress/dont-validate-stack-offset-in-b3-because-it-might-be-guarded-by-control-flow.js
new file mode 100644 (file)
index 0000000..0a8af33
--- /dev/null
@@ -0,0 +1,26 @@
+function assert(b) {
+    if (!b)
+        throw new Error;
+}
+noInline(assert);
+
+function test() {
+    function a(a1, a2, a3, ...rest) {
+        return [rest.length, rest[0], rest[10]];
+    }
+
+    function b(...rest) {
+        return a.apply(null, rest);
+    }
+    noInline(b);
+
+    for (let i = 0; i < 12000; i++) {
+        b();
+        let r = a(undefined, 0);
+        assert(r[0] === 0);
+        assert(r[1] === undefined);
+        assert(r[2] === undefined);
+    }
+}
+
+test();
index 99ed80d..3374457 100644 (file)
@@ -1,3 +1,19 @@
+2018-02-26  Saam Barati  <sbarati@apple.com>
+
+        validateStackAccess should not validate if the offset is within the stack bounds
+        https://bugs.webkit.org/show_bug.cgi?id=183067
+        <rdar://problem/37749988>
+
+        Reviewed by Mark Lam.
+
+        The validation rule was saying that any load from the stack must be
+        within the stack bounds of the frame. However, it's natural for a user
+        of B3 to emit code that may be outside of B3's stack bounds, but guard
+        such a load with a branch. The FTL does exactly this with GetMyArgumentByVal.
+        B3 is wrong to assert that this is a static property about all stack loads.
+
+        * b3/B3Validate.cpp:
+
 2018-02-23  Saam Barati  <sbarati@apple.com>
 
         Make Number.isInteger an intrinsic
index bb9de9d..ce29541 100644 (file)
@@ -608,10 +608,7 @@ private:
         if (!slotBase)
             return;
 
-        StackSlot* stack = slotBase->slot();
-
         VALIDATE(memory->offset() >= 0, ("At ", *value));
-        VALIDATE(memory->offset() + memory->accessByteSize() <= stack->byteSize(), ("At ", *value));
     }
     
     NO_RETURN_DUE_TO_CRASH void fail(