Title: [144356] trunk/Source/_javascript_Core
- Revision
- 144356
- Author
- [email protected]
- Date
- 2013-02-28 13:31:21 -0800 (Thu, 28 Feb 2013)
Log Message
CodeBlock::valueProfile() has a bogus assertion
https://bugs.webkit.org/show_bug.cgi?id=111106
<rdar://problem/13131427>
Reviewed by Mark Hahnenberg.
This was just a bad assertion: m_bytecodeOffset == -1 means that the value profile is constructed but not initialized.
ValueProfile constructs itself in a safe way; you can call any method you want on a constructed but not initialized
ValueProfile. CodeBlock first constructs all ValueProfiles (by growing the ValueProfile vector) and then initializes
their m_bytecodeOffset later. This is necessary because the initialization is linking bytecode instructions to their
ValueProfiles, so at that point we don't want the ValueProfile vector to resize, which implies that we want all of
them to already be constructed. A GC can happen during this phase, and the GC may want to walk all ValueProfiles.
This is safe, but one of the ValueProfile getters (CodeBlock::valueProfile()) was asserting that any value profile
you get has had its m_bytecodeOffset initialized. This need not be the case and nothing will go wrong if it isn't.
The solution is to remove the assertion, which I believe was put there to ensure that my m_valueProfiles refactoring
a long time ago was sound: it used to be that a ValueProfile with m_bytecodeOffset == -1 was an argument profile; now
all argument profiles are in m_argumentValueProfiles instead. I think it's safe to say that this refactoring was done
soundly since it was a long time ago. So we should kill the assertion - I don't see an easy way to make the assertion
sound with respect to the GC-during-CodeBlock-construction issue, and I don't believe that the assertion is buying us
anything at this point.
* bytecode/CodeBlock.h:
(JSC::CodeBlock::valueProfile):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (144355 => 144356)
--- trunk/Source/_javascript_Core/ChangeLog 2013-02-28 21:30:00 UTC (rev 144355)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-02-28 21:31:21 UTC (rev 144356)
@@ -1,3 +1,30 @@
+2013-02-28 Filip Pizlo <[email protected]>
+
+ CodeBlock::valueProfile() has a bogus assertion
+ https://bugs.webkit.org/show_bug.cgi?id=111106
+ <rdar://problem/13131427>
+
+ Reviewed by Mark Hahnenberg.
+
+ This was just a bad assertion: m_bytecodeOffset == -1 means that the value profile is constructed but not initialized.
+ ValueProfile constructs itself in a safe way; you can call any method you want on a constructed but not initialized
+ ValueProfile. CodeBlock first constructs all ValueProfiles (by growing the ValueProfile vector) and then initializes
+ their m_bytecodeOffset later. This is necessary because the initialization is linking bytecode instructions to their
+ ValueProfiles, so at that point we don't want the ValueProfile vector to resize, which implies that we want all of
+ them to already be constructed. A GC can happen during this phase, and the GC may want to walk all ValueProfiles.
+ This is safe, but one of the ValueProfile getters (CodeBlock::valueProfile()) was asserting that any value profile
+ you get has had its m_bytecodeOffset initialized. This need not be the case and nothing will go wrong if it isn't.
+
+ The solution is to remove the assertion, which I believe was put there to ensure that my m_valueProfiles refactoring
+ a long time ago was sound: it used to be that a ValueProfile with m_bytecodeOffset == -1 was an argument profile; now
+ all argument profiles are in m_argumentValueProfiles instead. I think it's safe to say that this refactoring was done
+ soundly since it was a long time ago. So we should kill the assertion - I don't see an easy way to make the assertion
+ sound with respect to the GC-during-CodeBlock-construction issue, and I don't believe that the assertion is buying us
+ anything at this point.
+
+ * bytecode/CodeBlock.h:
+ (JSC::CodeBlock::valueProfile):
+
2013-02-27 Filip Pizlo <[email protected]>
DFG CFA should leave behind information in Edge that says if the Edge's type check is proven to succeed
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (144355 => 144356)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2013-02-28 21:30:00 UTC (rev 144355)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2013-02-28 21:31:21 UTC (rev 144356)
@@ -605,12 +605,7 @@
}
unsigned numberOfValueProfiles() { return m_valueProfiles.size(); }
- ValueProfile* valueProfile(int index)
- {
- ValueProfile* result = &m_valueProfiles[index];
- ASSERT(result->m_bytecodeOffset != -1);
- return result;
- }
+ ValueProfile* valueProfile(int index) { return &m_valueProfiles[index]; }
ValueProfile* valueProfileForBytecodeOffset(int bytecodeOffset)
{
ValueProfile* result = binarySearch<ValueProfile, int>(
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
