<!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>[202463] 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/202463">202463</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-06-24 17:03:44 -0700 (Fri, 24 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>B3 should die sooner if a Value has the wrong number of children
https://bugs.webkit.org/show_bug.cgi?id=159108
Reviewed by Mark Lam.
I've been looking at a bug (rdar://problem/26500743) that's about a Vector OOB crash in
ReduceStrength::rangeFor(). The only Vector accesses are to Value::m_children, and all of
the accesses in rangeFor() are for child(0) or child(1) of binary arithmetic opcodes.
Clearly those should never go out-of-bounds.
Maybe we have horrible memory corruption. Or maybe some path creates a Value with the
wrong number of children, and that path is not tested by any of our tests. This patch adds
release assertions that will catch the latter.
I've tested this a lot. It's not a regression on our benchmarks.
* b3/B3Opcode.h:
* b3/B3Value.cpp:
(JSC::B3::Value::dumpMeta):
(JSC::B3::Value::typeFor):
(JSC::B3::Value::badOpcode):
(JSC::B3::Value::checkOpcode): Deleted.
* b3/B3Value.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3Opcodeh">trunk/Source/JavaScriptCore/b3/B3Opcode.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3Valuecpp">trunk/Source/JavaScriptCore/b3/B3Value.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3Valueh">trunk/Source/JavaScriptCore/b3/B3Value.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (202462 => 202463)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-06-24 23:54:22 UTC (rev 202462)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-06-25 00:03:44 UTC (rev 202463)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2016-06-24 Filip Pizlo <fpizlo@apple.com>
+
+ B3 should die sooner if a Value has the wrong number of children
+ https://bugs.webkit.org/show_bug.cgi?id=159108
+
+ Reviewed by Mark Lam.
+
+ I've been looking at a bug (rdar://problem/26500743) that's about a Vector OOB crash in
+ ReduceStrength::rangeFor(). The only Vector accesses are to Value::m_children, and all of
+ the accesses in rangeFor() are for child(0) or child(1) of binary arithmetic opcodes.
+ Clearly those should never go out-of-bounds.
+
+ Maybe we have horrible memory corruption. Or maybe some path creates a Value with the
+ wrong number of children, and that path is not tested by any of our tests. This patch adds
+ release assertions that will catch the latter.
+
+ I've tested this a lot. It's not a regression on our benchmarks.
+
+ * b3/B3Opcode.h:
+ * b3/B3Value.cpp:
+ (JSC::B3::Value::dumpMeta):
+ (JSC::B3::Value::typeFor):
+ (JSC::B3::Value::badOpcode):
+ (JSC::B3::Value::checkOpcode): Deleted.
+ * b3/B3Value.h:
+
</ins><span class="cx"> 2016-06-24 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> [JSC] Error prototypes are called on remote scripts.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3Opcodeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3Opcode.h (202462 => 202463)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3Opcode.h 2016-06-24 23:54:22 UTC (rev 202462)
+++ trunk/Source/JavaScriptCore/b3/B3Opcode.h 2016-06-25 00:03:44 UTC (rev 202463)
</span><span class="lines">@@ -265,7 +265,7 @@
</span><span class="cx">
</span><span class="cx"> class PrintStream;
</span><span class="cx">
</span><del>-void printInternal(PrintStream&, JSC::B3::Opcode);
</del><ins>+JS_EXPORT_PRIVATE void printInternal(PrintStream&, JSC::B3::Opcode);
</ins><span class="cx">
</span><span class="cx"> } // namespace WTF
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3Valuecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3Value.cpp (202462 => 202463)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3Value.cpp 2016-06-24 23:54:22 UTC (rev 202462)
+++ trunk/Source/JavaScriptCore/b3/B3Value.cpp 2016-06-25 00:03:44 UTC (rev 202463)
</span><span class="lines">@@ -633,25 +633,6 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#if !ASSERT_DISABLED
-void Value::checkOpcode(Opcode opcode)
-{
- ASSERT(!ArgumentRegValue::accepts(opcode));
- ASSERT(!CCallValue::accepts(opcode));
- ASSERT(!CheckValue::accepts(opcode));
- ASSERT(!Const32Value::accepts(opcode));
- ASSERT(!Const64Value::accepts(opcode));
- ASSERT(!ConstDoubleValue::accepts(opcode));
- ASSERT(!ConstFloatValue::accepts(opcode));
- ASSERT(!ControlValue::accepts(opcode));
- ASSERT(!MemoryValue::accepts(opcode));
- ASSERT(!PatchpointValue::accepts(opcode));
- ASSERT(!SlotBaseValue::accepts(opcode));
- ASSERT(!UpsilonValue::accepts(opcode));
- ASSERT(!VariableValue::accepts(opcode));
-}
-#endif // !ASSERT_DISABLED
-
</del><span class="cx"> Type Value::typeFor(Opcode opcode, Value* firstChild, Value* secondChild)
</span><span class="cx"> {
</span><span class="cx"> switch (opcode) {
</span><span class="lines">@@ -729,6 +710,12 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void Value::badOpcode(Opcode opcode, unsigned numArgs)
+{
+ dataLog("Bad opcode ", opcode, " with ", numArgs, " args.\n");
+ RELEASE_ASSERT_NOT_REACHED();
+}
+
</ins><span class="cx"> } } // namespace JSC::B3
</span><span class="cx">
</span><span class="cx"> #endif // ENABLE(B3_JIT)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3Valueh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3Value.h (202462 => 202463)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3Value.h 2016-06-24 23:54:22 UTC (rev 202462)
+++ trunk/Source/JavaScriptCore/b3/B3Value.h 2016-06-25 00:03:44 UTC (rev 202463)
</span><span class="lines">@@ -235,11 +235,71 @@
</span><span class="cx"> friend class SparseCollection<Value>;
</span><span class="cx">
</span><span class="cx"> // Checks that this opcode is valid for use with B3::Value.
</span><del>-#if ASSERT_DISABLED
- static void checkOpcode(Opcode) { }
-#else
- static void checkOpcode(Opcode);
-#endif
</del><ins>+ ALWAYS_INLINE static void checkOpcode(Opcode opcode, unsigned numArgs)
+ {
+ switch (opcode) {
+ case FramePointer:
+ case Nop:
+ case Phi:
+ if (UNLIKELY(numArgs))
+ badOpcode(opcode, numArgs);
+ break;
+ case Identity:
+ case Neg:
+ case Clz:
+ case Abs:
+ case Ceil:
+ case Floor:
+ case Sqrt:
+ case SExt8:
+ case SExt16:
+ case Trunc:
+ case SExt32:
+ case ZExt32:
+ case FloatToDouble:
+ case IToD:
+ case DoubleToFloat:
+ case IToF:
+ case BitwiseCast:
+ if (UNLIKELY(numArgs != 1))
+ badOpcode(opcode, numArgs);
+ break;
+ case Add:
+ case Sub:
+ case Mul:
+ case Div:
+ case Mod:
+ case ChillDiv:
+ case ChillMod:
+ case BitAnd:
+ case BitOr:
+ case BitXor:
+ case Shl:
+ case SShr:
+ case ZShr:
+ case Equal:
+ case NotEqual:
+ case LessThan:
+ case GreaterThan:
+ case LessEqual:
+ case GreaterEqual:
+ case Above:
+ case Below:
+ case AboveEqual:
+ case BelowEqual:
+ case EqualOrUnordered:
+ if (UNLIKELY(numArgs != 2))
+ badOpcode(opcode, numArgs);
+ break;
+ case Select:
+ if (UNLIKELY(numArgs != 3))
+ badOpcode(opcode, numArgs);
+ break;
+ default:
+ badOpcode(opcode, numArgs);
+ break;
+ }
+ }
</ins><span class="cx">
</span><span class="cx"> protected:
</span><span class="cx"> enum CheckedOpcodeTag { CheckedOpcode };
</span><span class="lines">@@ -309,11 +369,35 @@
</span><span class="cx"> // This is the constructor you end up actually calling, if you're instantiating Value
</span><span class="cx"> // directly.
</span><span class="cx"> template<typename... Arguments>
</span><del>- explicit Value(Opcode opcode, Arguments&&... arguments)
- : Value(CheckedOpcode, opcode, std::forward<Arguments>(arguments)...)
</del><ins>+ explicit Value(Opcode opcode, Type type, Origin origin)
+ : Value(CheckedOpcode, opcode, type, origin)
</ins><span class="cx"> {
</span><del>- checkOpcode(opcode);
</del><ins>+ checkOpcode(opcode, 0);
</ins><span class="cx"> }
</span><ins>+ template<typename... Arguments>
+ explicit Value(Opcode opcode, Type type, Origin origin, Value* firstChild, Arguments&&... arguments)
+ : Value(CheckedOpcode, opcode, type, origin, firstChild, std::forward<Arguments>(arguments)...)
+ {
+ checkOpcode(opcode, 1 + sizeof...(arguments));
+ }
+ template<typename... Arguments>
+ explicit Value(Opcode opcode, Type type, Origin origin, const AdjacencyList& children)
+ : Value(CheckedOpcode, opcode, type, origin, children)
+ {
+ checkOpcode(opcode, children.size());
+ }
+ template<typename... Arguments>
+ explicit Value(Opcode opcode, Type type, Origin origin, AdjacencyList&& children)
+ : Value(CheckedOpcode, opcode, type, origin, WTFMove(children))
+ {
+ checkOpcode(opcode, m_children.size());
+ }
+ template<typename... Arguments>
+ explicit Value(Opcode opcode, Origin origin, Arguments&&... arguments)
+ : Value(CheckedOpcode, opcode, origin, std::forward<Arguments>(arguments)...)
+ {
+ checkOpcode(opcode, sizeof...(arguments));
+ }
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> friend class CheckValue; // CheckValue::convertToAdd() modifies m_opcode.
</span><span class="lines">@@ -330,6 +414,8 @@
</span><span class="cx"> Origin m_origin;
</span><span class="cx"> AdjacencyList m_children;
</span><span class="cx">
</span><ins>+ JS_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH static void badOpcode(Opcode, unsigned);
+
</ins><span class="cx"> public:
</span><span class="cx"> BasicBlock* owner { nullptr }; // computed by Procedure::resetValueOwners().
</span><span class="cx"> };
</span></span></pre>
</div>
</div>
</body>
</html>