[webkit-changes] [196490] trunk/Source/JavaScriptCore

[fpizlo]

Title: [196490] trunk/Source/_javascript_Core

Revision

196490

Author

fpi...@apple.com

Date

2016-02-12 11:50:49 -0800 (Fri, 12 Feb 2016)

Log Message

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.

Modified Paths

• trunk/Source/_javascript_Core/ChangeLog
• trunk/Source/_javascript_Core/runtime/JSObject.cpp
• trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp

Added Paths

• trunk/Source/_javascript_Core/tests/stress/sparse-define-empty-descriptor.js

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (196489 => 196490)

--- trunk/Source/_javascript_Core/ChangeLog	2016-02-12 19:43:10 UTC (rev 196489)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-02-12 19:50:49 UTC (rev 196490)
@@ -1,3 +1,18 @@
+2016-02-12  Filip Pizlo  <fpi...@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  <bb...@apple.com>
 
         Web Inspector: RemoteInspector's listings should include whether an AutomationTarget is paired

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (196489 => 196490)

--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2016-02-12 19:43:10 UTC (rev 196489)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2016-02-12 19:50:49 UTC (rev 196490)
@@ -1763,13 +1763,13 @@
 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);
         }

Modified: trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp (196489 => 196490)

--- trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp	2016-02-12 19:43:10 UTC (rev 196489)
+++ trunk/Source/_javascript_Core/runtime/SparseArrayValueMap.cpp	2016-02-12 19:50:49 UTC (rev 196490)
@@ -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 @@
 
 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 @@
 
 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;
 

Added: trunk/Source/_javascript_Core/tests/stress/sparse-define-empty-descriptor.js (0 => 196490)

--- trunk/Source/_javascript_Core/tests/stress/sparse-define-empty-descriptor.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/sparse-define-empty-descriptor.js	2016-02-12 19:50:49 UTC (rev 196490)
@@ -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;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes