Changeset 239227 in webkit

Timestamp:

Dec 14, 2018 12:53:08 PM (5 years ago)

Author:

keith_miller@apple.com

Message:

Callers of JSString::getIndex should check for OOM exceptions
​https://bugs.webkit.org/show_bug.cgi?id=192709

Reviewed by Mark Lam.

JSTests:

• stress/StringObject-define-length-getter-rope-string-oom.js: Added.

Source/JavaScriptCore:

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/StringObject-define-length-getter-rope-string-oom.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSObject.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSObject.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSString.h (modified) (3 diffs)
• Source/JavaScriptCore/runtime/StringObject.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+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  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -2432 +2432 @@
 bool JSObject::isExtensible(JSObject* obj, ExecState* exec)
 {
-    return obj->isExtensibleImpl(exec->vm());
+    return obj->isStructureExtensible(exec->vm());
 }
 

trunk/Source/JavaScriptCore/runtime/JSObject.h

@@ -754 +754 @@
 private:
     NonPropertyTransition suggestedArrayStorageTransition(VM&) const;
-    ALWAYS_INLINE bool isExtensibleImpl(VM& vm) { return isStructureExtensible(vm); }
 public:
     // You should only call isStructureExtensible() when:

trunk/Source/JavaScriptCore/runtime/JSString.h

@@ -688 +688 @@
 {
     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()));
@@ -695 +697 @@
     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;
     }
@@ -704 +708 @@
 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;
     }

trunk/Source/JavaScriptCore/runtime/StringObject.cpp

@@ -115 +115 @@
         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);