[wasm] Inspect right control frames for unreachable flag

src/wasm/function-body-decoder.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/signature.h"
 
 #include "src/base/platform/elapsed-timer.h"
 #include "src/bit-vector.h"
 #include "src/flags.h"
 #include "src/handles.h"
@@ skipping 71 matching lines @@
   explicit TryInfo(SsaEnv* c) : catch_env(c), exception(nullptr) {}
 };
 
 struct MergeValues {
   uint32_t arity;
   union {
     Value* array;
     Value first;
   } vals;  // Either multiple values or a single value.
 
-  Value& first() {
-    DCHECK_GT(arity, 0);
-    return arity == 1 ? vals.first : vals.array[0];
+  Value& operator[](uint32_t i) {
+    DCHECK_GT(arity, i);
+    return arity == 1 ? vals.first : vals.array[i];
   }
 };
 
 static Value* NO_VALUE = nullptr;
 
 enum ControlKind { kControlIf, kControlBlock, kControlLoop, kControlTry };
 
 // An entry on the control stack (i.e. if, block, loop).
 struct Control {
   const byte* pc;
@@ skipping 848 matching lines @@
               Value key = Pop(0, kWasmI32);
               if (failed()) break;
 
               SsaEnv* break_env = ssa_env_;
               if (operand.table_count > 0) {
                 // Build branches to the various blocks based on the table.
                 TFNode* sw = BUILD(Switch, operand.table_count + 1, key.node);
 
                 SsaEnv* copy = Steal(break_env);
                 ssa_env_ = copy;
+                MergeValues* merge = nullptr;
                 while (ok() && iterator.has_next()) {
                   uint32_t i = iterator.cur_index();
                   const byte* pos = iterator.pc();
                   uint32_t target = iterator.next();
                   if (target >= control_.size()) {
                     error(pos, "improper branch in br_table");
                     break;
                   }
                   ssa_env_ = Split(copy);
                   ssa_env_->control = (i == operand.table_count)
                                           ? BUILD(IfDefault, sw)
                                           : BUILD(IfValue, i, sw);
                   BreakTo(target);
+
+                  if (control_.back().unreachable) {

[titzer, 2017/02/14 13:30:27]

At first glance this logic seems best to put into BreakTo itself.

[rossberg, 2017/02/14 15:11:17]

Unfortunately, no, because it compares across targets (with the label type
remembered in `merge`).

+                    // Check that label types still match up.
+                    Control* c = &control_[control_.size() - target - 1];
+                    if (i == 0) {
+                      merge = &c->merge;
+                    } else if (merge->arity != c->merge.arity) {
+                      error(pos, pos, "inconsistent arity in br_table taret %d"

[titzer, 2017/02/14 13:30:27]

s/taret/target/

[rossberg, 2017/02/14 15:11:17]

Done.

+                            " (previous was %u, this one %u)",
+                            i, merge->arity, c->merge.arity);
+                    } else {
+                      for (uint32_t j = 0; ok() && j < merge->arity; ++j) {
+                        if ((*merge)[j].type != c->merge[j].type) {
+                          error(pos, pos,
+                                "type error in br_table target %d operand %d"
+                                " (previous expected %s, this one %s)", i, j,
+                                WasmOpcodes::TypeName((*merge)[j].type),
+                                WasmOpcodes::TypeName(c->merge[j].type));
+                        }
+                      }
+                    }
+                  }
                 }
                 if (failed()) break;
               } else {
                 // Only a default target. Do the equivalent of br.
                 const byte* pos = iterator.pc();
                 uint32_t target = iterator.next();
                 if (target >= control_.size()) {
                   error(pos, "improper branch in br_table");
                   break;
                 }
@@ skipping 514 matching lines @@
     EndControl();
   }
 
   void Push(ValueType type, TFNode* node) {
     if (type != kWasmStmt) {
       stack_.push_back({pc_, node, type});
     }
   }
 
   void PushEndValues(Control* c) {
+    DCHECK_EQ(c, &control_.back());
     stack_.resize(c->stack_depth);
     if (c->merge.arity == 1) {
       stack_.push_back(c->merge.vals.first);
     } else {
       for (unsigned i = 0; i < c->merge.arity; i++) {
         stack_.push_back(c->merge.vals.array[i]);
       }
     }
     DCHECK_EQ(c->stack_depth + c->merge.arity, stack_.size());
   }
@@ skipping 41 matching lines @@
 
   int startrel(const byte* ptr) { return static_cast<int>(ptr - start_); }
 
   void BreakTo(unsigned depth) {
     Control* c = &control_[control_.size() - depth - 1];
     if (c->is_loop()) {
       // This is the inner loop block, which does not have a value.
       Goto(ssa_env_, c->end_env);
     } else {
       // Merge the value(s) into the end of the block.
-      size_t expected = c->stack_depth + c->merge.arity;
-      if (!c->unreachable && stack_.size() < expected) {
+      size_t expected = control_.back().stack_depth + c->merge.arity;
+      if (stack_.size() < expected && !control_.back().unreachable) {
         error(
             pc_, pc_,
             "expected at least %u values on the stack for br to @%d, found %d",
             c->merge.arity, startrel(c->pc),
             static_cast<int>(stack_.size() - c->stack_depth));
         return;
       }
       MergeValuesInto(c);
     }
   }
 
   void FallThruTo(Control* c) {
+    DCHECK_EQ(c, &control_.back());
     // Merge the value(s) into the end of the block.
     size_t expected = c->stack_depth + c->merge.arity;
     if (stack_.size() == expected ||
-        (c->unreachable && stack_.size() < expected)) {
+        (stack_.size() < expected && c->unreachable)) {
       MergeValuesInto(c);
       c->unreachable = false;
       return;
     }
     error(pc_, pc_, "expected %u elements on the stack for fallthru to @%d",
           c->merge.arity, startrel(c->pc));
   }
 
   inline Value& GetMergeValueFromStack(Control* c, size_t i) {
     return stack_[stack_.size() - c->merge.arity + i];
   }
 
   void TypeCheckFallThru(Control* c) {
+    DCHECK_EQ(c, &control_.back());
     // Fallthru must match arity exactly.
     int arity = static_cast<int>(c->merge.arity);
     if (c->stack_depth + arity < stack_.size() ||
-        (!c->unreachable && c->stack_depth + arity != stack_.size())) {
+        (c->stack_depth + arity != stack_.size() && !c->unreachable)) {
       error(pc_, pc_, "expected %d elements on the stack for fallthru to @%d",
             arity, startrel(c->pc));
       return;
     }
     // Typecheck the values left on the stack.
     size_t avail = stack_.size() - c->stack_depth;
     for (size_t i = avail >= c->merge.arity ? 0 : c->merge.arity - avail;
          i < c->merge.arity; i++) {
       Value& val = GetMergeValueFromStack(c, i);
-      Value& old =
-          c->merge.arity == 1 ? c->merge.vals.first : c->merge.vals.array[i];
+      Value& old = c->merge[i];
       if (val.type != old.type) {
         error(pc_, pc_, "type error in merge[%zu] (expected %s, got %s)", i,
               WasmOpcodes::TypeName(old.type), WasmOpcodes::TypeName(val.type));
         return;
       }
     }
   }
 
   void MergeValuesInto(Control* c) {
     SsaEnv* target = c->end_env;
     bool first = target->state == SsaEnv::kUnreachable;
     bool reachable = ssa_env_->go();
     Goto(ssa_env_, target);
 
-    size_t avail = stack_.size() - c->stack_depth;
+    size_t avail = stack_.size() - control_.back().stack_depth;
     for (size_t i = avail >= c->merge.arity ? 0 : c->merge.arity - avail;
          i < c->merge.arity; i++) {
       Value& val = GetMergeValueFromStack(c, i);
-      Value& old =
-          c->merge.arity == 1 ? c->merge.vals.first : c->merge.vals.array[i];
+      Value& old = c->merge[i];
       if (val.type != old.type && val.type != kWasmVar) {
         error(pc_, pc_, "type error in merge[%zu] (expected %s, got %s)", i,
               WasmOpcodes::TypeName(old.type), WasmOpcodes::TypeName(val.type));
         return;
       }
       if (builder_ && reachable) {
         DCHECK_NOT_NULL(val.node);
         old.node =
             first ? val.node : CreateOrMergeIntoPhi(old.type, target->control,
                                                     old.node, val.node);
@@ skipping 482 matching lines @@
 BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, size_t num_locals,
                                            const byte* start, const byte* end) {
   Decoder decoder(start, end);
   return WasmDecoder::AnalyzeLoopAssignment(&decoder, start,
                                             static_cast<int>(num_locals), zone);
 }
 
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8