[wasm] Fix detection of unreachable code

The current Control::unreachable flag served two things at the same
time: It tracked whether the label of that block is reachable (anyone
jumps / falls through to there), and it tracked whether that block was
ended by an unconditional branch or something similar.

It turns out that these two things cannot be tracked with a single
boolean per control struct. This CL introduces a Merge::reached flag,
which tracks whether a merge point was reached so far.
Also, the boolean flag to track unreachability of instructions within a
block is extended to track implicitly unreachable instructions.
According to the spec, a new block inside of unreachable code must be
validated as reachable code again, and also code after a block which
never returns has to be validated. It's not needed to generate code for
such instructions, however.
This new state will be particularly needed for the baseline compiler.

A follow-up CL will avoid calling interface methods for unreachable
code.

R=titzer@chromium.org

Bug: v8:6600
Change-Id: I54c97a19121eace65e25c448639330d06f61b1c8
Reviewed-on: https://chromium-review.googlesource.com/715637
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Rossberg <rossberg@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48509}

src/wasm/function-body-decoder-impl.h

@@ -413,13 +413,19 @@ struct Merge {
     Value first;
   } vals;  // Either multiple values or a single value.
 
+  // Tracks whether this merge was ever reached. Uses precise reachability, like
+  // Reachability::kReachable.
+  bool reached;
+
+  Merge(bool reached = false) : reached(reached) {}
+
   Value& operator[](uint32_t i) {
     DCHECK_GT(arity, i);
     return arity == 1 ? vals.first : vals.array[i];
   }
 };
 
