<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[185932] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/185932">185932</a></dd>
<dt>Author</dt> <dd>msaboff@apple.com</dd>
<dt>Date</dt> <dd>2015-06-24 15:50:44 -0700 (Wed, 24 Jun 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Crash on gog.com due to PolymorphicCallNode's having stale references to CallLinkInfo
https://bugs.webkit.org/show_bug.cgi?id=146285
Reviewed by Filip Pizlo.
CallLinkInfo's contain a RefPtr to a PolymorphicCallStubRoutine, named stub, which contains
a collection of PolymorphicCallNode. Those PolymorphicCallNodes have a reference back to the
CallLinkInfo. When a CallLinkInfo replaces or clears "stub", the ref count of the
PolymorphicCallStubRoutine is decremented as expected, but since it inherits from
GCAwareJITStubRoutine, it isn't actually deleted until GC. In the mean time, the original
CallLinkInfo can go away. If PolymorphicCallNode::unlink() is called at that point,
it will try to unlink a now deleted CallLinkInfo and crash as a result.
The fix is to clear the CallLinkInfo references from any PolymorphicCallNode objects when
when we set a new stub or clear an existing stub for a CallLinkInfo. This is done by
calling PolymorphicCallNode::clearCallNodesFor() on the old stub.
The prior code would only call clearCallNodesFor() from the CallLinkInfo destructor.
This only took care of the last PolymorphicCallStubRoutine held in the CallLinkInfo.
Any prior PolymorphicCallStubRoutine would still have a, now bad, reference to the CallLinkInfo.
In the process I refactored CallLinkInfo from a struct to a class with proper accessors and
made all the data elements private.
* bytecode/CallLinkInfo.cpp:
(JSC::CallLinkInfo::clearStub): Updated to call PolymorphicCallStubRoutine::clearCallNodesFor()
to clear the back references to this CallLinkInfo.
* bytecode/CallLinkInfo.h:
(JSC::CallLinkInfo::~CallLinkInfo): Moved clearCallNodesFor() call to clearStub().
(JSC::CallLinkInfo::setStub): Clear any prior stub before changing to the new stub.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCallLinkInfocpp">trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCallLinkInfoh">trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (185931 => 185932)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-06-24 22:41:39 UTC (rev 185931)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-06-24 22:50:44 UTC (rev 185932)
</span><span class="lines">@@ -1,5 +1,38 @@
</span><span class="cx"> 2015-06-24 Michael Saboff <msaboff@apple.com>
</span><span class="cx">
</span><ins>+ Crash on gog.com due to PolymorphicCallNode's having stale references to CallLinkInfo
+ https://bugs.webkit.org/show_bug.cgi?id=146285
+
+ Reviewed by Filip Pizlo.
+
+ CallLinkInfo's contain a RefPtr to a PolymorphicCallStubRoutine, named stub, which contains
+ a collection of PolymorphicCallNode. Those PolymorphicCallNodes have a reference back to the
+ CallLinkInfo. When a CallLinkInfo replaces or clears "stub", the ref count of the
+ PolymorphicCallStubRoutine is decremented as expected, but since it inherits from
+ GCAwareJITStubRoutine, it isn't actually deleted until GC. In the mean time, the original
+ CallLinkInfo can go away. If PolymorphicCallNode::unlink() is called at that point,
+ it will try to unlink a now deleted CallLinkInfo and crash as a result.
+
+ The fix is to clear the CallLinkInfo references from any PolymorphicCallNode objects when
+ when we set a new stub or clear an existing stub for a CallLinkInfo. This is done by
+ calling PolymorphicCallNode::clearCallNodesFor() on the old stub.
+
+ The prior code would only call clearCallNodesFor() from the CallLinkInfo destructor.
+ This only took care of the last PolymorphicCallStubRoutine held in the CallLinkInfo.
+ Any prior PolymorphicCallStubRoutine would still have a, now bad, reference to the CallLinkInfo.
+
+ In the process I refactored CallLinkInfo from a struct to a class with proper accessors and
+ made all the data elements private.
+
+ * bytecode/CallLinkInfo.cpp:
+ (JSC::CallLinkInfo::clearStub): Updated to call PolymorphicCallStubRoutine::clearCallNodesFor()
+ to clear the back references to this CallLinkInfo.
+ * bytecode/CallLinkInfo.h:
+ (JSC::CallLinkInfo::~CallLinkInfo): Moved clearCallNodesFor() call to clearStub().
+ (JSC::CallLinkInfo::setStub): Clear any prior stub before changing to the new stub.
+
+2015-06-24 Michael Saboff <msaboff@apple.com>
+
</ins><span class="cx"> Refactor CallLinkInfo from a struct to a class
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=146292
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCallLinkInfocpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp (185931 => 185932)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp 2015-06-24 22:41:39 UTC (rev 185931)
+++ trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp 2015-06-24 22:50:44 UTC (rev 185932)
</span><span class="lines">@@ -42,6 +42,7 @@
</span><span class="cx"> if (!stub())
</span><span class="cx"> return;
</span><span class="cx">
</span><ins>+ m_stub->clearCallNodesFor(this);
</ins><span class="cx"> m_stub.clear();
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCallLinkInfoh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h (185931 => 185932)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h 2015-06-24 22:41:39 UTC (rev 185931)
+++ trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.h 2015-06-24 22:50:44 UTC (rev 185932)
</span><span class="lines">@@ -69,9 +69,6 @@
</span><span class="cx">
</span><span class="cx"> ~CallLinkInfo()
</span><span class="cx"> {
</span><del>- if (stub())
- m_stub->clearCallNodesFor(this);
-
</del><span class="cx"> clearStub();
</span><span class="cx">
</span><span class="cx"> if (isOnList())
</span><span class="lines">@@ -170,6 +167,7 @@
</span><span class="cx">
</span><span class="cx"> void setStub(PassRefPtr<PolymorphicCallStubRoutine> newStub)
</span><span class="cx"> {
</span><ins>+ clearStub();
</ins><span class="cx"> m_stub = newStub;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>