Attachment 370586 Details for Bug 198229 – Patch

[patch] Patch

bug-198229-20190524225943.patch (text/plain), 4.11 KB, created by Tadeu Zagallo on 2019-05-24 13:59:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-24 13:59:44 PDT

Size: 4.11 KB

patch

obsolete

>Subversion Revision: 245742
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index e9d34b8bafd14c1a66aab6c66f8d603fe17c128d..27f62b3b80e22b06f9411ada2034700d200626cb 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-24  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        JITOperations getByVal should mark negative array indices as out-of-bounds
>+        https://bugs.webkit.org/show_bug.cgi?id=198229
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        get_by_val with an array or string as base value and a negative index causes DFG to OSR exit,
>+        but baseline doesn't mark it as out-of-bounds, since it only considers positive indices. This
>+        leads to discarding DFG code, recompiling it and exiting at the same bytecode.
>+
>+        * jit/JITOperations.cpp:
>+        (JSC::getByVal):
>+
> 2019-05-23  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
>diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
>index 12b78aadf4589dd483cc8792d2d6386351990ef4..b5c545b8c0cbf95b25e1c5b676e6214a574410a7 100644
>--- a/Source/JavaScriptCore/jit/JITOperations.cpp
>+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
>@@ -1850,13 +1850,13 @@ static JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript, B
>         }
>     }
> 
>-    if (subscript.isUInt32()) {
>+    if (subscript.isInt32()) {
>         ASSERT(exec->bytecodeOffset());
>         byValInfo->tookSlowPath = true;
> 
>-        uint32_t i = subscript.asUInt32();
>+        int32_t i = subscript.asInt32();
>         if (isJSString(baseValue)) {
>-            if (asString(baseValue)->canGetIndex(i)) {
>+            if (i >= 0 && asString(baseValue)->canGetIndex(i)) {
>                 ctiPatchCallByReturnAddress(returnAddress, operationGetByValString);
>                 RELEASE_AND_RETURN(scope, asString(baseValue)->getIndex(exec, i));
>             }
>@@ -1868,7 +1868,7 @@ static JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript, B
> 
>             bool skipMarkingOutOfBounds = false;
> 
>-            if (object->indexingType() == ArrayWithContiguous && i < object->butterfly()->publicLength()) {
>+            if (object->indexingType() == ArrayWithContiguous && i >= 0 && static_cast<uint32_t>(i) < object->butterfly()->publicLength()) {
>                 // FIXME: expand this to ArrayStorage, Int32, and maybe Double:
>                 // https://bugs.webkit.org/show_bug.cgi?id=182940
>                 auto* globalObject = object->globalObject(vm);
>@@ -1883,7 +1883,8 @@ static JSValue getByVal(ExecState* exec, JSValue baseValue, JSValue subscript, B
>             }
>         }
> 
>-        RELEASE_AND_RETURN(scope, baseValue.get(exec, i));
>+        if (i >= 0)
>+            RELEASE_AND_RETURN(scope, baseValue.get(exec, static_cast<uint32_t>(i)));
>     }
> 
>     baseValue.requireObjectCoercible(exec);
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 68de36b936122f781bac003da877ac8209041dfa..0ef06b08958b847f9a4aef8f8b992970bf79f86a 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-05-24  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        JITOperations getByVal should mark negative array indices as out-of-bounds
>+        https://bugs.webkit.org/show_bug.cgi?id=198229
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * microbenchmarks/get-by-val-negative-array-index.js: Added.
>+        (foo):
>+
> 2019-05-23  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         DFG::OSREntry should not perform arity check
>diff --git a/JSTests/microbenchmarks/get-by-val-negative-array-index.js b/JSTests/microbenchmarks/get-by-val-negative-array-index.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..1a86915f59c47e8793a3ddac87a1ae6ea11819fe
>--- /dev/null
>+++ b/JSTests/microbenchmarks/get-by-val-negative-array-index.js
>@@ -0,0 +1,11 @@
>+function foo(arr, index) {
>+    return arr[index];
>+}
>+noInline(foo);
>+
>+const arr = new Array(1000).fill({});
>+for (let i = 0; i < 1e7; i++) {
>+    foo(arr, i % arr.length);
>+    if (!(i % 1e3))
>+        foo(arr, -1);
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 198229: 370586 | 370630