[ignition] More accurate dead statement elision

The Ignition statement list visitor will skip the rest of the
statements in the list if it hits a jump statement (like a return
or break), as the rest of the code in the list can be considered
dead.

    return;
    dead_call(); // skipped

However, since this is at an AST node level, it does not take into
account condition shortcutting:

    if(2.2) return;
    dead_call(); // not skipped

There is also a second dead code elimination in Ignition compilation, at
the bytecode array writer level, where a bytecodes are not emitted if an
"exit" bytecode (Return, Jump, or a few others) has been written, until
the next basic block starts (i.e. a Bind).

This can cause an issue with statements that resurrect the bytecode
array writer part-way through their visit. An example is try-catch
statements, which save the context to a register, and then Bind to start
the try region.

For the case:

    if (2.2) return;
    try {     // try statement not skipped
        ...
    }

the bytecode writer is called with

    OutputReturn()             // exit bytecode seen
    OutputMove(<context>, r1)  // not emitted
    Bind(&try_begin)           // starts new basic block
    // try body

So, the try is emitted, but without saving the context to a register.
This means that the liveness analysis sees the read of that register
(as the output liveness of throwing bytecodes), but does not have a
write to the register, which means that the liveness escapes.

This patch fixes this by using the bytecode array writer dead-code
elimination (i.e. "exit bytecode seen") to inform the statement list
visitor, so that in this example the try statement is not visited at
all.

Bug: chromium:902395
Change-Id: Ieb8e46a4318df3edbac0ae17235e0ce8fba12ee3
Reviewed-on: https://chromium-review.googlesource.com/c/1322951Reviewed-by: Mythri Alle <mythria@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57350}

src/interpreter/bytecode-array-builder.cc

@@ -1342,6 +1342,8 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::MarkHandler(
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::MarkTryBegin(int handler_id,
                                                          Register context) {
+  // TODO(leszeks): Do we need to start a new basic block here? Could we simply
+  // get the current bytecode offset from the array writer instead?
   BytecodeLabel try_begin;
   Bind(&try_begin);
   handler_table_builder()->SetTryRegionStart(handler_id, try_begin.offset());

src/interpreter/bytecode-array-builder.h

@@ -521,6 +521,9 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final {
   }
 
   bool RequiresImplicitReturn() const { return !return_seen_in_block_; }
+  bool RemainderOfBlockIsDead() const {
+    return bytecode_array_writer_.RemainderOfBlockIsDead();
+  }
 
   // Returns the raw operand value for the given register or register list.
   uint32_t GetInputRegisterOperand(Register reg);

src/interpreter/bytecode-array-writer.h

@@ -45,6 +45,8 @@ class V8_EXPORT_PRIVATE BytecodeArrayWriter final {
                                         int parameter_count,
                                         Handle<ByteArray> handler_table);
 
+  bool RemainderOfBlockIsDead() const { return exit_seen_in_block_; }
+
  private:
   // Maximum sized packed bytecode is comprised of a prefix bytecode,
   // plus the actual bytecode, plus the maximum number of operands times

src/interpreter/bytecode-generator.cc

@@ -1341,7 +1341,7 @@ void BytecodeGenerator::VisitStatements(
     RegisterAllocationScope allocation_scope(this);
     Statement* stmt = statements->at(i);
     Visit(stmt);
-    if (stmt->IsJump()) break;
+    if (builder()->RemainderOfBlockIsDead()) break;
   }
 }
 

test/mjsunit/regress/regress-crbug-902395.js

+// Copyright 2018 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.
+
+// Flags: --allow-natives-syntax
+
+function opt() {
+  try{
+    Object.seal({})
+  }finally{
+    try{
+      // Carefully crafted by clusterfuzz to alias the temporary object literal
+      // register with the below dead try block's context register.
+      (
+        {
+          toString(){
+          }
+        }
+      ).apply(-1).x(  )
+    }
+    finally{
+      if(2.2)
+      {
+        return
+      }
+      // This code should be dead.
+      try{
+        Reflect.construct
+      }finally{
+      }
+    }
+  }
+}
+
+opt();
+%OptimizeFunctionOnNextCall(opt);
+opt();