<!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>[185719] trunk/Source/WebCore</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/185719">185719</a></dd>
<dt>Author</dt> <dd>benjamin@webkit.org</dd>
<dt>Date</dt> <dd>2015-06-18 13:29:37 -0700 (Thu, 18 Jun 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[CSS JIT][ARMv7] The pseudo element early exit trashes <a href="http://trac.webkit.org/projects/webkit/changeset/6">r6</a>
https://bugs.webkit.org/show_bug.cgi?id=146078
Patch by Benjamin Poulain <bpoulain@apple.com> on 2015-06-18
Reviewed by Alex Christensen.
The pseudo element early failure runs before we generate the prologue.
The reason is that we can often exit immediately on function entry, before
we even touch any memory.
On ARMv7, we don't have many spare registers so the MacroAssembler
uses <a href="http://trac.webkit.org/projects/webkit/changeset/6">r6</a> as a scratch register and the client code is expected to save
it.
In the early failure case, we were not pushing <a href="http://trac.webkit.org/projects/webkit/changeset/6">r6</a> before using the MacroAssembler
and its value could be trashed.
This patch push the macro assembler registers separately from the prologue.
For restoring the registers, a new function generateFunctionEnding() encapsulate
the pop() and ret().
* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::SelectorCodeGenerator::pushMacroAssemblerRegisters):
(WebCore::SelectorCompiler::SelectorCodeGenerator::popMacroAssemblerRegisters):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generatePrologue):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateEpilogue):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorChecker):
* cssjit/StackAllocator.h:
(WebCore::StackAllocator::operator=):
We have a new case for the stack allocator: some stack changes are conditional
at compile time instead of runtime. This is easy to deal with by overriding
the stack if a path is not taken at compile time.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssjitSelectorCompilercpp">trunk/Source/WebCore/cssjit/SelectorCompiler.cpp</a></li>
<li><a href="#trunkSourceWebCorecssjitStackAllocatorh">trunk/Source/WebCore/cssjit/StackAllocator.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (185718 => 185719)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-06-18 19:38:15 UTC (rev 185718)
+++ trunk/Source/WebCore/ChangeLog 2015-06-18 20:29:37 UTC (rev 185719)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2015-06-18 Benjamin Poulain <bpoulain@apple.com>
+
+ [CSS JIT][ARMv7] The pseudo element early exit trashes r6
+ https://bugs.webkit.org/show_bug.cgi?id=146078
+
+ Reviewed by Alex Christensen.
+
+ The pseudo element early failure runs before we generate the prologue.
+ The reason is that we can often exit immediately on function entry, before
+ we even touch any memory.
+
+ On ARMv7, we don't have many spare registers so the MacroAssembler
+ uses r6 as a scratch register and the client code is expected to save
+ it.
+
+ In the early failure case, we were not pushing r6 before using the MacroAssembler
+ and its value could be trashed.
+
+ This patch push the macro assembler registers separately from the prologue.
+
+ For restoring the registers, a new function generateFunctionEnding() encapsulate
+ the pop() and ret().
+
+ * cssjit/SelectorCompiler.cpp:
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::pushMacroAssemblerRegisters):
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::popMacroAssemblerRegisters):
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::generatePrologue):
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::generateEpilogue):
+ (WebCore::SelectorCompiler::SelectorCodeGenerator::generateSelectorChecker):
+
+ * cssjit/StackAllocator.h:
+ (WebCore::StackAllocator::operator=):
+ We have a new case for the stack allocator: some stack changes are conditional
+ at compile time instead of runtime. This is easy to deal with by overriding
+ the stack if a path is not taken at compile time.
+
</ins><span class="cx"> 2015-06-17 Conrad Shultz <conrad_shultz@apple.com>
</span><span class="cx">
</span><span class="cx"> REGRESSION: js/dom/navigator-plugins-crash.html asserts a lot
</span></span></pre></div>
<a id="trunkSourceWebCorecssjitSelectorCompilercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (185718 => 185719)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2015-06-18 19:38:15 UTC (rev 185718)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2015-06-18 20:29:37 UTC (rev 185719)
</span><span class="lines">@@ -340,8 +340,11 @@
</span><span class="cx"> Assembler::Jump modulo(JSC::MacroAssembler::ResultCondition, Assembler::RegisterID inputDividend, int divisor);
</span><span class="cx"> void moduloIsZero(Assembler::JumpList& failureCases, Assembler::RegisterID inputDividend, int divisor);
</span><span class="cx">
</span><ins>+ void pushMacroAssemblerRegisters();
+ void popMacroAssemblerRegisters(StackAllocator&);
</ins><span class="cx"> bool generatePrologue();
</span><del>- void generateEpilogue();
</del><ins>+ void generateEpilogue(StackAllocator&);
+ StackAllocator::StackReferenceVector m_macroAssemblerRegistersStackReferences;
</ins><span class="cx"> StackAllocator::StackReferenceVector m_prologueStackReferences;
</span><span class="cx">
</span><span class="cx"> Assembler m_assembler;
</span><span class="lines">@@ -1679,6 +1682,25 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+inline void SelectorCodeGenerator::pushMacroAssemblerRegisters()
+{
+#if CPU(ARM_THUMB2)
+ // r6 is tempRegister in RegisterAllocator.h and addressTempRegister in MacroAssemblerARMv7.h and must be preserved by the callee.
+ Vector<JSC::MacroAssembler::RegisterID, 1> macroAssemblerRegisters({ JSC::ARMRegisters::r6 });
+ m_macroAssemblerRegistersStackReferences = m_stackAllocator.push(macroAssemblerRegisters);
+#endif
+}
+
+inline void SelectorCodeGenerator::popMacroAssemblerRegisters(StackAllocator& stackAllocator)
+{
+#if CPU(ARM_THUMB2)
+ Vector<JSC::MacroAssembler::RegisterID, 1> macroAssemblerRegisters({ JSC::ARMRegisters::r6 });
+ stackAllocator.pop(m_macroAssemblerRegistersStackReferences, macroAssemblerRegisters);
+#else
+ UNUSED_PARAM(stackAllocator);
+#endif
+}
+
</ins><span class="cx"> inline bool SelectorCodeGenerator::generatePrologue()
</span><span class="cx"> {
</span><span class="cx"> #if CPU(ARM64)
</span><span class="lines">@@ -1688,10 +1710,8 @@
</span><span class="cx"> m_prologueStackReferences = m_stackAllocator.push(prologueRegisters);
</span><span class="cx"> return true;
</span><span class="cx"> #elif CPU(ARM_THUMB2)
</span><del>- Vector<JSC::MacroAssembler::RegisterID, 2> prologueRegisters;
</del><ins>+ Vector<JSC::MacroAssembler::RegisterID, 1> prologueRegisters;
</ins><span class="cx"> prologueRegisters.append(JSC::ARMRegisters::lr);
</span><del>- // r6 is tempRegister in RegisterAllocator.h and addressTempRegister in MacroAssemblerARMv7.h and must be preserved by the callee.
- prologueRegisters.append(JSC::ARMRegisters::r6);
</del><span class="cx"> m_prologueStackReferences = m_stackAllocator.push(prologueRegisters);
</span><span class="cx"> return true;
</span><span class="cx"> #elif CPU(X86_64) && CSS_SELECTOR_JIT_DEBUGGING
</span><span class="lines">@@ -1703,22 +1723,19 @@
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-inline void SelectorCodeGenerator::generateEpilogue()
</del><ins>+inline void SelectorCodeGenerator::generateEpilogue(StackAllocator& stackAllocator)
</ins><span class="cx"> {
</span><span class="cx"> #if CPU(ARM64)
</span><del>- Vector<JSC::MacroAssembler::RegisterID, 2> prologueRegisters;
- prologueRegisters.append(JSC::ARM64Registers::lr);
- prologueRegisters.append(JSC::ARM64Registers::fp);
- m_stackAllocator.pop(m_prologueStackReferences, prologueRegisters);
</del><ins>+ Vector<JSC::MacroAssembler::RegisterID, 2> prologueRegisters({ JSC::ARM64Registers::lr, JSC::ARM64Registers::fp });
+ stackAllocator.pop(m_prologueStackReferences, prologueRegisters);
</ins><span class="cx"> #elif CPU(ARM_THUMB2)
</span><del>- Vector<JSC::MacroAssembler::RegisterID, 2> prologueRegisters;
- prologueRegisters.append(JSC::ARMRegisters::lr);
- prologueRegisters.append(JSC::ARMRegisters::r6);
- m_stackAllocator.pop(m_prologueStackReferences, prologueRegisters);
</del><ins>+ Vector<JSC::MacroAssembler::RegisterID, 1> prologueRegister({ JSC::ARMRegisters::lr });
+ stackAllocator.pop(m_prologueStackReferences, prologueRegister);
</ins><span class="cx"> #elif CPU(X86_64) && CSS_SELECTOR_JIT_DEBUGGING
</span><del>- Vector<JSC::MacroAssembler::RegisterID, 1> prologueRegister;
- prologueRegister.append(callFrameRegister);
- m_stackAllocator.pop(m_prologueStackReferences, prologueRegister);
</del><ins>+ Vector<JSC::MacroAssembler::RegisterID, 1> prologueRegister({ callFrameRegister });
+ stackAllocator.pop(m_prologueStackReferences, prologueRegister);
+#else
+ UNUSED_PARAM(stackAllocator);
</ins><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1734,6 +1751,9 @@
</span><span class="cx">
</span><span class="cx"> void SelectorCodeGenerator::generateSelectorChecker()
</span><span class="cx"> {
</span><ins>+ pushMacroAssemblerRegisters();
+ StackAllocator earlyFailureStack = m_stackAllocator;
+
</ins><span class="cx"> Assembler::JumpList failureOnFunctionEntry;
</span><span class="cx"> // Test selector's pseudo element equals to requested PseudoId.
</span><span class="cx"> if (m_selectorContext != SelectorContext::QuerySelector && m_functionType == FunctionType::SelectorCheckerWithCheckingContext) {
</span><span class="lines">@@ -1760,6 +1780,7 @@
</span><span class="cx"> unsigned maximumBacktrackingAllocations = 8;
</span><span class="cx"> if (m_selectorFragments.stackRequirements > maximumBacktrackingAllocations) {
</span><span class="cx"> m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
</span><ins>+ popMacroAssemblerRegisters(m_stackAllocator);
</ins><span class="cx"> m_assembler.ret();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="lines">@@ -1818,9 +1839,13 @@
</span><span class="cx">
</span><span class="cx"> if (m_functionType == FunctionType::SimpleSelectorChecker) {
</span><span class="cx"> if (temporaryStackBase == m_stackAllocator.stackTop() && !reservedCalleeSavedRegisters && !needsEpilogue) {
</span><ins>+ StackAllocator successStack = m_stackAllocator;
+ StackAllocator failureStack = m_stackAllocator;
+
</ins><span class="cx"> ASSERT(!m_selectorFragments.stackRequirements);
</span><span class="cx"> // Success.
</span><span class="cx"> m_assembler.move(Assembler::TrustedImm32(1), returnRegister);
</span><ins>+ popMacroAssemblerRegisters(successStack);
</ins><span class="cx"> m_assembler.ret();
</span><span class="cx">
</span><span class="cx"> // Failure.
</span><span class="lines">@@ -1828,8 +1853,12 @@
</span><span class="cx"> if (!failureCases.empty()) {
</span><span class="cx"> failureCases.link(&m_assembler);
</span><span class="cx"> m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
</span><ins>+ popMacroAssemblerRegisters(failureStack);
</ins><span class="cx"> m_assembler.ret();
</span><del>- }
</del><ins>+ } else
+ failureStack = successStack;
+
+ m_stackAllocator.merge(WTF::move(successStack), WTF::move(failureStack));
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="lines">@@ -1849,16 +1878,22 @@
</span><span class="cx"> m_stackAllocator.popAndDiscardUpTo(temporaryStackBase);
</span><span class="cx"> if (reservedCalleeSavedRegisters)
</span><span class="cx"> m_stackAllocator.pop(calleeSavedRegisterStackReferences, m_registerAllocator.restoreCalleeSavedRegisters());
</span><ins>+
+ StackAllocator successStack = m_stackAllocator;
</ins><span class="cx"> if (needsEpilogue)
</span><del>- generateEpilogue();
</del><ins>+ generateEpilogue(successStack);
+ popMacroAssemblerRegisters(successStack);
</ins><span class="cx"> m_assembler.ret();
</span><span class="cx">
</span><span class="cx"> // Early failure on function entry case.
</span><span class="cx"> if (!failureOnFunctionEntry.empty()) {
</span><span class="cx"> failureOnFunctionEntry.link(&m_assembler);
</span><span class="cx"> m_assembler.move(Assembler::TrustedImm32(0), returnRegister);
</span><ins>+ popMacroAssemblerRegisters(earlyFailureStack);
</ins><span class="cx"> m_assembler.ret();
</span><del>- }
</del><ins>+ } else
+ earlyFailureStack = successStack;
+ m_stackAllocator.merge(WTF::move(successStack), WTF::move(earlyFailureStack));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void SelectorCodeGenerator::generateSelectorCheckerExcludingPseudoElements(Assembler::JumpList& failureCases, const SelectorFragmentList& selectorFragmentList)
</span></span></pre></div>
<a id="trunkSourceWebCorecssjitStackAllocatorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/cssjit/StackAllocator.h (185718 => 185719)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/cssjit/StackAllocator.h 2015-06-18 19:38:15 UTC (rev 185718)
+++ trunk/Source/WebCore/cssjit/StackAllocator.h 2015-06-18 20:29:37 UTC (rev 185719)
</span><span class="lines">@@ -239,6 +239,15 @@
</span><span class="cx"> return JSC::MacroAssembler::Address(JSC::MacroAssembler::stackPointerRegister, offsetToStackReference(stackReference));
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ StackAllocator& operator=(const StackAllocator& other)
+ {
+ RELEASE_ASSERT(&m_assembler == &other.m_assembler);
+ m_offsetFromTop = other.m_offsetFromTop;
+ m_hasFunctionCallPadding = other.m_hasFunctionCallPadding;
+ return *this;
+ }
+
+
</ins><span class="cx"> private:
</span><span class="cx"> static unsigned stackUnitInBytes()
</span><span class="cx"> {
</span></span></pre>
</div>
</div>
</body>
</html>