[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
barraclough at apple.com
barraclough at apple.com
Thu Apr 8 02:05:41 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 40ddeb929ab07ebb63eda673ed8c98e9eac9ab85
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Mar 1 22:14:22 2010 +0000
Bug 35537 - put_by_id does will incorrectly cache writes where a specific value exists,
where at the point of caching the same value is being written.
Reviewed by Oliver Hunt.
JavaScriptCore:
When performing a put_by_id that is replacing a property already present on the object,
there are three interesting cases regarding the state of the specific value:
(1) No specific value set - nothing to do, leave the structure in it's current state,
can cache.
(2) A specific value was set, the new put is not of a specified value (i.e. function),
or is of a different specific value - in these cases we need to perform a despecifying
transition to clear the specific value in the structure, but having done so this is a
normal property so as such we can again cache normally.
(3) A specific value was set, and we are overwriting with the same value - in these cases
leave the structure unchanged, but since a specific value is set we cannot cache this
put (we would need the JIT to dynamically check the value being written matched).
Unfortunately, the current behaviour does not match this. the checks for a specific value
being present & the value matching are combined in such a way that in case (2), above we
will unnecessarily prevent the transition being cached, but in case (3) we will incorrectly
fail to prevent caching.
The bug exposes itself if multiple puts of the same specific value are performed to a
property, and erroneously the put is allowed to be cached by the JIT. Method checks may be
generated caching calls of this structure. Subsequent puts performed from JIT code may
write different values without triggering a despecify transition, and as such cached method
checks will continue to pass, despite the value having changed.
* runtime/JSObject.h:
(JSC::JSObject::putDirectInternal):
LayoutTests:
Add test case.
* fast/js/method-check-expected.txt:
* fast/js/script-tests/method-check.js:
(addOne):
(addOneHundred):
(totalizer.makeCall):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@55379 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 4140b74..345b879 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,37 @@
+2010-03-01 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Bug 35537 - put_by_id does will incorrectly cache writes where a specific value exists,
+ where at the point of caching the same value is being written.
+
+ When performing a put_by_id that is replacing a property already present on the object,
+ there are three interesting cases regarding the state of the specific value:
+
+ (1) No specific value set - nothing to do, leave the structure in it's current state,
+ can cache.
+ (2) A specific value was set, the new put is not of a specified value (i.e. function),
+ or is of a different specific value - in these cases we need to perform a despecifying
+ transition to clear the specific value in the structure, but having done so this is a
+ normal property so as such we can again cache normally.
+ (3) A specific value was set, and we are overwriting with the same value - in these cases
+ leave the structure unchanged, but since a specific value is set we cannot cache this
+ put (we would need the JIT to dynamically check the value being written matched).
+
+ Unfortunately, the current behaviour does not match this. the checks for a specific value
+ being present & the value matching are combined in such a way that in case (2), above we
+ will unnecessarily prevent the transition being cached, but in case (3) we will incorrectly
+ fail to prevent caching.
+
+ The bug exposes itself if multiple puts of the same specific value are performed to a
+ property, and erroneously the put is allowed to be cached by the JIT. Method checks may be
+ generated caching calls of this structure. Subsequent puts performed from JIT code may
+ write different values without triggering a despecify transition, and as such cached method
+ checks will continue to pass, despite the value having changed.
+
+ * runtime/JSObject.h:
+ (JSC::JSObject::putDirectInternal):
+
2010-03-01 Tor Arne Vestbø <tor.arne.vestbo at nokia.com>
Reviewed by Simon Hausmann.
@@ -685,7 +719,7 @@
another rope node. A low bit in each pointer is used to distinguish between
rope & string elements, in a fashion similar to the recently-removed
PtrAndFlags class (see https://bugs.webkit.org/show_bug.cgi?id=33731 ). Again,
- this causes a problem for Leaks – refactor to remove the magic pointer
+ this causes a problem for Leaks - refactor to remove the magic pointer
mangling.
Move Rope out from JSString.h and rename to URopeImpl, to match UStringImpl.
@@ -8161,7 +8195,7 @@ The very last cell in the block is not allocated -- should not be marked.
Fix branchDouble behaviour on ARM THUMB2 JIT.
The ARMv7 JIT is currently using ARMv7Assembler::ConditionEQ to branch
- for DoubleEqualOrUnordered, however this is incorrect – ConditionEQ won't
+ for DoubleEqualOrUnordered, however this is incorrect - ConditionEQ won't
branch on unordered operands. Similarly, DoubleLessThanOrUnordered &
DoubleLessThanOrEqualOrUnordered use ARMv7Assembler::ConditionLO &
ARMv7Assembler::ConditionLS, whereas they should be using
diff --git a/JavaScriptCore/runtime/JSObject.h b/JavaScriptCore/runtime/JSObject.h
index 2b31a65..d8c7396 100644
--- a/JavaScriptCore/runtime/JSObject.h
+++ b/JavaScriptCore/runtime/JSObject.h
@@ -436,12 +436,20 @@ inline void JSObject::putDirectInternal(const Identifier& propertyName, JSValue
JSCell* currentSpecificFunction;
size_t offset = m_structure->get(propertyName, currentAttributes, currentSpecificFunction);
if (offset != WTF::notFound) {
+ // If there is currently a specific function, and there now either isn't,
+ // or the new value is different, then despecify.
if (currentSpecificFunction && (specificFunction != currentSpecificFunction))
m_structure->despecifyDictionaryFunction(propertyName);
if (checkReadOnly && currentAttributes & ReadOnly)
return;
putDirectOffset(offset, value);
- if (!specificFunction && !currentSpecificFunction)
+ // At this point, the objects structure only has a specific value set if previously there
+ // had been one set, and if the new value being specified is the same (otherwise we would
+ // have despecified, above). So, if currentSpecificFunction is not set, or if the new
+ // value is different (or there is no new value), then the slot now has no value - and
+ // as such it is cachable.
+ // If there was previously a value, and the new value is the same, then we cannot cache.
+ if (!currentSpecificFunction || (specificFunction != currentSpecificFunction))
slot.setExistingProperty(this, offset);
return;
}
@@ -468,7 +476,8 @@ inline void JSObject::putDirectInternal(const Identifier& propertyName, JSValue
ASSERT(offset < structure->propertyStorageCapacity());
setStructure(structure.release());
putDirectOffset(offset, value);
- // See comment on setNewProperty call below.
+ // This is a new property; transitions with specific values are not currently cachable,
+ // so leave the slot in an uncachable state.
if (!specificFunction)
slot.setNewProperty(this, offset);
return;
@@ -481,14 +490,28 @@ inline void JSObject::putDirectInternal(const Identifier& propertyName, JSValue
if (checkReadOnly && currentAttributes & ReadOnly)
return;
- if (currentSpecificFunction && (specificFunction != currentSpecificFunction)) {
+ // There are three possibilities here:
+ // (1) There is an existing specific value set, and we're overwriting with *the same value*.
+ // * Do nothing – no need to despecify, but that means we can't cache (a cached
+ // put could write a different value). Leave the slot in an uncachable state.
+ // (2) There is a specific value currently set, but we're writing a different value.
+ // * First, we have to despecify. Having done so, this is now a regular slot
+ // with no specific value, so go ahead & cache like normal.
+ // (3) Normal case, there is no specific value set.
+ // * Go ahead & cache like normal.
+ if (currentSpecificFunction) {
+ // case (1) Do the put, then return leaving the slot uncachable.
+ if (specificFunction == currentSpecificFunction) {
+ putDirectOffset(offset, value);
+ return;
+ }
+ // case (2) Despecify, fall through to (3).
setStructure(Structure::despecifyFunctionTransition(m_structure, propertyName));
- putDirectOffset(offset, value);
- // Function transitions are not currently cachable, so leave the slot in an uncachable state.
- return;
}
- putDirectOffset(offset, value);
+
+ // case (3) set the slot, do the put, return.
slot.setExistingProperty(this, offset);
+ putDirectOffset(offset, value);
return;
}
@@ -510,7 +533,8 @@ inline void JSObject::putDirectInternal(const Identifier& propertyName, JSValue
ASSERT(offset < structure->propertyStorageCapacity());
setStructure(structure.release());
putDirectOffset(offset, value);
- // Function transitions are not currently cachable, so leave the slot in an uncachable state.
+ // This is a new property; transitions with specific values are not currently cachable,
+ // so leave the slot in an uncachable state.
if (!specificFunction)
slot.setNewProperty(this, offset);
}
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 0874d12..36f68d9 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2010-03-01 Gavin Barraclough <barraclough at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Bug 35537 - put_by_id does will incorrectly cache writes where a specific value exists,
+ where at the point of caching the same value is being written.
+
+ Add test case.
+
+ * fast/js/method-check-expected.txt:
+ * fast/js/script-tests/method-check.js:
+ (addOne):
+ (addOneHundred):
+ (totalizer.makeCall):
+
2010-03-01 Kenneth Russell <kbr at google.com>
Reviewed by Oliver Hunt.
diff --git a/LayoutTests/fast/js/method-check-expected.txt b/LayoutTests/fast/js/method-check-expected.txt
index c45fbff..cc730d5 100644
--- a/LayoutTests/fast/js/method-check-expected.txt
+++ b/LayoutTests/fast/js/method-check-expected.txt
@@ -1,8 +1,9 @@
-This test yields PASS, if malloc does not reuse the memory address for the structure of String prototype
+Test method-check related bugs
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS total is 200
PASS successfullyParsed is true
TEST COMPLETE
diff --git a/LayoutTests/fast/js/script-tests/method-check.js b/LayoutTests/fast/js/script-tests/method-check.js
index ed62713..a4f8bdf 100644
--- a/LayoutTests/fast/js/script-tests/method-check.js
+++ b/LayoutTests/fast/js/script-tests/method-check.js
@@ -1,9 +1,10 @@
description(
-"This test yields PASS, if malloc does not reuse the memory address for the structure of String prototype"
+"Test method-check related bugs"
);
function func2() { }
+// This test yields PASS, if malloc does not reuse the memory address for the structure of String prototype
function func()
{
String.prototype.a = function() { }
@@ -30,4 +31,26 @@ function func()
func()
func()
+// Test that method caching correctly invalidates (doesn't incorrectly continue to call a previously cached function).
+var total = 0;
+function addOne()
+{
+ ++total;
+}
+function addOneHundred()
+{
+ total+=100;
+}
+var totalizer = {
+ makeCall: function(callback)
+ {
+ this.callback = callback;
+ this.callback();
+ }
+};
+for (var i=0; i<100; ++i)
+ totalizer.makeCall(addOne);
+totalizer.makeCall(addOneHundred);
+shouldBe('total', '200');
+
var successfullyParsed = true;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list