Attachment 346933 Details for Bug 185127 – Patch

[patch] Patch

bug-185127-20180810150556.patch (text/plain), 11.72 KB, created by Keith Miller on 2018-08-10 15:05:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-08-10 15:05:57 PDT

Size: 11.72 KB

patch

obsolete

>Subversion Revision: 234728
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index f048ee111b4c7e6d6f68fd6039859a33b129f7a3..6605f1a65865f583505c745683f99ecf4d288e07 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-08-10  Keith Miller  <keith_miller@apple.com>
>+
>+        Slicing an ArrayBuffer with a long number returns an ArrayBuffer with byteLength zero
>+        https://bugs.webkit.org/show_bug.cgi?id=185127
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Previously, we would truncate the indicies passed to slice to an
>+        int. This meant that the value was not getting properly clamped
>+        later.
>+
>+        This patch also removes a non-spec compliant check that slice was
>+        passed at least one argument.
>+
>+        * runtime/ArrayBuffer.cpp:
>+        (JSC::ArrayBuffer::clampValue):
>+        (JSC::ArrayBuffer::clampIndex const):
>+        (JSC::ArrayBuffer::slice const):
>+        * runtime/ArrayBuffer.h:
>+        (JSC::ArrayBuffer::clampValue): Deleted.
>+        (JSC::ArrayBuffer::clampIndex const): Deleted.
>+        * runtime/JSArrayBufferPrototype.cpp:
>+        (JSC::arrayBufferProtoFuncSlice):
>+
> 2018-08-08  Keith Miller  <keith_miller@apple.com>
> 
>         Array.prototype.sort should call @toLength instead of ">>> 0"
>diff --git a/Source/JavaScriptCore/runtime/ArrayBuffer.cpp b/Source/JavaScriptCore/runtime/ArrayBuffer.cpp
>index 726d664d23f419aa1e86f8fbccbb01a9b5de66d8..25ea8900f17937904b0e219f7372ea23b4cbcf91 100644
>--- a/Source/JavaScriptCore/runtime/ArrayBuffer.cpp
>+++ b/Source/JavaScriptCore/runtime/ArrayBuffer.cpp
>@@ -267,12 +267,30 @@ ArrayBuffer::ArrayBuffer(ArrayBufferContents&& contents)
> {
> }
> 
>-RefPtr<ArrayBuffer> ArrayBuffer::slice(int begin, int end) const
>+unsigned ArrayBuffer::clampValue(double x, unsigned left, unsigned right)
>+{
>+    ASSERT(left <= right);
>+    if (x < left)
>+        x = left;
>+    if (right < x)
>+        x = right;
>+    return x;
>+}
>+
>+unsigned ArrayBuffer::clampIndex(double index) const
>+{
>+    unsigned currentLength = byteLength();
>+    if (index < 0)
>+        index = currentLength + index;
>+    return clampValue(index, 0, currentLength);
>+}
>+
>+RefPtr<ArrayBuffer> ArrayBuffer::slice(double begin, double end) const
> {
>     return sliceImpl(clampIndex(begin), clampIndex(end));
> }
> 
>-RefPtr<ArrayBuffer> ArrayBuffer::slice(int begin) const
>+RefPtr<ArrayBuffer> ArrayBuffer::slice(double begin) const
> {
>     return sliceImpl(clampIndex(begin), byteLength());
> }
>diff --git a/Source/JavaScriptCore/runtime/ArrayBuffer.h b/Source/JavaScriptCore/runtime/ArrayBuffer.h
>index fc027ea79e62aefe354d775a7e15a5448eab2d5d..deea2545096cb596c181aec768515fc1c1a08051 100644
>--- a/Source/JavaScriptCore/runtime/ArrayBuffer.h
>+++ b/Source/JavaScriptCore/runtime/ArrayBuffer.h
>@@ -126,8 +126,8 @@ public:
> 
>     inline size_t gcSizeEstimateInBytes() const;
> 
>-    JS_EXPORT_PRIVATE RefPtr<ArrayBuffer> slice(int begin, int end) const;
>-    JS_EXPORT_PRIVATE RefPtr<ArrayBuffer> slice(int begin) const;
>+    JS_EXPORT_PRIVATE RefPtr<ArrayBuffer> slice(double begin, double end) const;
>+    JS_EXPORT_PRIVATE RefPtr<ArrayBuffer> slice(double begin) const;
>     
>     inline void pin();
>     inline void unpin();
>@@ -153,8 +153,8 @@ private:
>     static RefPtr<ArrayBuffer> tryCreate(unsigned numElements, unsigned elementByteSize, ArrayBufferContents::InitializationPolicy);
>     ArrayBuffer(ArrayBufferContents&&);
>     RefPtr<ArrayBuffer> sliceImpl(unsigned begin, unsigned end) const;
>-    inline unsigned clampIndex(int index) const;
>-    static inline int clampValue(int x, int left, int right);
>+    inline unsigned clampIndex(double index) const;
>+    static inline unsigned clampValue(double x, unsigned left, unsigned right);
> 
>     void notifyIncommingReferencesOfTransfer(VM&);
> 
>@@ -169,16 +169,6 @@ public:
>     Weak<JSArrayBuffer> m_wrapper;
> };
> 
>-int ArrayBuffer::clampValue(int x, int left, int right)
>-{
>-    ASSERT(left <= right);
>-    if (x < left)
>-        x = left;
>-    if (right < x)
>-        x = right;
>-    return x;
>-}
>-
> void* ArrayBuffer::data()
> {
>     return m_contents.m_data.getMayBeNull();
>@@ -205,14 +195,6 @@ size_t ArrayBuffer::gcSizeEstimateInBytes() const
>     return sizeof(ArrayBuffer) + static_cast<size_t>(byteLength());
> }
> 
>-unsigned ArrayBuffer::clampIndex(int index) const
>-{
>-    unsigned currentLength = byteLength();
>-    if (index < 0)
>-        index = currentLength + index;
>-    return clampValue(index, 0, currentLength);
>-}
>-
> void ArrayBuffer::pin()
> {
>     m_pinCount++;
>diff --git a/Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp b/Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp
>index a3ac968ac2561497fb77737bf7cd5d163c05d451..28811cc507917a0b3ba0197c33776d83e6c65deb 100644
>--- a/Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp
>@@ -43,18 +43,15 @@ static EncodedJSValue JSC_HOST_CALL arrayBufferProtoFuncSlice(ExecState* exec)
>     JSFunction* callee = jsCast<JSFunction*>(exec->jsCallee());
>     
>     JSArrayBuffer* thisObject = jsDynamicCast<JSArrayBuffer*>(vm, exec->thisValue());
>-    if (!thisObject)
>-        return throwVMTypeError(exec, scope, "Receiver of slice must be an array buffer."_s);
>-    
>-    if (!exec->argumentCount())
>-        return throwVMTypeError(exec, scope, "Slice requires at least one argument."_s);
>-    
>-    int32_t begin = exec->argument(0).toInt32(exec);
>+    if (!thisObject || thisObject->impl()->isShared())
>+        return throwVMTypeError(exec, scope, "Receiver of slice must be an ArrayBuffer."_s);
>+
>+    double begin = exec->argument(0).toInteger(exec);
>     RETURN_IF_EXCEPTION(scope, encodedJSValue());
>     
>-    int32_t end;
>-    if (exec->argumentCount() >= 2) {
>-        end = exec->uncheckedArgument(1).toInt32(exec);
>+    double end;
>+    if (!exec->argument(1).isUndefined()) {
>+        end = exec->uncheckedArgument(1).toInteger(exec);
>         RETURN_IF_EXCEPTION(scope, encodedJSValue());
>     } else
>         end = thisObject->impl()->byteLength();
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index e83671fd371f22362816ec9cf7593dccd37eb84d..2ca671115d54093ca5a333efbb83d54ce3dfd50b 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-08-10  Keith Miller  <keith_miller@apple.com>
>+
>+        Slicing an ArrayBuffer with a long number returns an ArrayBuffer with byteLength zero
>+        https://bugs.webkit.org/show_bug.cgi?id=185127
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rebaseline the expectations.
>+
>+        * test262/expectations.yaml:
>+
> 2018-08-08  Keith Miller  <keith_miller@apple.com>
> 
>         Array.prototype.sort should call @toLength instead of ">>> 0"
>diff --git a/JSTests/test262/expectations.yaml b/JSTests/test262/expectations.yaml
>index 3fe3d5cf2f335ecf303d297254ced2253a548477..0b6c5981863af93f16000b37b0dbbc7551f71543 100644
>--- a/JSTests/test262/expectations.yaml
>+++ b/JSTests/test262/expectations.yaml
>@@ -708,33 +708,30 @@ test/built-ins/ArrayBuffer/proto-from-ctor-realm.js:
> test/built-ins/ArrayBuffer/prototype/byteLength/detached-buffer.js:
>   default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>   strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>-test/built-ins/ArrayBuffer/prototype/slice/end-default-if-undefined.js:
>-  default: 'Test262Error: Expected SameValue(ÃÂ«0ÃÂ», ÃÂ«2ÃÂ») to be true'
>-  strict mode: 'Test262Error: Expected SameValue(ÃÂ«0ÃÂ», ÃÂ«2ÃÂ») to be true'
>-test/built-ins/ArrayBuffer/prototype/slice/end-exceeds-length.js:
>-  default: 'Test262Error: slice(2, 0x100000000) Expected SameValue(ÃÂ«0ÃÂ», ÃÂ«6ÃÂ») to be true'
>-  strict mode: 'Test262Error: slice(2, 0x100000000) Expected SameValue(ÃÂ«0ÃÂ», ÃÂ«6ÃÂ») to be true'
>-test/built-ins/ArrayBuffer/prototype/slice/species-constructor-is-undefined.js:
>-  default: 'TypeError: Slice requires at least one argument.'
>-  strict mode: 'TypeError: Slice requires at least one argument.'
>-test/built-ins/ArrayBuffer/prototype/slice/species-is-null.js:
>-  default: 'TypeError: Slice requires at least one argument.'
>-  strict mode: 'TypeError: Slice requires at least one argument.'
>-test/built-ins/ArrayBuffer/prototype/slice/species-is-undefined.js:
>-  default: 'TypeError: Slice requires at least one argument.'
>-  strict mode: 'TypeError: Slice requires at least one argument.'
>+test/built-ins/ArrayBuffer/prototype/slice/species-constructor-is-not-object.js:
>+  default: 'Test262Error: `constructor` value is null Expected a TypeError to be thrown but no exception was thrown at all'
>+  strict mode: 'Test262Error: `constructor` value is null Expected a TypeError to be thrown but no exception was thrown at all'
>+test/built-ins/ArrayBuffer/prototype/slice/species-is-not-constructor.js:
>+  default: 'Test262Error: `constructor[Symbol.species]` value is Object Expected a TypeError to be thrown but no exception was thrown at all'
>+  strict mode: 'Test262Error: `constructor[Symbol.species]` value is Object Expected a TypeError to be thrown but no exception was thrown at all'
>+test/built-ins/ArrayBuffer/prototype/slice/species-is-not-object.js:
>+  default: 'Test262Error: `constructor[Symbol.species]` value is Boolean Expected a TypeError to be thrown but no exception was thrown at all'
>+  strict mode: 'Test262Error: `constructor[Symbol.species]` value is Boolean Expected a TypeError to be thrown but no exception was thrown at all'
> test/built-ins/ArrayBuffer/prototype/slice/species-returns-larger-arraybuffer.js:
>-  default: 'TypeError: Slice requires at least one argument.'
>-  strict mode: 'TypeError: Slice requires at least one argument.'
>+  default: 'Test262Error: Expected SameValue(ÃÂ«8ÃÂ», ÃÂ«10ÃÂ») to be true'
>+  strict mode: 'Test262Error: Expected SameValue(ÃÂ«8ÃÂ», ÃÂ«10ÃÂ») to be true'
>+test/built-ins/ArrayBuffer/prototype/slice/species-returns-not-arraybuffer.js:
>+  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>+  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>+test/built-ins/ArrayBuffer/prototype/slice/species-returns-same-arraybuffer.js:
>+  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>+  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>+test/built-ins/ArrayBuffer/prototype/slice/species-returns-smaller-arraybuffer.js:
>+  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
>+  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
> test/built-ins/ArrayBuffer/prototype/slice/species.js:
>-  default: 'TypeError: Slice requires at least one argument.'
>-  strict mode: 'TypeError: Slice requires at least one argument.'
>-test/built-ins/ArrayBuffer/prototype/slice/start-default-if-absent.js:
>-  default: 'TypeError: Slice requires at least one argument.'
>-  strict mode: 'TypeError: Slice requires at least one argument.'
>-test/built-ins/ArrayBuffer/prototype/slice/start-exceeds-length.js:
>-  default: 'Test262Error: slice(0x100000000, 7) Expected SameValue(ÃÂ«7ÃÂ», ÃÂ«0ÃÂ») to be true'
>-  strict mode: 'Test262Error: slice(0x100000000, 7) Expected SameValue(ÃÂ«7ÃÂ», ÃÂ«0ÃÂ») to be true'
>+  default: 'Test262Error: Expected SameValue(ÃÂ«[object ArrayBuffer]ÃÂ», ÃÂ«undefinedÃÂ») to be true'
>+  strict mode: 'Test262Error: Expected SameValue(ÃÂ«[object ArrayBuffer]ÃÂ», ÃÂ«undefinedÃÂ») to be true'
> test/built-ins/ArrayIteratorPrototype/next/detach-typedarray-in-progress.js:
>   default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
>   strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185127: 346933