Attachment 357330 Details for Bug 192709 – Patch for landing

[patch] Patch for landing

bug-192709-20181214121521.patch (text/plain), 6.47 KB, created by Keith Miller on 2018-12-14 12:15:23 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-12-14 12:15:23 PST

Size: 6.47 KB

patch

obsolete

>Subversion Revision: 239180
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 75aa37446fb7bbe4281f1aaf02984bc1c2b13a7a..fc56c75f0d84411dd23ed46eef21ed2dad19be2b 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2018-12-14  Keith Miller  <keith_miller@apple.com>
>+
>+        Callers of JSString::getIndex should check for OOM exceptions
>+        https://bugs.webkit.org/show_bug.cgi?id=192709
>+
>+        Reviewed by Mark Lam.
>+
>+        This patch also allows Strings to OOM when the StringObject wrapper
>+        attempts to look up an own property on the string.
>+
>+        Remove isExtensibleImpl because it's only used in one place and call
>+        isStructureExtensible instead.
>+
>+        * runtime/JSObject.cpp:
>+        (JSC::JSObject::isExtensible):
>+        * runtime/JSObject.h:
>+        (JSC::JSObject::isExtensibleImpl): Deleted.
>+        * runtime/JSString.h:
>+        (JSC::JSString::getStringPropertySlot):
>+        * runtime/StringObject.cpp:
>+        (JSC::StringObject::defineOwnProperty):
>+
> 2018-12-13  Caio Lima  <ticaiolima@gmail.com>
> 
>         [BigInt] Add ValueDiv into DFG
>diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
>index b7fa7959286297155861f85c576cbcac3c3a80ac..20fcd4032ce6b38317c76c53879bcfd3b6b1688f 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.cpp
>+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
>@@ -2431,7 +2431,7 @@ bool JSObject::preventExtensions(JSObject* object, ExecState* exec)
> 
> bool JSObject::isExtensible(JSObject* obj, ExecState* exec)
> {
>-    return obj->isExtensibleImpl(exec->vm());
>+    return obj->isStructureExtensible(exec->vm());
> }
> 
> bool JSObject::isExtensible(ExecState* exec)
>diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h
>index 148b1b1bbf6bb1a7fb6e5772b0e4319e379807d3..a634ae1350525091f5960fa46db13d5f19cc95a9 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.h
>+++ b/Source/JavaScriptCore/runtime/JSObject.h
>@@ -753,7 +753,6 @@ public:
> 
> private:
>     NonPropertyTransition suggestedArrayStorageTransition(VM&) const;
>-    ALWAYS_INLINE bool isExtensibleImpl(VM& vm) { return isStructureExtensible(vm); }
> public:
>     // You should only call isStructureExtensible() when:
>     // - Performing this check in a way that isn't described in the specification 
>diff --git a/Source/JavaScriptCore/runtime/JSString.h b/Source/JavaScriptCore/runtime/JSString.h
>index 605f40a3b857c8189bfdf6f55fd065a891869aaa..e0c04deb3821beae2431c6e1796534b3fd829f13 100644
>--- a/Source/JavaScriptCore/runtime/JSString.h
>+++ b/Source/JavaScriptCore/runtime/JSString.h
>@@ -687,6 +687,8 @@ ALWAYS_INLINE JSString* jsStringWithCache(ExecState* exec, const String& s)
> ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
> {
>     VM& vm = exec->vm();
>+    auto scope = DECLARE_THROW_SCOPE(vm);
>+
>     if (propertyName == vm.propertyNames->length) {
>         slot.setValue(this, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, jsNumber(length()));
>         return true;
>@@ -694,7 +696,9 @@ ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, PropertyName
> 
>     std::optional<uint32_t> index = parseIndex(propertyName);
>     if (index && index.value() < length()) {
>-        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, getIndex(exec, index.value()));
>+        JSValue value = getIndex(exec, index.value());
>+        RETURN_IF_EXCEPTION(scope, false);
>+        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, value);
>         return true;
>     }
> 
>@@ -703,8 +707,13 @@ ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, PropertyName
> 
> ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, unsigned propertyName, PropertySlot& slot)
> {
>+    VM& vm = exec->vm();
>+    auto scope = DECLARE_THROW_SCOPE(vm);
>+
>     if (propertyName < length()) {
>-        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, getIndex(exec, propertyName));
>+        JSValue value = getIndex(exec, propertyName);
>+        RETURN_IF_EXCEPTION(scope, false);
>+        slot.setValue(this, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, value);
>         return true;
>     }
> 
>diff --git a/Source/JavaScriptCore/runtime/StringObject.cpp b/Source/JavaScriptCore/runtime/StringObject.cpp
>index 504a600e62cc4555dc3be2c8927b1f35e5a8c7c7..234ef89f54f0c9173ae60e5e8f43f410cd8be675 100644
>--- a/Source/JavaScriptCore/runtime/StringObject.cpp
>+++ b/Source/JavaScriptCore/runtime/StringObject.cpp
>@@ -114,7 +114,8 @@ bool StringObject::defineOwnProperty(JSObject* object, ExecState* exec, Property
>         // https://tc39.github.io/ecma262/#sec-string-exotic-objects-getownproperty-p
>         PropertyDescriptor current;
>         bool isCurrentDefined = thisObject->getOwnPropertyDescriptor(exec, propertyName, current);
>-        ASSERT(isCurrentDefined);
>+        EXCEPTION_ASSERT(!scope.exception() == isCurrentDefined);
>+        RETURN_IF_EXCEPTION(scope, false);
>         bool isExtensible = thisObject->isExtensible(exec);
>         RETURN_IF_EXCEPTION(scope, false);
>         RELEASE_AND_RETURN(scope, validateAndApplyPropertyDescriptor(exec, nullptr, propertyName, isExtensible, descriptor, isCurrentDefined, current, throwException));
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index e33fa4051c8d1c583f852da242f01500f9d1f027..1ed0c28a0ce8ee9949e7492265ac17639928c91f 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-12-14  Keith Miller  <keith_miller@apple.com>
>+
>+        Callers of JSString::getIndex should check for OOM exceptions
>+        https://bugs.webkit.org/show_bug.cgi?id=192709
>+
>+        Reviewed by Mark Lam.
>+
>+        * stress/StringObject-define-length-getter-rope-string-oom.js: Added.
>+
> 2018-12-13  Caio Lima  <ticaiolima@gmail.com>
> 
>         [BigInt] Add ValueDiv into DFG
>diff --git a/JSTests/stress/StringObject-define-length-getter-rope-string-oom.js b/JSTests/stress/StringObject-define-length-getter-rope-string-oom.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..e7e706c0fe84619547b5755f098ee89e823adde1
>--- /dev/null
>+++ b/JSTests/stress/StringObject-define-length-getter-rope-string-oom.js
>@@ -0,0 +1,5 @@
>+try {
>+    let char16 = decodeURI('%E7%9A%84');
>+    let rope = char16.padEnd(2147483644, 1);
>+    rope.__defineGetter__(256, function () {});
>+} catch { }

Actions: View | Formatted Diff | Diff

Attachments on bug 192709: 357327 | 357330