Crash when BytecodeGenerator::emitJump calls Label::bind on null pointer.
<https://webkit.org/b/124508>
Reviewed by Oliver Hunt.
Source/JavaScriptCore:
The issue is that BreakNode::emitBytecode() is holding onto a LabelScope
pointer from the BytecodeGenerator's m_localScopes vector, and then it
calls emitPopScopes(). emitPopScopes() may do finally clause handling
which will require the m_localScopes to be cloned so that it can change
the local scopes for the finally block, and then restore it after
handling the finally clause. These modifications of the m_localScopes
vector will result in the LabelScope pointer in BreakNode::emitBytecode()
becoming stale, thereby causing the crash.
The same issue applies to the ContinueNode as well.
The fix is to use the existing LabelScopePtr abstraction instead of raw
LabelScope pointers. The LabelScopePtr is resilient to the underlying
vector re-allocating its backing store.
I also changed the LabelScopePtr constructor that takes a LabelScopeStore
to expect a reference to the owner store instead of a pointer because the
owner store should never be a null pointer.
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::newLabelScope):
(JSC::BytecodeGenerator::breakTarget):
(JSC::BytecodeGenerator::continueTarget):
* bytecompiler/BytecodeGenerator.h:
* bytecompiler/LabelScope.h:
(JSC::LabelScopePtr::LabelScopePtr):
(JSC::LabelScopePtr::operator bool):
(JSC::LabelScopePtr::null):
* bytecompiler/NodesCodegen.cpp:
(JSC::ContinueNode::trivialTarget):
(JSC::ContinueNode::emitBytecode):
(JSC::BreakNode::trivialTarget):
(JSC::BreakNode::emitBytecode):
LayoutTests:
* js/regress-124508-expected.txt: Added.
* js/regress-124508.html: Added.
* js/script-tests/regress-124508.js: Added.
(function_0):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@166107 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 617d58a..2bbdd2d 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,44 @@
+2014-03-21 Mark Lam <mark.lam@apple.com>
+
+ Crash when BytecodeGenerator::emitJump calls Label::bind on null pointer.
+ <https://webkit.org/b/124508>
+
+ Reviewed by Oliver Hunt.
+
+ The issue is that BreakNode::emitBytecode() is holding onto a LabelScope
+ pointer from the BytecodeGenerator's m_localScopes vector, and then it
+ calls emitPopScopes(). emitPopScopes() may do finally clause handling
+ which will require the m_localScopes to be cloned so that it can change
+ the local scopes for the finally block, and then restore it after
+ handling the finally clause. These modifications of the m_localScopes
+ vector will result in the LabelScope pointer in BreakNode::emitBytecode()
+ becoming stale, thereby causing the crash.
+
+ The same issue applies to the ContinueNode as well.
+
+ The fix is to use the existing LabelScopePtr abstraction instead of raw
+ LabelScope pointers. The LabelScopePtr is resilient to the underlying
+ vector re-allocating its backing store.
+
+ I also changed the LabelScopePtr constructor that takes a LabelScopeStore
+ to expect a reference to the owner store instead of a pointer because the
+ owner store should never be a null pointer.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::newLabelScope):
+ (JSC::BytecodeGenerator::breakTarget):
+ (JSC::BytecodeGenerator::continueTarget):
+ * bytecompiler/BytecodeGenerator.h:
+ * bytecompiler/LabelScope.h:
+ (JSC::LabelScopePtr::LabelScopePtr):
+ (JSC::LabelScopePtr::operator bool):
+ (JSC::LabelScopePtr::null):
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::ContinueNode::trivialTarget):
+ (JSC::ContinueNode::emitBytecode):
+ (JSC::BreakNode::trivialTarget):
+ (JSC::BreakNode::emitBytecode):
+
2014-03-21 Mark Hahnenberg <mhahnenberg@apple.com>
6% SunSpider commandline regression due to r165940
diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
index cf7f2f6..1535920 100644
--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -580,7 +580,7 @@
// Allocate new label scope.
LabelScope scope(type, name, scopeDepth(), newLabel(), type == LabelScope::Loop ? newLabel() : PassRefPtr<Label>()); // Only loops have continue targets.
m_labelScopes.append(scope);
- return LabelScopePtr(&m_labelScopes, m_labelScopes.size() - 1);
+ return LabelScopePtr(m_labelScopes, m_labelScopes.size() - 1);
}
PassRefPtr<Label> BytecodeGenerator::newLabel()
@@ -1989,7 +1989,7 @@
m_finallyDepth--;
}
-LabelScope* BytecodeGenerator::breakTarget(const Identifier& name)
+LabelScopePtr BytecodeGenerator::breakTarget(const Identifier& name)
{
// Reclaim free label scopes.
//
@@ -2005,7 +2005,7 @@
}
if (!m_labelScopes.size())
- return 0;
+ return LabelScopePtr::null();
// We special-case the following, which is a syntax error in Firefox:
// label:
@@ -2015,55 +2015,55 @@
LabelScope* scope = &m_labelScopes[i];
if (scope->type() != LabelScope::NamedLabel) {
ASSERT(scope->breakTarget());
- return scope;
+ return LabelScopePtr(m_labelScopes, i);
}
}
- return 0;
+ return LabelScopePtr::null();
}
for (int i = m_labelScopes.size() - 1; i >= 0; --i) {
LabelScope* scope = &m_labelScopes[i];
if (scope->name() && *scope->name() == name) {
ASSERT(scope->breakTarget());
- return scope;
+ return LabelScopePtr(m_labelScopes, i);
}
}
- return 0;
+ return LabelScopePtr::null();
}
-LabelScope* BytecodeGenerator::continueTarget(const Identifier& name)
+LabelScopePtr BytecodeGenerator::continueTarget(const Identifier& name)
{
// Reclaim free label scopes.
while (m_labelScopes.size() && !m_labelScopes.last().refCount())
m_labelScopes.removeLast();
if (!m_labelScopes.size())
- return 0;
+ return LabelScopePtr::null();
if (name.isEmpty()) {
for (int i = m_labelScopes.size() - 1; i >= 0; --i) {
LabelScope* scope = &m_labelScopes[i];
if (scope->type() == LabelScope::Loop) {
ASSERT(scope->continueTarget());
- return scope;
+ return LabelScopePtr(m_labelScopes, i);
}
}
- return 0;
+ return LabelScopePtr::null();
}
// Continue to the loop nested nearest to the label scope that matches
// 'name'.
- LabelScope* result = 0;
+ LabelScopePtr result = LabelScopePtr::null();
for (int i = m_labelScopes.size() - 1; i >= 0; --i) {
LabelScope* scope = &m_labelScopes[i];
if (scope->type() == LabelScope::Loop) {
ASSERT(scope->continueTarget());
- result = scope;
+ result = LabelScopePtr(m_labelScopes, i);
}
if (scope->name() && *scope->name() == name)
- return result; // may be 0
+ return result; // may be null.
}
- return 0;
+ return LabelScopePtr::null();
}
void BytecodeGenerator::emitComplexPopScopes(ControlFlowContext* topScope, ControlFlowContext* bottomScope)
diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
index 2c5555e..0113b80 100644
--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
@@ -451,8 +451,8 @@
m_forInContextStack.removeLast();
}
- LabelScope* breakTarget(const Identifier&);
- LabelScope* continueTarget(const Identifier&);
+ LabelScopePtr breakTarget(const Identifier&);
+ LabelScopePtr continueTarget(const Identifier&);
void beginSwitch(RegisterID*, SwitchInfo::SwitchType);
void endSwitch(uint32_t clauseCount, RefPtr<Label>*, ExpressionNode**, Label* defaultLabel, int32_t min, int32_t range);
diff --git a/Source/JavaScriptCore/bytecompiler/LabelScope.h b/Source/JavaScriptCore/bytecompiler/LabelScope.h
index c231b0e..9b84cb3 100644
--- a/Source/JavaScriptCore/bytecompiler/LabelScope.h
+++ b/Source/JavaScriptCore/bytecompiler/LabelScope.h
@@ -85,8 +85,8 @@
, m_index(0)
{
}
- LabelScopePtr(LabelScopeStore* owner, size_t index)
- : m_owner(owner)
+ LabelScopePtr(LabelScopeStore& owner, size_t index)
+ : m_owner(&owner)
, m_index(index)
{
m_owner->at(index).ref();
@@ -117,11 +117,15 @@
m_owner->at(m_index).deref();
}
+ bool operator!() const { return !m_owner; }
+
LabelScope& operator*() { ASSERT(m_owner); return m_owner->at(m_index); }
LabelScope* operator->() { ASSERT(m_owner); return &m_owner->at(m_index); }
const LabelScope& operator*() const { ASSERT(m_owner); return m_owner->at(m_index); }
const LabelScope* operator->() const { ASSERT(m_owner); return &m_owner->at(m_index); }
+ static LabelScopePtr null() { return LabelScopePtr(); }
+
private:
LabelScopeStore* m_owner;
size_t m_index;
diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
index 81e0eb1..5f06034 100644
--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
@@ -1955,7 +1955,7 @@
if (generator.shouldEmitDebugHooks())
return 0;
- LabelScope* scope = generator.continueTarget(m_ident);
+ LabelScopePtr scope = generator.continueTarget(m_ident);
ASSERT(scope);
if (generator.scopeDepth() != scope->scopeDepth())
@@ -1968,7 +1968,7 @@
{
generator.emitDebugHook(WillExecuteStatement, firstLine(), startOffset(), lineStartOffset());
- LabelScope* scope = generator.continueTarget(m_ident);
+ LabelScopePtr scope = generator.continueTarget(m_ident);
ASSERT(scope);
generator.emitPopScopes(scope->scopeDepth());
@@ -1982,7 +1982,7 @@
if (generator.shouldEmitDebugHooks())
return 0;
- LabelScope* scope = generator.breakTarget(m_ident);
+ LabelScopePtr scope = generator.breakTarget(m_ident);
ASSERT(scope);
if (generator.scopeDepth() != scope->scopeDepth())
@@ -1995,7 +1995,7 @@
{
generator.emitDebugHook(WillExecuteStatement, firstLine(), startOffset(), lineStartOffset());
- LabelScope* scope = generator.breakTarget(m_ident);
+ LabelScopePtr scope = generator.breakTarget(m_ident);
ASSERT(scope);
generator.emitPopScopes(scope->scopeDepth());
