<!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>[175653] 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/175653">175653</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2014-11-05 17:53:25 -0800 (Wed, 05 Nov 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>PutById inline caches should have a store barrier when it triggers a structure transition.
<https://webkit.org/b/138441>
Reviewed by Geoffrey Garen.
After <a href="http://trac.webkit.org/projects/webkit/changeset/174025">r174025</a>, we no longer insert DFG store barriers when the payload of a
PutById operation is not a cell. However, this can lead to a crash when we have
PutById inline cache code transitioning the structure and re-allocating the
butterfly of an old gen object. The lack of a store barrier in that inline
cache results in the old gen object not being noticed during an eden GC scan.
As a result, its newly allocated butterfly will not be kept alive, which leads
to a stale butterfly pointer and, eventually, a crash.
It is also possible that the new structure can be collected by the eden GC if
(at GC time):
1. It is in the eden gen.
2. The inline cache that installed it has been evicted.
3. There are no live eden gen objects referring to it.
The chances of this should be more rare than the butterfly re-allocation, but
it is still possible. Hence, the fix is to always add a store barrier if the
inline caches performs a structure transition.
* jit/Repatch.cpp:
(JSC::emitPutTransitionStub):
- Added store barrier code based on SpeculativeJIT::storeToWriteBarrierBuffer()'s
implementation.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (175652 => 175653)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2014-11-06 01:37:45 UTC (rev 175652)
+++ trunk/Source/JavaScriptCore/ChangeLog 2014-11-06 01:53:25 UTC (rev 175653)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2014-11-05 Mark Lam <mark.lam@apple.com>
+
+ PutById inline caches should have a store barrier when it triggers a structure transition.
+ <https://webkit.org/b/138441>
+
+ Reviewed by Geoffrey Garen.
+
+ After r174025, we no longer insert DFG store barriers when the payload of a
+ PutById operation is not a cell. However, this can lead to a crash when we have
+ PutById inline cache code transitioning the structure and re-allocating the
+ butterfly of an old gen object. The lack of a store barrier in that inline
+ cache results in the old gen object not being noticed during an eden GC scan.
+ As a result, its newly allocated butterfly will not be kept alive, which leads
+ to a stale butterfly pointer and, eventually, a crash.
+
+ It is also possible that the new structure can be collected by the eden GC if
+ (at GC time):
+ 1. It is in the eden gen.
+ 2. The inline cache that installed it has been evicted.
+ 3. There are no live eden gen objects referring to it.
+
+ The chances of this should be more rare than the butterfly re-allocation, but
+ it is still possible. Hence, the fix is to always add a store barrier if the
+ inline caches performs a structure transition.
+
+ * jit/Repatch.cpp:
+ (JSC::emitPutTransitionStub):
+ - Added store barrier code based on SpeculativeJIT::storeToWriteBarrierBuffer()'s
+ implementation.
+
</ins><span class="cx"> 2014-11-05 Gyuyoung Kim <gyuyoung.kim@samsung.com>
</span><span class="cx">
</span><span class="cx"> Use std::unique_ptr in JSClassRef and JSCallbackObject
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (175652 => 175653)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp 2014-11-06 01:37:45 UTC (rev 175652)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp 2014-11-06 01:53:25 UTC (rev 175653)
</span><span class="lines">@@ -1113,6 +1113,39 @@
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+ ScratchBuffer* scratchBuffer = nullptr;
+
+#if ENABLE(GGC)
+ MacroAssembler::Call callFlushWriteBarrierBuffer;
+ MacroAssembler::Jump ownerIsRememberedOrInEden = stubJit.jumpIfIsRememberedOrInEden(baseGPR);
+ {
+ WriteBarrierBuffer* writeBarrierBuffer = &stubJit.vm()->heap.writeBarrierBuffer();
+ stubJit.move(MacroAssembler::TrustedImmPtr(writeBarrierBuffer), scratchGPR1);
+ stubJit.load32(MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::currentIndexOffset()), scratchGPR2);
+ MacroAssembler::Jump needToFlush =
+ stubJit.branch32(MacroAssembler::AboveOrEqual, scratchGPR2, MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::capacityOffset()));
+
+ stubJit.add32(MacroAssembler::TrustedImm32(1), scratchGPR2);
+ stubJit.store32(scratchGPR2, MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::currentIndexOffset()));
+
+ stubJit.loadPtr(MacroAssembler::Address(scratchGPR1, WriteBarrierBuffer::bufferOffset()), scratchGPR1);
+ // We use an offset of -sizeof(void*) because we already added 1 to scratchGPR2.
+ stubJit.storePtr(baseGPR, MacroAssembler::BaseIndex(scratchGPR1, scratchGPR2, MacroAssembler::ScalePtr, static_cast<int32_t>(-sizeof(void*))));
+
+ MacroAssembler::Jump doneWithBarrier = stubJit.jump();
+ needToFlush.link(&stubJit);
+
+ scratchBuffer = vm->scratchBufferForSize(allocator.desiredScratchBufferSizeForCall());
+ allocator.preserveUsedRegistersToScratchBufferForCall(stubJit, scratchBuffer, scratchGPR2);
+ stubJit.setupArgumentsWithExecState(baseGPR);
+ callFlushWriteBarrierBuffer = stubJit.call();
+ allocator.restoreUsedRegistersFromScratchBufferForCall(stubJit, scratchBuffer, scratchGPR2);
+
+ doneWithBarrier.link(&stubJit);
+ }
+ ownerIsRememberedOrInEden.link(&stubJit);
+#endif
+
</ins><span class="cx"> MacroAssembler::Jump success;
</span><span class="cx"> MacroAssembler::Jump failure;
</span><span class="cx">
</span><span class="lines">@@ -1133,7 +1166,8 @@
</span><span class="cx"> slowPath.link(&stubJit);
</span><span class="cx">
</span><span class="cx"> allocator.restoreReusedRegistersByPopping(stubJit);
</span><del>- ScratchBuffer* scratchBuffer = vm->scratchBufferForSize(allocator.desiredScratchBufferSizeForCall());
</del><ins>+ if (!scratchBuffer)
+ scratchBuffer = vm->scratchBufferForSize(allocator.desiredScratchBufferSizeForCall());
</ins><span class="cx"> allocator.preserveUsedRegistersToScratchBufferForCall(stubJit, scratchBuffer, scratchGPR1);
</span><span class="cx"> #if USE(JSVALUE64)
</span><span class="cx"> stubJit.setupArgumentsWithExecState(baseGPR, MacroAssembler::TrustedImmPtr(structure), MacroAssembler::TrustedImm32(slot.cachedOffset()), valueGPR);
</span><span class="lines">@@ -1151,6 +1185,9 @@
</span><span class="cx"> patchBuffer.link(failure, failureLabel);
</span><span class="cx"> else
</span><span class="cx"> patchBuffer.link(failureCases, failureLabel);
</span><ins>+#if ENABLE(GGC)
+ patchBuffer.link(callFlushWriteBarrierBuffer, operationFlushWriteBarrierBuffer);
+#endif
</ins><span class="cx"> if (structure->outOfLineCapacity() != oldStructure->outOfLineCapacity()) {
</span><span class="cx"> patchBuffer.link(operationCall, operationReallocateStorageAndFinishPut);
</span><span class="cx"> patchBuffer.link(successInSlowPath, stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToDone));
</span></span></pre>
</div>
</div>
</body>
</html>