Reviewers: Benedikt Meurer,
Message:
PTAL.
We can't add a regression test because... long story :-)
But I have an idea how to add instrumentation that would have found this
issue.
https://codereview.chromium.org/302063004/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (left):
https://codereview.chromium.org/302063004/diff/20001/src/hydrogen.cc#oldcode1681
src/hydrogen.cc:1681: // SMIs and heap number, so it is sufficient to do
a SMI check here).
This comment is a lie! Unused entries in the cache are |undefined|.
https://codereview.chromium.org/302063004/diff/20001/src/hydrogen.cc#oldcode1686
src/hydrogen.cc:1686: // Check if values of key and object match.
This entire block is actually unchanged, just indented one level deeper.
Description:
BuildNumberToString: Check for undefined keys in the cache
BUG=chromium:368114
LOG=y
Please review this at https://codereview.chromium.org/302063004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+20, -14 lines):
M src/hydrogen.cc
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
3b77c73c0d0357e4e6d660ba03a3735326f284ad..a2aaa27e854e8e492f7c2b51131adcd8bc6fc643
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -1677,26 +1677,32 @@ HValue* HGraphBuilder::BuildNumberToString(HValue*
object, Type* type) {
static_cast<HValue*>(NULL),
FAST_ELEMENTS, ALLOW_RETURN_HOLE);
- // Check if key is a heap number (the number string cache contains
only
- // SMIs and heap number, so it is sufficient to do a SMI check
here).
+ // Check if the key is a heap number and compare it with the
object.
IfBuilder if_keyisnotsmi(this);
HValue* keyisnotsmi = if_keyisnotsmi.IfNot<HIsSmiAndBranch>(key);
if_keyisnotsmi.Then();
{
- // Check if values of key and object match.
- IfBuilder if_keyeqobject(this);
- if_keyeqobject.If<HCompareNumericAndBranch>(
- Add<HLoadNamedField>(key, keyisnotsmi,
- HObjectAccess::ForHeapNumberValue()),
- Add<HLoadNamedField>(object, objectisnumber,
- HObjectAccess::ForHeapNumberValue()),
- Token::EQ);
- if_keyeqobject.Then();
+ IfBuilder if_keyisheapnumber(this);
+ if_keyisheapnumber.If<HCompareMap>(
+ key, isolate()->factory()->heap_number_map());
+ if_keyisheapnumber.Then();
{
- // Make the key_index available.
- Push(key_index);
+ // Check if values of key and object match.
+ IfBuilder if_keyeqobject(this);
+ if_keyeqobject.If<HCompareNumericAndBranch>(
+ Add<HLoadNamedField>(key, keyisnotsmi,
+ HObjectAccess::ForHeapNumberValue()),
+ Add<HLoadNamedField>(object, objectisnumber,
+ HObjectAccess::ForHeapNumberValue()),
+ Token::EQ);
+ if_keyeqobject.Then();
+ {
+ // Make the key_index available.
+ Push(key_index);
+ }
+ if_keyeqobject.JoinContinuation(&found);
}
- if_keyeqobject.JoinContinuation(&found);
+ if_keyisheapnumber.JoinContinuation(&found);
}
if_keyisnotsmi.JoinContinuation(&found);
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
