Fast path in JSObject::defineOwnIndexedProperty() forgets to check for the posibility...
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Feb 2016 19:50:49 +0000 (19:50 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Feb 2016 19:50:49 +0000 (19:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154175
rdar://problem/24291497

Reviewed by Geoffrey Garen.

* runtime/JSObject.cpp:
(JSC::JSObject::defineOwnIndexedProperty): Fix the bug.
* runtime/SparseArrayValueMap.cpp:
(JSC::SparseArrayValueMap::putEntry): Catch the bug sooner in debug.
(JSC::SparseArrayValueMap::putDirect):
* tests/stress/sparse-define-empty-descriptor.js: Added. This used to crash in release.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196490 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp
Source/JavaScriptCore/tests/stress/sparse-define-empty-descriptor.js [new file with mode: 0644]

index 70b7bc3..709d431 100644 (file)
@@ -1,3 +1,18 @@
+2016-02-12  Filip Pizlo  <fpizlo@apple.com>
+
+        Fast path in JSObject::defineOwnIndexedProperty() forgets to check for the posibility of a descriptor that doesn't have a value
+        https://bugs.webkit.org/show_bug.cgi?id=154175
+        rdar://problem/24291497
+
+        Reviewed by Geoffrey Garen.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::defineOwnIndexedProperty): Fix the bug.
+        * runtime/SparseArrayValueMap.cpp:
+        (JSC::SparseArrayValueMap::putEntry): Catch the bug sooner in debug.
+        (JSC::SparseArrayValueMap::putDirect):
+        * tests/stress/sparse-define-empty-descriptor.js: Added. This used to crash in release.
+
 2016-02-11  Brian Burg  <bburg@apple.com>
 
         Web Inspector: RemoteInspector's listings should include whether an AutomationTarget is paired
index 8ce9763..033b403 100644 (file)
@@ -1763,13 +1763,13 @@ void JSObject::putIndexedDescriptor(ExecState* exec, SparseArrayEntry* entryInMa
 bool JSObject::defineOwnIndexedProperty(ExecState* exec, unsigned index, const PropertyDescriptor& descriptor, bool throwException)
 {
     ASSERT(index <= MAX_ARRAY_INDEX);
-    
+
     if (!inSparseIndexingMode()) {
         // Fast case: we're putting a regular property to a regular array
         // FIXME: this will pessimistically assume that if attributes are missing then they'll default to false
         // however if the property currently exists missing attributes will override from their current 'true'
         // state (i.e. defineOwnProperty could be used to set a value without needing to entering 'SparseMode').
-        if (!descriptor.attributes()) {
+        if (!descriptor.attributes() && descriptor.value()) {
             ASSERT(!descriptor.isAccessorDescriptor());
             return putDirectIndex(exec, index, descriptor.value(), 0, throwException ? PutDirectIndexShouldThrow : PutDirectIndexShouldNotThrow);
         }
index b395984..24d5244 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011, 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2012, 2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -90,6 +90,8 @@ SparseArrayValueMap::AddResult SparseArrayValueMap::add(JSObject* array, unsigne
 
 void SparseArrayValueMap::putEntry(ExecState* exec, JSObject* array, unsigned i, JSValue value, bool shouldThrow)
 {
+    ASSERT(value);
+    
     AddResult result = add(array, i);
     SparseArrayEntry& entry = result.iterator->value;
 
@@ -108,6 +110,8 @@ void SparseArrayValueMap::putEntry(ExecState* exec, JSObject* array, unsigned i,
 
 bool SparseArrayValueMap::putDirect(ExecState* exec, JSObject* array, unsigned i, JSValue value, unsigned attributes, PutDirectIndexMode mode)
 {
+    ASSERT(value);
+    
     AddResult result = add(array, i);
     SparseArrayEntry& entry = result.iterator->value;
 
diff --git a/Source/JavaScriptCore/tests/stress/sparse-define-empty-descriptor.js b/Source/JavaScriptCore/tests/stress/sparse-define-empty-descriptor.js
new file mode 100644 (file)
index 0000000..6f9de14
--- /dev/null
@@ -0,0 +1,6 @@
+var array = [];
+array[10000000] = 42;
+Object.defineProperty(array, 10000000, {configurable: true, enumerable: true, writable: true});
+var result = array[10000000];
+if (result != 42)
+    throw "Error: bad result: " + result;