Skip to content

Commit

Permalink
Merge r229036 - validateStackAccess should not validate if the offset…
Browse files Browse the repository at this point in the history
… is within the stack bounds

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:
  • Loading branch information
Saam Barati authored and carlosgcampos committed Mar 5, 2018
1 parent 37cc3d4 commit 67f53cd
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 3 deletions.
14 changes: 14 additions & 0 deletions JSTests/ChangeLog
@@ -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
Expand Down
@@ -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();
16 changes: 16 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -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
Expand Down
3 changes: 0 additions & 3 deletions Source/JavaScriptCore/b3/B3Validate.cpp
Expand Up @@ -608,10 +608,7 @@ class Validater {
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(
Expand Down

0 comments on commit 67f53cd

Please sign in to comment.