-enum ControlKind {
+enum ControlKind : uint8_t {
   kControlIf,
   kControlIfElse,
   kControlBlock,
@@ -428,41 +434,69 @@ enum ControlKind {
   kControlTryCatch
 };
 
+enum Reachability : uint8_t {
+  // reachable code.
+  kReachable,
+  // reachable code in unreachable block (implies normal validation).
+  kSpecOnlyReachable,
+  // code unreachable in its own block (implies polymorphic validation).
+  kUnreachable
+};
+
 // An entry on the control stack (i.e. if, block, loop, or try).
 template <typename Value>
 struct ControlBase {
-  const byte* pc;
+  Reachability reachability = kReachable;
   ControlKind kind;
   uint32_t stack_depth;  // stack height at the beginning of the construct.
-  bool unreachable;    // The current block has been ended.
+  const byte* pc;
 
   // Values merged into the end of this control construct.
   Merge<Value> merge;
 
-  inline bool is_if() const { return is_onearmed_if() || is_if_else(); }
-  inline bool is_onearmed_if() const { return kind == kControlIf; }
-  inline bool is_if_else() const { return kind == kControlIfElse; }
-  inline bool is_block() const { return kind == kControlBlock; }
-  inline bool is_loop() const { return kind == kControlLoop; }
-  inline bool is_try() const { return is_incomplete_try() || is_try_catch(); }
-  inline bool is_incomplete_try() const { return kind == kControlTry; }
-  inline bool is_try_catch() const { return kind == kControlTryCatch; }
+  ControlBase() = default;
+  ControlBase(ControlKind kind, uint32_t stack_depth, const byte* pc,
+              bool merge_reached = false)
+      : kind(kind), stack_depth(stack_depth), pc(pc), merge(merge_reached) {}
+
+  // Check whether the current block is reachable.
+  bool reachable() const { return reachability == kReachable; }
+
+  // Check whether the rest of the block is unreachable.
+  // Note that this is different from {!reachable()}, as there is also the
+  // "indirect unreachable state", for which both {reachable()} and
+  // {unreachable()} return false.
+  bool unreachable() const { return reachability == kUnreachable; }
+
+  // Return the reachability of new control structs started in this block.
+  Reachability innerReachability() const {
+    return reachability == kReachable ? kReachable : kSpecOnlyReachable;
+  }
+
+  bool is_if() const { return is_onearmed_if() || is_if_else(); }
+  bool is_onearmed_if() const { return kind == kControlIf; }
+  bool is_if_else() const { return kind == kControlIfElse; }
+  bool is_block() const { return kind == kControlBlock; }
+  bool is_loop() const { return kind == kControlLoop; }
+  bool is_try() const { return is_incomplete_try() || is_try_catch(); }
+  bool is_incomplete_try() const { return kind == kControlTry; }
+  bool is_try_catch() const { return kind == kControlTryCatch; }
 
   // Named constructors.
-  static ControlBase Block(const byte* pc, size_t stack_depth) {
-    return {pc, kControlBlock, static_cast<uint32_t>(stack_depth), false, {}};
+  static ControlBase Block(const byte* pc, uint32_t stack_depth) {
+    return {kControlBlock, stack_depth, pc};
   }
 
-  static ControlBase If(const byte* pc, size_t stack_depth) {
-    return {pc, kControlIf, static_cast<uint32_t>(stack_depth), false, {}};
+  static ControlBase If(const byte* pc, uint32_t stack_depth) {
+    return {kControlIf, stack_depth, pc};
   }
 
-  static ControlBase Loop(const byte* pc, size_t stack_depth) {
-    return {pc, kControlLoop, static_cast<uint32_t>(stack_depth), false, {}};
+  static ControlBase Loop(const byte* pc, uint32_t stack_depth) {
+    return {kControlLoop, stack_depth, pc, true};
   }
 
-  static ControlBase Try(const byte* pc, size_t stack_depth) {
-    return {pc, kControlTry, static_cast<uint32_t>(stack_depth), false, {}};
+  static ControlBase Try(const byte* pc, uint32_t stack_depth) {
+    return {kControlTry, stack_depth, pc};
   }
 };
 
@@ -1277,6 +1311,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             std::vector<Value> args;
             PopArgs(operand.exception->ToFunctionSig(), &args);
             interface_.Throw(this, operand, &control_.back(), vec2vec(args));
+            EndControl();
             break;
           }
           case kExprTry: {
@@ -1322,6 +1357,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             Vector<Value> values(stack_.data() + c->stack_depth,
                                  sig->parameter_count());
             interface_.CatchException(this, operand, c, values);
+            c->reachability = control_at(1)->innerReachability();
             break;
           }
           case kExprCatchAll: {
@@ -1369,6 +1405,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             FallThruTo(c);
             stack_.resize(c->stack_depth);
             interface_.Else(this, c);
+            c->reachability = control_at(1)->innerReachability();
             break;
           }
           case kExprEnd: {
@@ -1384,8 +1421,10 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               break;
             }
             if (c->is_onearmed_if()) {
+              // The merge point is reached if the if is not taken.
+              c->merge.reached = true;
               // End the true branch of a one-armed if.
-              if (!VALIDATE(c->unreachable ||
+              if (!VALIDATE(c->unreachable() ||
                             stack_.size() == c->stack_depth)) {
                 this->error("end of if expected empty stack");
                 stack_.resize(c->stack_depth);
@@ -1431,6 +1470,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             Control* c = control_at(operand.depth);
             if (!TypeCheckBreak(c)) break;
             interface_.BreakTo(this, c);
+            if (control_.back().reachable()) c->merge.reached = true;
             len = 1 + operand.length;
             EndControl();
             break;
@@ -1442,6 +1482,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             Control* c = control_at(operand.depth);
             if (!TypeCheckBreak(c)) break;
             interface_.BrIf(this, cond, c);
+            if (control_.back().reachable()) c->merge.reached = true;
             len = 1 + operand.length;
             break;
           }
@@ -1471,6 +1512,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
                              i, br_arity, arity);
               }
               if (!TypeCheckBreak(c)) break;
+              if (control_.back().reachable()) c->merge.reached = true;
             }
             if (!VALIDATE(this->ok())) break;
 
@@ -1723,9 +1765,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
 #if DEBUG
       if (FLAG_trace_wasm_decoder) {
         PrintF(" ");
-        for (size_t i = 0; i < control_.size(); ++i) {
-          Control* c = &control_[i];
-          switch (c->kind) {
+        for (Control& c : control_) {
+          switch (c.kind) {
             case kControlIf:
               PrintF("I");
               break;
@@ -1741,8 +1782,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
             default:
               break;
           }
-          PrintF("%u", c->merge.arity);
-          if (c->unreachable) PrintF("*");
+          PrintF("%u", c.merge.arity);
+          if (!c.reachable()) PrintF("%c", c.unreachable() ? '*' : '#');
         }
         PrintF(" | ");
         for (size_t i = 0; i < stack_.size(); ++i) {
@@ -1789,8 +1830,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     DCHECK(!control_.empty());
     auto* current = &control_.back();
     stack_.resize(current->stack_depth);
-    current->unreachable = true;
     interface_.EndControl(this, current);
+    current->reachability = kUnreachable;
   }
 
   bool LookupBlockType(BlockTypeOperand<validate>* operand) {
@@ -1836,31 +1877,39 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     return sig->return_count() == 0 ? kWasmStmt : sig->GetReturn();
   }
 
-  Control* PushBlock() {
-    control_.emplace_back(Control::Block(this->pc_, stack_.size()));
-    return &control_.back();
+  Control* PushControl(Control&& new_control) {
+    Reachability reachability =
+        control_.empty() ? kReachable : control_.back().innerReachability();
+    control_.emplace_back(std::move(new_control));
+    Control* c = &control_.back();
+    c->reachability = reachability;
+    return c;
   }
 
+  Control* PushBlock() {
+    return PushControl(Control::Block(this->pc_, stack_size()));
+  }
   Control* PushLoop() {
-    control_.emplace_back(Control::Loop(this->pc_, stack_.size()));
-    return &control_.back();
+    return PushControl(Control::Loop(this->pc_, stack_size()));
   }
-
   Control* PushIf() {
-    control_.emplace_back(Control::If(this->pc_, stack_.size()));
-    return &control_.back();
+    return PushControl(Control::If(this->pc_, stack_size()));
   }
-
   Control* PushTry() {
-    control_.emplace_back(Control::Try(this->pc_, stack_.size()));
     // current_catch_ = static_cast<int32_t>(control_.size() - 1);
-    return &control_.back();
+    return PushControl(Control::Try(this->pc_, stack_size()));
   }
 
   void PopControl(Control* c) {
     DCHECK_EQ(c, &control_.back());
     interface_.PopControl(this, c);
+    bool reached = c->is_loop() ? c->reachable() : c->merge.reached;
     control_.pop_back();
+    // If the parent block was reachable before, but the popped control does not
+    // return to here, this block becomes indirectly unreachable.
+    if (!control_.empty() && !reached && control_.back().reachable()) {
+      control_.back().reachability = kSpecOnlyReachable;
+    }
   }
 
   int DecodeLoadMem(ValueType type, MachineType mem_type) {
@@ -2115,7 +2164,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     uint32_t limit = control_.back().stack_depth;
     if (stack_.size() <= limit) {
       // Popping past the current control start in reachable code.
-      if (!VALIDATE(control_.back().unreachable)) {
+      if (!VALIDATE(control_.back().unreachable())) {
         this->errorf(this->pc_, "%s found empty stack",
                      SafeOpcodeNameAt(this->pc_));
       }
@@ -2129,11 +2178,12 @@ class WasmFullDecoder : public WasmDecoder<validate> {
   int startrel(const byte* ptr) { return static_cast<int>(ptr - this->start_); }
 
   void FallThruTo(Control* c) {
+    DCHECK(!c->is_loop());
     DCHECK_EQ(c, &control_.back());
     if (!TypeCheckFallThru(c)) return;
-    c->unreachable = false;
 
     interface_.FallThruTo(this, c);
+    c->merge.reached = true;
   }
 
   bool TypeCheckMergeValues(Control* c) {
@@ -2201,7 +2251,7 @@ class WasmFullDecoder : public WasmDecoder<validate> {
     if (V8_LIKELY(actual >= expected)) {
       return true;  // enough actual values are there.
     }
-    if (!VALIDATE(control_.back().unreachable)) {
+    if (!VALIDATE(control_.back().unreachable())) {
       // There aren't enough values on the stack.
       return false;
     }