<!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>[208341] 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/208341">208341</a></dd>
<dt>Author</dt> <dd>keith_miller@apple.com</dd>
<dt>Date</dt> <dd>2016-11-03 13:47:57 -0700 (Thu, 03 Nov 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Asking for a value profile prediction should be defensive against not finding a value profile
https://bugs.webkit.org/show_bug.cgi?id=164306
Patch by Saam Barati <sbarati@apple.com> on 2016-11-03
Reviewed by Mark Lam.
Currently, the code that calls CodeBlock::valueProfilePredictionForBytecodeOffset
in the DFG assumes it will always be at a value producing node. However, this isn't
true if we tail call from an inlined setter. When we're at a tail call, we try
to find the first caller that isn't a tail call to see what value the
tail_call produces. If we inline a setter, however, we will end up finding
the put_by_id as our first non-tail-called "caller", and that won't have a
value profile associated with it since it's not a value producing node.
CodeBlock::valueProfilePredictionForBytecodeOffset should be defensive
against finding a null value profile.
* bytecode/CodeBlock.h:
(JSC::CodeBlock::valueProfilePredictionForBytecodeOffset):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::getPredictionWithoutOSRExit):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoretestWasmcpp">trunk/Source/JavaScriptCore/testWasm.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmB3IRGeneratorcpp">trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmFunctionParserh">trunk/Source/JavaScriptCore/wasm/WasmFunctionParser.h</a></li>
<li><a href="#trunkSourceJavaScriptCorewasmWasmValidatecpp">trunk/Source/JavaScriptCore/wasm/WasmValidate.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (208340 => 208341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-11-03 19:31:07 UTC (rev 208340)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-11-03 20:47:57 UTC (rev 208341)
</span><span class="lines">@@ -2792,6 +2792,46 @@
</span><span class="cx">
</span><span class="cx"> 2016-11-02 Keith Miller <keith_miller@apple.com>
</span><span class="cx">
</span><ins>+ Wasm starts a new stack whenever it adds a new block and has return types for blocks.
+ https://bugs.webkit.org/show_bug.cgi?id=164100
+
+ Reviewed by Saam Barati.
+
+ This patch overhauls much of the Wasm function parser, validator, and B3 IR generator
+ to work with block return types. In Wasm, blocks can act as expressions and have a
+ return value. Most of the control flow operators needed to be rewritten in order to
+ support this feature. To enable return types the function parser needed to be able
+ to save and restore the expression stack from previous blocks, which is done via the
+ control stack.
+
+ This patch also removes the lazy continuation block system added previously. It's
+ not clear if there would be any performance win from such a system. There are likely
+ many other things with orders of magnitude more impact on B3 IR generation. The
+ complexity cost of such a system is not worth the effort without sufficient evidence
+ otherwise.
+
+ * testWasm.cpp:
+ (runWasmTests):
+ * wasm/WasmB3IRGenerator.cpp:
+ * wasm/WasmFunctionParser.h:
+ (JSC::Wasm::FunctionParser<Context>::parseBlock):
+ (JSC::Wasm::FunctionParser<Context>::addReturn):
+ (JSC::Wasm::FunctionParser<Context>::parseExpression):
+ (JSC::Wasm::FunctionParser<Context>::parseUnreachableExpression):
+ (JSC::Wasm::FunctionParser<Context>::popExpressionStack):
+ * wasm/WasmValidate.cpp:
+ (JSC::Wasm::Validate::ControlData::hasNonVoidSignature):
+ (JSC::Wasm::Validate::addElse):
+ (JSC::Wasm::Validate::addElseToUnreachable):
+ (JSC::Wasm::Validate::addBranch):
+ (JSC::Wasm::Validate::endBlock):
+ (JSC::Wasm::Validate::addEndToUnreachable):
+ (JSC::Wasm::Validate::dump):
+ (JSC::Wasm::validateFunction):
+ (JSC::Wasm::Validate::isContinuationReachable): Deleted.
+
+2016-11-02 Keith Miller <keith_miller@apple.com>
+
</ins><span class="cx"> We should not pop from an empty stack in the Wasm function parser.
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=164275
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestWasmcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/testWasm.cpp (208340 => 208341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/testWasm.cpp 2016-11-03 19:31:07 UTC (rev 208340)
+++ trunk/Source/JavaScriptCore/testWasm.cpp 2016-11-03 20:47:57 UTC (rev 208341)
</span><span class="lines">@@ -293,10 +293,111 @@
</span><span class="cx"> // For now we inline the test files.
</span><span class="cx"> static void runWasmTests()
</span><span class="cx"> {
</span><ins>+ {
+ // Generated from:
+ // (module
+ // (func (export "if-then-both-fallthrough") (param $x i32) (param $y i32) (result i32)
+ // (block $block i32
+ // (if i32 (i32.eq (get_local $x) (i32.const 0))
+ // (then (i32.const 1))
+ // (else
+ // (i32.const 2)
+ // (br $block))
+ // )
+ // )
+ // )
+ // )
+ Vector<uint8_t> vector = {
+ 0x00, 0x61, 0x73, 0x6d, 0x0c, 0x00, 0x00, 0x00, 0x01, 0x87, 0x80, 0x80, 0x80, 0x00, 0x01, 0x40,
+ 0x02, 0x01, 0x01, 0x01, 0x01, 0x03, 0x82, 0x80, 0x80, 0x80, 0x00, 0x01, 0x00, 0x07, 0x9c, 0x80,
+ 0x80, 0x80, 0x00, 0x01, 0x18, 0x69, 0x66, 0x2d, 0x74, 0x68, 0x65, 0x6e, 0x2d, 0x62, 0x6f, 0x74,
+ 0x68, 0x2d, 0x66, 0x61, 0x6c, 0x6c, 0x74, 0x68, 0x72, 0x6f, 0x75, 0x67, 0x68, 0x00, 0x00, 0x0a,
+ 0x9a, 0x80, 0x80, 0x80, 0x00, 0x01, 0x94, 0x80, 0x80, 0x80, 0x00, 0x00, 0x01, 0x01, 0x14, 0x00,
+ 0x10, 0x00, 0x4d, 0x03, 0x01, 0x10, 0x01, 0x04, 0x10, 0x02, 0x06, 0x01, 0x0f, 0x0f, 0x0f
+ };
</ins><span class="cx">
</span><ins>+ Plan plan(*vm, vector);
+ checkPlan(plan, 1);
+
+ // Test this doesn't crash.
+ CHECK(isIdentical(invoke<int>(*plan.result(0)->jsEntryPoint, { boxf(0), boxf(32) }), 1));
+ CHECK(isIdentical(invoke<int>(*plan.result(0)->jsEntryPoint, { boxf(1), boxf(32) }), 2));
+ }
+
</ins><span class="cx"> {
</span><span class="cx"> // Generated from:
</span><span class="cx"> // (module
</span><ins>+ // (func (export "if-then-both-fallthrough") (param $x i32) (param $y i32) (result i32)
+ // (if i32 (i32.eq (get_local $x) (i32.const 0))
+ // (then (i32.const 1))
+ // (else (i32.const 2))
+ // )
+ // )
+ // )
+ Vector<uint8_t> vector = {
+ 0x00, 0x61, 0x73, 0x6d, 0x0c, 0x00, 0x00, 0x00, 0x01, 0x87, 0x80, 0x80, 0x80, 0x00, 0x01, 0x40,
+ 0x02, 0x01, 0x01, 0x01, 0x01, 0x03, 0x82, 0x80, 0x80, 0x80, 0x00, 0x01, 0x00, 0x07, 0x9c, 0x80,
+ 0x80, 0x80, 0x00, 0x01, 0x18, 0x69, 0x66, 0x2d, 0x74, 0x68, 0x65, 0x6e, 0x2d, 0x62, 0x6f, 0x74,
+ 0x68, 0x2d, 0x66, 0x61, 0x6c, 0x6c, 0x74, 0x68, 0x72, 0x6f, 0x75, 0x67, 0x68, 0x00, 0x00, 0x0a,
+ 0x95, 0x80, 0x80, 0x80, 0x00, 0x01, 0x8f, 0x80, 0x80, 0x80, 0x00, 0x00, 0x14, 0x00, 0x10, 0x00,
+ 0x4d, 0x03, 0x01, 0x10, 0x01, 0x04, 0x10, 0x02, 0x0f, 0x0f
+ };
+
+ Plan plan(*vm, vector);
+ checkPlan(plan, 1);
+
+ // Test this doesn't crash.
+ CHECK(isIdentical(invoke<int>(*plan.result(0)->jsEntryPoint, { boxf(0), boxf(32) }), 1));
+ CHECK(isIdentical(invoke<int>(*plan.result(0)->jsEntryPoint, { boxf(1), boxf(32) }), 2));
+ }
+
+ {
+ // Generated from:
+ // (module
+ // (func (export "dumb-less-than") (param $x i32) (param $y i32) (result i32)
+ // (loop $loop i32
+ // (i32.eq (get_local $x) (get_local $y))
+ // (if i32
+ // (then (return (i32.const 0)))
+ // (else
+ // (get_local $x)
+ // (set_local $x (i32.sub (get_local $x) (i32.const 1)))
+ // (i32.const 0)
+ // (i32.ne)
+ // (br_if $loop)
+ // (i32.const 1)
+ // )
+ // )
+ // )
+ // )
+ // )
+ Vector<uint8_t> vector = {
+ 0x00, 0x61, 0x73, 0x6d, 0x0c, 0x00, 0x00, 0x00, 0x01, 0x87, 0x80, 0x80, 0x80, 0x00, 0x01, 0x40,
+ 0x02, 0x01, 0x01, 0x01, 0x01, 0x03, 0x82, 0x80, 0x80, 0x80, 0x00, 0x01, 0x00, 0x07, 0x92, 0x80,
+ 0x80, 0x80, 0x00, 0x01, 0x0e, 0x64, 0x75, 0x6d, 0x62, 0x2d, 0x6c, 0x65, 0x73, 0x73, 0x2d, 0x74,
+ 0x68, 0x61, 0x6e, 0x00, 0x00, 0x0a, 0xa7, 0x80, 0x80, 0x80, 0x00, 0x01, 0xa1, 0x80, 0x80, 0x80,
+ 0x00, 0x00, 0x02, 0x01, 0x14, 0x00, 0x14, 0x01, 0x4d, 0x03, 0x01, 0x10, 0x00, 0x09, 0x04, 0x14,
+ 0x00, 0x14, 0x00, 0x10, 0x01, 0x41, 0x15, 0x00, 0x10, 0x00, 0x4e, 0x07, 0x01, 0x10, 0x01, 0x0f,
+ 0x0f, 0x0f
+ };
+
+ Plan plan(*vm, vector);
+ checkPlan(plan, 1);
+
+ // Test this doesn't crash.
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(0), box(1) }), 1);
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(1), box(0) }), 0);
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(2), box(1) }), 0);
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(1), box(2) }), 1);
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(2), box(2) }), 0);
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(1), box(1) }), 0);
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(2), box(6) }), 1);
+ CHECK_EQ(invoke<int>(*plan.result(0)->jsEntryPoint, { box(100), box(6) }), 0);
+ }
+
+ {
+ // Generated from:
+ // (module
</ins><span class="cx"> // (func $f32-sub (export "f32-sub") (param f32) (param f32) (result f32) (return (f32.sub (get_local 0) (get_local 1))))
</span><span class="cx"> // (func (export "indirect-f32-sub") (param f32) (param f32) (result f32) (return (call $f32-sub (get_local 0) (get_local 1))))
</span><span class="cx"> // )
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmB3IRGeneratorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp (208340 => 208341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp 2016-11-03 19:31:07 UTC (rev 208340)
+++ trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp 2016-11-03 20:47:57 UTC (rev 208341)
</span><span class="lines">@@ -79,43 +79,11 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> class B3IRGenerator {
</span><del>-private:
- class LazyBlock {
- public:
- LazyBlock(BasicBlock* block)
- : m_block(block)
- {
- }
-
- LazyBlock()
- {
- }
-
- explicit operator bool() const { return !!m_block; }
-
- BasicBlock* get(Procedure& proc)
- {
- if (!m_block)
- m_block = proc.addBlock();
- return m_block;
- }
-
- void dump(PrintStream& out) const
- {
- if (m_block)
- out.print(*m_block);
- else
- out.print("Uninitialized");
- }
-
- private:
- BasicBlock* m_block { nullptr };
- };
-
</del><span class="cx"> public:
</span><span class="cx"> struct ControlData {
</span><del>- ControlData(Procedure& proc, Type signature, BasicBlock* special = nullptr, BasicBlock* continuation = nullptr)
- : continuation(continuation)
</del><ins>+ ControlData(Procedure& proc, Type signature, BlockType type, BasicBlock* continuation, BasicBlock* special = nullptr)
+ : blockType(type)
+ , continuation(continuation)
</ins><span class="cx"> , special(special)
</span><span class="cx"> {
</span><span class="cx"> if (signature != Void)
</span><span class="lines">@@ -139,7 +107,7 @@
</span><span class="cx"> out.print("Loop: ");
</span><span class="cx"> break;
</span><span class="cx"> }
</span><del>- out.print("Continuation: ", continuation, ", Special: ");
</del><ins>+ out.print("Continuation: ", *continuation, ", Special: ");
</ins><span class="cx"> if (special)
</span><span class="cx"> out.print(*special);
</span><span class="cx"> else
</span><span class="lines">@@ -146,28 +114,28 @@
</span><span class="cx"> out.print("None");
</span><span class="cx"> }
</span><span class="cx">
</span><del>- BlockType type() const
- {
- if (!special)
- return BlockType::Block;
- if (continuation)
- return BlockType::If;
- return BlockType::Loop;
- }
</del><ins>+ BlockType type() const { return blockType; }
</ins><span class="cx">
</span><del>- BasicBlock* targetBlockForBranch(Procedure& proc)
</del><ins>+ bool hasNonVoidSignature() const { return result.size(); }
+
+ BasicBlock* targetBlockForBranch()
</ins><span class="cx"> {
</span><span class="cx"> if (type() == BlockType::Loop)
</span><span class="cx"> return special;
</span><del>- return continuation.get(proc);
</del><ins>+ return continuation;
</ins><span class="cx"> }
</span><span class="cx">
</span><ins>+ void convertIfToBlock()
+ {
+ ASSERT(type() == BlockType::If);
+ blockType = BlockType::Block;
+ special = nullptr;
+ }
+
</ins><span class="cx"> private:
</span><span class="cx"> friend class B3IRGenerator;
</span><del>- // We use a LazyBlock for the continuation since B3::validate does not like orphaned blocks. Note,
- // it's possible to create an orphaned block by doing something like (block (return (...))). In
- // that example, if we eagerly allocate a BasicBlock for the continuation it will never be reachable.
- LazyBlock continuation;
</del><ins>+ BlockType blockType;
+ BasicBlock* continuation;
</ins><span class="cx"> BasicBlock* special;
</span><span class="cx"> Vector<Variable*, 1> result;
</span><span class="cx"> };
</span><span class="lines">@@ -176,6 +144,8 @@
</span><span class="cx"> typedef ControlData ControlType;
</span><span class="cx"> typedef Vector<ExpressionType, 1> ExpressionList;
</span><span class="cx"> typedef Vector<Variable*, 1> ResultList;
</span><ins>+ typedef FunctionParser<B3IRGenerator>::ControlEntry ControlEntry;
+
</ins><span class="cx"> static constexpr ExpressionType emptyExpression = nullptr;
</span><span class="cx">
</span><span class="cx"> B3IRGenerator(Memory*, Procedure&, Vector<UnlinkedCall>& unlinkedCalls);
</span><span class="lines">@@ -201,17 +171,17 @@
</span><span class="cx"> ControlData WARN_UNUSED_RETURN addLoop(Type signature);
</span><span class="cx"> bool WARN_UNUSED_RETURN addIf(ExpressionType condition, Type signature, ControlData& result);
</span><span class="cx"> bool WARN_UNUSED_RETURN addElse(ControlData&, const ExpressionList&);
</span><ins>+ bool WARN_UNUSED_RETURN addElseToUnreachable(ControlData&);
</ins><span class="cx">
</span><span class="cx"> bool WARN_UNUSED_RETURN addReturn(const ExpressionList& returnValues);
</span><span class="cx"> bool WARN_UNUSED_RETURN addBranch(ControlData&, ExpressionType condition, const ExpressionList& returnValues);
</span><del>- bool WARN_UNUSED_RETURN endBlock(ControlData&, ExpressionList& expressionStack);
</del><ins>+ bool WARN_UNUSED_RETURN endBlock(ControlEntry&, ExpressionList& expressionStack);
+ bool WARN_UNUSED_RETURN addEndToUnreachable(ControlEntry&);
</ins><span class="cx">
</span><span class="cx"> bool WARN_UNUSED_RETURN addCall(unsigned calleeIndex, const FunctionInformation&, Vector<ExpressionType>& args, ExpressionType& result);
</span><span class="cx">
</span><del>- bool isContinuationReachable(ControlData&);
</del><ins>+ void dump(const Vector<ControlEntry>& controlStack, const ExpressionList& expressionStack);
</ins><span class="cx">
</span><del>- void dump(const Vector<ControlType>& controlStack, const ExpressionList& expressionStack);
-
</del><span class="cx"> void setErrorMessage(String&&) { UNREACHABLE_FOR_PLATFORM(); }
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="lines">@@ -498,16 +468,17 @@
</span><span class="cx">
</span><span class="cx"> B3IRGenerator::ControlData B3IRGenerator::addBlock(Type signature)
</span><span class="cx"> {
</span><del>- return ControlData(m_proc, signature);
</del><ins>+ return ControlData(m_proc, signature, BlockType::Block, m_proc.addBlock());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> B3IRGenerator::ControlData B3IRGenerator::addLoop(Type signature)
</span><span class="cx"> {
</span><span class="cx"> BasicBlock* body = m_proc.addBlock();
</span><ins>+ BasicBlock* continuation = m_proc.addBlock();
</ins><span class="cx"> m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), body);
</span><span class="cx"> body->addPredecessor(m_currentBlock);
</span><span class="cx"> m_currentBlock = body;
</span><del>- return ControlData(m_proc, signature, body);
</del><ins>+ return ControlData(m_proc, signature, BlockType::Loop, continuation, body);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool B3IRGenerator::addIf(ExpressionType condition, Type signature, ControlType& result)
</span><span class="lines">@@ -524,17 +495,22 @@
</span><span class="cx"> notTaken->addPredecessor(m_currentBlock);
</span><span class="cx">
</span><span class="cx"> m_currentBlock = taken;
</span><del>- result = ControlData(m_proc, signature, notTaken, continuation);
</del><ins>+ result = ControlData(m_proc, signature, BlockType::If, continuation, notTaken);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool B3IRGenerator::addElse(ControlData& data, const ExpressionList&)
</del><ins>+bool B3IRGenerator::addElse(ControlData& data, const ExpressionList& currentStack)
</ins><span class="cx"> {
</span><del>- ASSERT(data.continuation);
</del><ins>+ unifyValuesWithBlock(currentStack, data.result);
+ m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), data.continuation);
+ return addElseToUnreachable(data);
+}
+
+bool B3IRGenerator::addElseToUnreachable(ControlData& data)
+{
+ ASSERT(data.type() == BlockType::If);
</ins><span class="cx"> m_currentBlock = data.special;
</span><del>- // Clear the special pointer so that when we parse the end we don't think that this block is an if block.
- data.special = nullptr;
- ASSERT(data.type() == BlockType::Block);
</del><ins>+ data.convertIfToBlock();
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -550,8 +526,10 @@
</span><span class="cx">
</span><span class="cx"> bool B3IRGenerator::addBranch(ControlData& data, ExpressionType condition, const ExpressionList& returnValues)
</span><span class="cx"> {
</span><del>- BasicBlock* target = data.targetBlockForBranch(m_proc);
- unifyValuesWithBlock(returnValues, data.result);
</del><ins>+ if (data.type() != BlockType::Loop)
+ unifyValuesWithBlock(returnValues, data.result);
+
+ BasicBlock* target = data.targetBlockForBranch();
</ins><span class="cx"> if (condition) {
</span><span class="cx"> BasicBlock* continuation = m_proc.addBlock();
</span><span class="cx"> m_currentBlock->appendNew<Value>(m_proc, B3::Branch, Origin(), condition);
</span><span class="lines">@@ -567,24 +545,31 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool B3IRGenerator::endBlock(ControlData& data, ExpressionList& expressionStack)
</del><ins>+bool B3IRGenerator::endBlock(ControlEntry& entry, ExpressionList& expressionStack)
</ins><span class="cx"> {
</span><del>- if (!data.continuation)
- return true;
</del><ins>+ ControlData& data = entry.controlData;
</ins><span class="cx">
</span><del>- BasicBlock* continuation = data.continuation.get(m_proc);
</del><ins>+ unifyValuesWithBlock(expressionStack, data.result);
+ m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), data.continuation);
+ data.continuation->addPredecessor(m_currentBlock);
+
+ return addEndToUnreachable(entry);
+}
+
+
+bool B3IRGenerator::addEndToUnreachable(ControlEntry& entry)
+{
+ ControlData& data = entry.controlData;
+ m_currentBlock = data.continuation;
+
</ins><span class="cx"> if (data.type() == BlockType::If) {
</span><del>- ASSERT(!data.special->size() && !data.special->successors().size());
- // Since we don't have any else block we need to point the notTaken branch to the continuation.
- data.special->appendNewControlValue(m_proc, Jump, Origin());
- data.special->setSuccessors(FrequentedBlock(continuation));
- continuation->addPredecessor(data.special);
</del><ins>+ data.special->appendNewControlValue(m_proc, Jump, Origin(), m_currentBlock);
+ m_currentBlock->addPredecessor(data.special);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- unifyValuesWithBlock(expressionStack, data.result);
- m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), continuation);
- continuation->addPredecessor(m_currentBlock);
- m_currentBlock = continuation;
</del><ins>+ for (Variable* result : data.result)
+ entry.enclosedExpressionStack.append(m_currentBlock->appendNew<VariableValue>(m_proc, B3::Get, Origin(), result));
+
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -614,22 +599,6 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool B3IRGenerator::isContinuationReachable(ControlData& data)
-{
- // If nothing targets the continuation of the current block then we don't want to create
- // an orphaned BasicBlock since it can't be reached by fallthrough.
- if (!data.continuation)
- return false;
-
- m_currentBlock = data.continuation.get(m_proc);
- if (data.type() == BlockType::If) {
- data.special->appendNewControlValue(m_proc, Jump, Origin(), m_currentBlock);
- m_currentBlock->addPredecessor(data.special);
- }
-
- return true;
-}
-
</del><span class="cx"> void B3IRGenerator::unify(Variable* variable, ExpressionType source)
</span><span class="cx"> {
</span><span class="cx"> m_currentBlock->appendNew<VariableValue>(m_proc, Set, Origin(), variable, source);
</span><span class="lines">@@ -637,23 +606,35 @@
</span><span class="cx">
</span><span class="cx"> void B3IRGenerator::unifyValuesWithBlock(const ExpressionList& resultStack, ResultList& result)
</span><span class="cx"> {
</span><del>- ASSERT(result.size() >= resultStack.size());
</del><ins>+ ASSERT(result.size() <= resultStack.size());
</ins><span class="cx">
</span><del>- for (size_t i = 0; i < resultStack.size(); ++i)
- unify(result[i], resultStack[i]);
</del><ins>+ for (size_t i = 0; i < result.size(); ++i)
+ unify(result[result.size() - 1 - i], resultStack[resultStack.size() - 1 - i]);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void B3IRGenerator::dump(const Vector<ControlType>& controlStack, const ExpressionList& expressionStack)
</del><ins>+static void dumpExpressionStack(const CommaPrinter& comma, const B3IRGenerator::ExpressionList& expressionStack)
</ins><span class="cx"> {
</span><ins>+ dataLogLn(comma, "ExpressionStack:");
+ for (const auto& expression : expressionStack)
+ dataLogLn(comma, *expression);
+}
+
+void B3IRGenerator::dump(const Vector<ControlEntry>& controlStack, const ExpressionList& expressionStack)
+{
</ins><span class="cx"> dataLogLn("Processing Graph:");
</span><span class="cx"> dataLog(m_proc);
</span><span class="cx"> dataLogLn("With current block:", *m_currentBlock);
</span><span class="cx"> dataLogLn("Control stack:");
</span><del>- for (const ControlType& data : controlStack)
- dataLogLn(" ", data);
- dataLogLn("ExpressionStack:");
- for (const ExpressionType& expression : expressionStack)
- dataLogLn(" ", *expression);
</del><ins>+ for (auto& data : controlStack) {
+ dataLogLn(" ", data.controlData);
+ if (data.enclosedExpressionStack.size()) {
+ CommaPrinter comma(" ", " with ");
+ dumpExpressionStack(comma, data.enclosedExpressionStack);
+ }
+ }
+
+ CommaPrinter comma(" ", "");
+ dumpExpressionStack(comma, expressionStack);
</ins><span class="cx"> dataLogLn("\n");
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmFunctionParserh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmFunctionParser.h (208340 => 208341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmFunctionParser.h 2016-11-03 19:31:07 UTC (rev 208340)
+++ trunk/Source/JavaScriptCore/wasm/WasmFunctionParser.h 2016-11-03 20:47:57 UTC (rev 208341)
</span><span class="lines">@@ -43,11 +43,17 @@
</span><span class="cx"> public:
</span><span class="cx"> typedef typename Context::ExpressionType ExpressionType;
</span><span class="cx"> typedef typename Context::ControlType ControlType;
</span><ins>+ typedef typename Context::ExpressionList ExpressionList;
</ins><span class="cx">
</span><span class="cx"> FunctionParser(Context&, const uint8_t* functionStart, size_t functionLength, const Signature*, const Vector<FunctionInformation>& functions);
</span><span class="cx">
</span><span class="cx"> bool WARN_UNUSED_RETURN parse();
</span><span class="cx">
</span><ins>+ struct ControlEntry {
+ ExpressionList enclosedExpressionStack;
+ ControlType controlData;
+ };
+
</ins><span class="cx"> private:
</span><span class="cx"> static const bool verbose = false;
</span><span class="cx">
</span><span class="lines">@@ -54,13 +60,14 @@
</span><span class="cx"> bool WARN_UNUSED_RETURN parseBlock();
</span><span class="cx"> bool WARN_UNUSED_RETURN parseExpression(OpType);
</span><span class="cx"> bool WARN_UNUSED_RETURN parseUnreachableExpression(OpType);
</span><ins>+ bool WARN_UNUSED_RETURN addReturn();
</ins><span class="cx"> bool WARN_UNUSED_RETURN unifyControl(Vector<ExpressionType>&, unsigned level);
</span><span class="cx">
</span><span class="cx"> bool WARN_UNUSED_RETURN popExpressionStack(ExpressionType& result);
</span><span class="cx">
</span><span class="cx"> Context& m_context;
</span><del>- Vector<ExpressionType, 1> m_expressionStack;
- Vector<ControlType> m_controlStack;
</del><ins>+ ExpressionList m_expressionStack;
+ Vector<ControlEntry> m_controlStack;
</ins><span class="cx"> const Signature* m_signature;
</span><span class="cx"> const Vector<FunctionInformation>& m_functions;
</span><span class="cx"> unsigned m_unreachableBlocks { 0 };
</span><span class="lines">@@ -120,7 +127,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (op == OpType::End && !m_controlStack.size())
</span><del>- break;
</del><ins>+ return m_unreachableBlocks ? true : addReturn();
</ins><span class="cx">
</span><span class="cx"> if (m_unreachableBlocks) {
</span><span class="cx"> if (!parseUnreachableExpression(static_cast<OpType>(op))) {
</span><span class="lines">@@ -136,9 +143,24 @@
</span><span class="cx">
</span><span class="cx"> }
</span><span class="cx">
</span><del>- // I'm not sure if we should check the expression stack here...
- return true;
</del><ins>+ RELEASE_ASSERT_NOT_REACHED();
</ins><span class="cx"> }
</span><ins>+
+template<typename Context>
+bool FunctionParser<Context>::addReturn()
+{
+ ExpressionList returnValues;
+ if (m_signature->returnType != Void) {
+ ExpressionType returnValue;
+ if (!popExpressionStack(returnValue))
+ return false;
+ returnValues.append(returnValue);
+ }
+
+ m_unreachableBlocks = 1;
+ return m_context.addReturn(returnValues);
+}
+
</ins><span class="cx"> #define CREATE_CASE(name, id, b3op) case name:
</span><span class="cx">
</span><span class="cx"> template<typename Context>
</span><span class="lines">@@ -288,7 +310,8 @@
</span><span class="cx"> if (!parseValueType(inlineSignature))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- m_controlStack.append(m_context.addBlock(inlineSignature));
</del><ins>+ m_controlStack.append({ WTFMove(m_expressionStack), m_context.addBlock(inlineSignature) });
+ m_expressionStack = ExpressionList();
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -297,7 +320,8 @@
</span><span class="cx"> if (!parseValueType(inlineSignature))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- m_controlStack.append(m_context.addLoop(inlineSignature));
</del><ins>+ m_controlStack.append({ WTFMove(m_expressionStack), m_context.addLoop(inlineSignature) });
+ m_expressionStack = ExpressionList();
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -314,7 +338,8 @@
</span><span class="cx"> if (!m_context.addIf(condition, inlineSignature, control))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- m_controlStack.append(control);
</del><ins>+ m_controlStack.append({ WTFMove(m_expressionStack), control });
+ m_expressionStack = ExpressionList();
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -323,7 +348,11 @@
</span><span class="cx"> m_context.setErrorMessage("Attempted to use else block at the top-level of a function");
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><del>- return m_context.addElse(m_controlStack.last(), m_expressionStack);
</del><ins>+
+ if (!m_context.addElse(m_controlStack.last().controlData, m_expressionStack))
+ return false;
+ m_expressionStack.shrink(0);
+ return true;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case OpType::Br:
</span><span class="lines">@@ -339,29 +368,31 @@
</span><span class="cx"> } else
</span><span class="cx"> m_unreachableBlocks = 1;
</span><span class="cx">
</span><del>- ControlType& data = m_controlStack[m_controlStack.size() - 1 - target];
</del><ins>+ ControlType& data = m_controlStack[m_controlStack.size() - 1 - target].controlData;
</ins><span class="cx">
</span><span class="cx"> return m_context.addBranch(data, condition, m_expressionStack);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case OpType::Return: {
</span><del>- Vector<ExpressionType, 1> returnValues;
- if (m_signature->returnType != Void) {
- ExpressionType returnValue;
- if (!popExpressionStack(returnValue))
- returnValues.append(returnValue);
- }
-
- m_unreachableBlocks = 1;
- return m_context.addReturn(returnValues);
</del><ins>+ return addReturn();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case OpType::End: {
</span><del>- ControlType data = m_controlStack.takeLast();
- return m_context.endBlock(data, m_expressionStack);
</del><ins>+ ControlEntry data = m_controlStack.takeLast();
+ // FIXME: This is a little weird in that it will modify the expressionStack for the result of the block.
+ // That's a little too effectful for me but I don't have a better API right now.
+ // see: https://bugs.webkit.org/show_bug.cgi?id=164353
+ if (!m_context.endBlock(data, m_expressionStack))
+ return false;
+ m_expressionStack.swap(data.enclosedExpressionStack);
+ return true;
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- case OpType::Unreachable:
</del><ins>+ case OpType::Unreachable: {
+ m_unreachableBlocks = 1;
+ return true;
+ }
+
</ins><span class="cx"> case OpType::Select:
</span><span class="cx"> case OpType::BrTable:
</span><span class="cx"> case OpType::Nop:
</span><span class="lines">@@ -386,17 +417,20 @@
</span><span class="cx"> if (m_unreachableBlocks > 1)
</span><span class="cx"> return true;
</span><span class="cx">
</span><del>- ControlType& data = m_controlStack.last();
- ASSERT(data.type() == BlockType::If);
</del><ins>+ ControlEntry& data = m_controlStack.last();
</ins><span class="cx"> m_unreachableBlocks = 0;
</span><del>- return m_context.addElse(data, m_expressionStack);
</del><ins>+ if (!m_context.addElseToUnreachable(data.controlData))
+ return false;
+ m_expressionStack.shrink(0);
+ return true;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> case OpType::End: {
</span><span class="cx"> if (m_unreachableBlocks == 1) {
</span><del>- ControlType data = m_controlStack.takeLast();
- if (!m_context.isContinuationReachable(data))
- return true;
</del><ins>+ ControlEntry data = m_controlStack.takeLast();
+ if (!m_context.addEndToUnreachable(data))
+ return false;
+ m_expressionStack.swap(data.enclosedExpressionStack);
</ins><span class="cx"> }
</span><span class="cx"> m_unreachableBlocks--;
</span><span class="cx"> return true;
</span><span class="lines">@@ -439,12 +473,12 @@
</span><span class="cx"> template<typename Context>
</span><span class="cx"> bool FunctionParser<Context>::popExpressionStack(ExpressionType& result)
</span><span class="cx"> {
</span><del>- if (!m_expressionStack.size()) {
</del><ins>+ if (m_expressionStack.size()) {
</ins><span class="cx"> result = m_expressionStack.takeLast();
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- m_context.setErrorMessage("Attempted to use an stack value when none existed");
</del><ins>+ m_context.setErrorMessage("Attempted to use a stack value when none existed");
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorewasmWasmValidatecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/wasm/WasmValidate.cpp (208340 => 208341)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/wasm/WasmValidate.cpp 2016-11-03 19:31:07 UTC (rev 208340)
+++ trunk/Source/JavaScriptCore/wasm/WasmValidate.cpp 2016-11-03 20:47:57 UTC (rev 208341)
</span><span class="lines">@@ -29,6 +29,7 @@
</span><span class="cx"> #if ENABLE(WEBASSEMBLY)
</span><span class="cx">
</span><span class="cx"> #include "WasmFunctionParser.h"
</span><ins>+#include <wtf/CommaPrinter.h>
</ins><span class="cx"> #include <wtf/text/StringBuilder.h>
</span><span class="cx">
</span><span class="cx"> namespace JSC { namespace Wasm {
</span><span class="lines">@@ -62,6 +63,8 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ bool hasNonVoidSignature() const { return m_signature != Void; }
+
</ins><span class="cx"> BlockType type() const { return m_blockType; }
</span><span class="cx"> Type signature() const { return m_signature; }
</span><span class="cx"> private:
</span><span class="lines">@@ -71,6 +74,8 @@
</span><span class="cx"> typedef Type ExpressionType;
</span><span class="cx"> typedef ControlData ControlType;
</span><span class="cx"> typedef Vector<ExpressionType, 1> ExpressionList;
</span><ins>+ typedef FunctionParser<Validate>::ControlEntry ControlEntry;
+
</ins><span class="cx"> static const ExpressionType emptyExpression = Void;
</span><span class="cx">
</span><span class="cx"> bool WARN_UNUSED_RETURN addArguments(const Vector<Type>&);
</span><span class="lines">@@ -94,15 +99,17 @@
</span><span class="cx"> ControlData WARN_UNUSED_RETURN addLoop(Type signature);
</span><span class="cx"> bool WARN_UNUSED_RETURN addIf(ExpressionType condition, Type signature, ControlData& result);
</span><span class="cx"> bool WARN_UNUSED_RETURN addElse(ControlData&, const ExpressionList&);
</span><ins>+ bool WARN_UNUSED_RETURN addElseToUnreachable(ControlData&);
</ins><span class="cx">
</span><span class="cx"> bool WARN_UNUSED_RETURN addReturn(const ExpressionList& returnValues);
</span><span class="cx"> bool WARN_UNUSED_RETURN addBranch(ControlData&, ExpressionType condition, const ExpressionList& returnValues);
</span><del>- bool WARN_UNUSED_RETURN endBlock(ControlData&, ExpressionList& expressionStack);
</del><ins>+ bool WARN_UNUSED_RETURN endBlock(ControlEntry&, ExpressionList& expressionStack);
+ bool WARN_UNUSED_RETURN addEndToUnreachable(ControlEntry&);
</ins><span class="cx">
</span><ins>+
</ins><span class="cx"> bool WARN_UNUSED_RETURN addCall(unsigned calleeIndex, const FunctionInformation&, const Vector<ExpressionType>& args, ExpressionType& result);
</span><del>- bool WARN_UNUSED_RETURN isContinuationReachable(ControlData&) { return true; }
</del><span class="cx">
</span><del>- void dump(const Vector<ControlType>& controlStack, const ExpressionList& expressionStack);
</del><ins>+ void dump(const Vector<ControlEntry>& controlStack, const ExpressionList& expressionStack);
</ins><span class="cx">
</span><span class="cx"> void setErrorMessage(String&& message) { ASSERT(m_errorMessage.isNull()); m_errorMessage = WTFMove(message); }
</span><span class="cx"> String errorMessage() const { return m_errorMessage; }
</span><span class="lines">@@ -184,13 +191,18 @@
</span><span class="cx">
</span><span class="cx"> bool Validate::addElse(ControlType& current, const ExpressionList& values)
</span><span class="cx"> {
</span><del>- if (current.type() != BlockType::If) {
- m_errorMessage = makeString("Attempting to add else block to something other than an if");
</del><ins>+ if (!unify(values, current)) {
+ ASSERT(errorMessage());
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (!unify(values, current)) {
- ASSERT(errorMessage());
</del><ins>+ return addElseToUnreachable(current);
+}
+
+bool Validate::addElseToUnreachable(ControlType& current)
+{
+ if (current.type() != BlockType::If) {
+ m_errorMessage = makeString("Attempting to add else block to something other than an if");
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -219,7 +231,7 @@
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (target.type() == BlockType::If)
</del><ins>+ if (target.type() == BlockType::Loop)
</ins><span class="cx"> return true;
</span><span class="cx">
</span><span class="cx"> if (target.signature() == Void)
</span><span class="lines">@@ -233,20 +245,33 @@
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool Validate::endBlock(ControlType& block, ExpressionList& stack)
</del><ins>+bool Validate::endBlock(ControlEntry& entry, ExpressionList& stack)
</ins><span class="cx"> {
</span><ins>+ ControlData& block = entry.controlData;
</ins><span class="cx"> if (block.signature() == Void)
</span><span class="cx"> return true;
</span><span class="cx">
</span><ins>+ if (!stack.size()) {
+ m_errorMessage = makeString("Block fallthough expected type: ", toString(block.signature()), " but the stack was empty");
+ return false;
+ }
</ins><span class="cx">
</span><del>- ASSERT(stack.size() == 1);
- if (block.signature() == stack[0])
</del><ins>+ if (block.signature() == stack.last()) {
+ entry.enclosedExpressionStack.append(block.signature());
</ins><span class="cx"> return true;
</span><ins>+ }
</ins><span class="cx">
</span><span class="cx"> m_errorMessage = makeString("Block fallthrough has expected type: ", toString(block.signature()), " but produced type: ", toString(block.signature()));
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool Validate::addEndToUnreachable(ControlEntry& entry)
+{
+ if (entry.controlData.signature() != Void)
+ entry.enclosedExpressionStack.append(entry.controlData.signature());
+ return true;
+}
+
</ins><span class="cx"> bool Validate::addCall(unsigned, const FunctionInformation& info, const Vector<ExpressionType>& args, ExpressionType& result)
</span><span class="cx"> {
</span><span class="cx"> if (info.signature->arguments.size() != args.size()) {
</span><span class="lines">@@ -288,9 +313,10 @@
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void Validate::dump(const Vector<ControlType>&, const ExpressionList&)
</del><ins>+void Validate::dump(const Vector<ControlEntry>&, const ExpressionList&)
</ins><span class="cx"> {
</span><del>- dataLogLn("Validating");
</del><ins>+ // If you need this then you should fix the validator's error messages instead...
+ // Think of this as penance for the sin of bad error messages.
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> String validateFunction(const uint8_t* source, size_t length, const Signature* signature, const Vector<FunctionInformation>& functions)
</span><span class="lines">@@ -299,6 +325,10 @@
</span><span class="cx"> FunctionParser<Validate> validator(context, source, length, signature, functions);
</span><span class="cx"> if (!validator.parse()) {
</span><span class="cx"> // FIXME: add better location information here. see: https://bugs.webkit.org/show_bug.cgi?id=164288
</span><ins>+ // FIXME: We should never not have an error message if we return false.
+ // see: https://bugs.webkit.org/show_bug.cgi?id=164354
+ if (context.errorMessage().isNull())
+ return "Unknown error";
</ins><span class="cx"> return context.errorMessage();
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>