Preserve exception message in iterator finalization.

The parser uses a try-catch in order to record when the client of an iterator
throws.  The exception then used to get rethrown via 'throw', which
unfortunately resulted in the original exception message object getting
overwritten.

This CL solves this as follows:
- add a clear_pending_message flag to TryCatchStatement (set to true in normal
  cases),
- set clear_pending_message to false for the TryCatchStatement used in iterator
  finalization
- change full-codegen, turbofan, and the interpreter to emit the ClearPendingMessage call
  only when the flag is set,
- replace 'throw' with '%ReThrow' in the iterator finalization code, thus
  reusing the (not-cleared) pending message

R=littledan@chromium.org, mstarzinger@chromium.org, yangguo@chromium.org
BUG=v8:4875
LOG=n

Review URL: https://codereview.chromium.org/1842953003

Cr-Commit-Position: refs/heads/master@{#35226}

src/ast/ast.h

@@ -1182,19 +1182,34 @@ class TryCatchStatement final : public TryStatement {
   Variable* variable() { return variable_; }
   Block* catch_block() const { return catch_block_; }
   void set_catch_block(Block* b) { catch_block_ = b; }
+  bool clear_pending_message() { return clear_pending_message_; }
+
+  // The clear_pending_message flag indicates whether or not to clear the
+  // isolate's pending exception message before executing the catch_block.  In
+  // the normal use case, this flag is always on because the message object
+  // is not needed anymore when entering the catch block and should not be kept
+  // alive.
+  // The use case where the flag is off is when the catch block is guaranteed to
+  // rethrow the caught exception (using %ReThrow), which reuses the pending
+  // message instead of generating a new one.
+  // (When the catch block doesn't rethrow but is guaranteed to perform an
+  // ordinary throw, not clearing the old message is safe but not very useful.)
 
  protected:
   TryCatchStatement(Zone* zone, Block* try_block, Scope* scope,
-                    Variable* variable, Block* catch_block, int pos)
+                    Variable* variable, Block* catch_block,
+                    bool clear_pending_message, int pos)
       : TryStatement(zone, try_block, pos),
         scope_(scope),
         variable_(variable),
-        catch_block_(catch_block) {}
+        catch_block_(catch_block),
+        clear_pending_message_(clear_pending_message) {}
 
  private:
   Scope* scope_;
   Variable* variable_;
   Block* catch_block_;
+  bool clear_pending_message_;
 };
 
 
@@ -3134,8 +3149,17 @@ class AstNodeFactory final BASE_EMBEDDED {
   TryCatchStatement* NewTryCatchStatement(Block* try_block, Scope* scope,
                                           Variable* variable,
                                           Block* catch_block, int pos) {
-    return new (local_zone_) TryCatchStatement(local_zone_, try_block, scope,
-                                               variable, catch_block, pos);
+    return new (local_zone_) TryCatchStatement(
+        local_zone_, try_block, scope, variable, catch_block, true, pos);
+  }
+
+  TryCatchStatement* NewTryCatchStatementForReThrow(Block* try_block,
+                                                    Scope* scope,
+                                                    Variable* variable,
+                                                    Block* catch_block,
+                                                    int pos) {
+    return new (local_zone_) TryCatchStatement(
+        local_zone_, try_block, scope, variable, catch_block, false, pos);
   }
 
   TryFinallyStatement* NewTryFinallyStatement(Block* try_block,

src/compiler/ast-graph-builder.cc

@@ -1442,9 +1442,11 @@ void AstGraphBuilder::VisitTryCatchStatement(TryCatchStatement* stmt) {
   }
   try_control.EndTry();
 
-  // Clear message object as we enter the catch block.
-  Node* the_hole = jsgraph()->TheHoleConstant();
-  NewNode(javascript()->StoreMessage(), the_hole);
+  // If requested, clear message object as we enter the catch block.
+  if (stmt->clear_pending_message()) {
+    Node* the_hole = jsgraph()->TheHoleConstant();
+    NewNode(javascript()->StoreMessage(), the_hole);
+  }
 
   // Create a catch scope that binds the exception.
   Node* exception = try_control.GetExceptionNode();

src/full-codegen/full-codegen.cc

@@ -1300,7 +1300,7 @@ void FullCodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) {
   Label try_entry, handler_entry, exit;
   __ jmp(&try_entry);
   __ bind(&handler_entry);
-  ClearPendingMessage();
+  if (stmt->clear_pending_message()) ClearPendingMessage();
 
   // Exception handler code, the exception is in the result register.
   // Extend the context before executing the catch block.

src/interpreter/bytecode-generator.cc

@@ -1199,8 +1199,10 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) {
   VisitNewLocalCatchContext(stmt->variable());
   builder()->StoreAccumulatorInRegister(context);
 
-  // Clear message object as we enter the catch block.
-  builder()->CallRuntime(Runtime::kInterpreterClearPendingMessage, no_reg, 0);
+  // If requested, clear message object as we enter the catch block.
+  if (stmt->clear_pending_message()) {
+    builder()->CallRuntime(Runtime::kInterpreterClearPendingMessage, no_reg, 0);
+  }
 
   // Load the catch context into the accumulator.
   builder()->LoadAccumulatorWithRegister(context);

src/parsing/parser.cc

@@ -6465,7 +6465,7 @@ void ParserTraits::FinalizeIteratorUse(Variable* completion,
   //       iterator_use
   //     } catch(e) {
   //       if (completion === kAbruptCompletion) completion = kThrowCompletion;
-  //       throw e;
+  //       %ReThrow(e);
   //     }
   //   } finally {
   //     if (condition) {
@@ -6526,7 +6526,7 @@ void ParserTraits::FinalizeIteratorUse(Variable* completion,
   // try { #try_block }
   // catch(e) {
   //   #set_completion_throw;
-  //   throw e;
+  //   %ReThrow(e);
   // }
   Statement* try_catch;
   {
@@ -6536,17 +6536,22 @@ void ParserTraits::FinalizeIteratorUse(Variable* completion,
                                   kCreatedInitialized, Variable::NORMAL);
 
     Statement* rethrow;
+    // We use %ReThrow rather than the ordinary throw because we want to
+    // preserve the original exception message.  This is also why we create a
+    // TryCatchStatementForReThrow below (which does not clear the pending
+    // message), rather than a TryCatchStatement.
     {
-      Expression* proxy = factory->NewVariableProxy(catch_variable);
-      rethrow = factory->NewExpressionStatement(factory->NewThrow(proxy, nopos),
-                                                nopos);
+      auto args = new (zone) ZoneList<Expression*>(1, zone);
+      args->Add(factory->NewVariableProxy(catch_variable), zone);
+      rethrow = factory->NewExpressionStatement(
+          factory->NewCallRuntime(Runtime::kReThrow, args, nopos), nopos);
     }
 
     Block* catch_block = factory->NewBlock(nullptr, 2, false, nopos);
     catch_block->statements()->Add(set_completion_throw, zone);
     catch_block->statements()->Add(rethrow, zone);
 
-    try_catch = factory->NewTryCatchStatement(
+    try_catch = factory->NewTryCatchStatementForReThrow(
         iterator_use, catch_scope, catch_variable, catch_block, nopos);
   }
 
@@ -6742,7 +6747,7 @@ Statement* ParserTraits::FinalizeForOfStatement(ForOfStatement* loop, int pos) {
   //       #loop;
   //     } catch(e) {
   //       if (completion === kAbruptCompletion) completion = kThrowCompletion;
-  //       throw e;
+  //       %ReThrow(e);
   //     }
   //   } finally {
   //     if (!(completion === kNormalCompletion || IS_UNDEFINED(#iterator))) {

test/cctest/interpreter/bytecode_expectations/ForOf.golden

@@ -13,7 +13,7 @@ snippet: "
 "
 frame size: 16
 parameter count: 1
-bytecode array length: 346
+bytecode array length: 347
 bytecodes: [
   B(StackCheck),
   B(LdaUndefined),
@@ -59,7 +59,7 @@ bytecodes: [
   B(LdaZero),
   B(Star), R(3),
   B(Jump), U8(-70),
-  B(Jump), U8(46),
+  B(Jump), U8(47),
   B(Star), R(14),
   B(LdaConstant), U8(5),
   B(Star), R(13),
@@ -67,8 +67,6 @@ bytecodes: [
   B(Star), R(15),
   B(CallRuntime), U16(Runtime::kPushCatchContext), R(13), U8(3),
   B(Star), R(12),
-  B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), R(0), U8(0),
-  B(Ldar), R(12),
   B(PushContext), R(8),
   B(Ldar), R(3),
   B(Star), R(13),
@@ -78,7 +76,9 @@ bytecodes: [
   B(LdaSmi), U8(1),
   B(Star), R(3),
   B(LdaContextSlot), R(context), U8(4),
-  B(Throw),
+  B(Star), R(13),
+  B(CallRuntime), U16(Runtime::kReThrow), R(13), U8(1),
+  B(PopContext), R(8),
   B(LdaSmi), U8(-1),
   B(Star), R(9),
   B(Jump), U8(7),
@@ -180,9 +180,9 @@ constant pool: [
   kInstanceTypeDontCare,
 ]
 handlers: [
-  [10, 151, 157],
+  [10, 152, 158],
   [13, 105, 107],
-  [249, 262, 264],
+  [250, 263, 265],
 ]
 
 ---
@@ -192,7 +192,7 @@ snippet: "
 "
 frame size: 17
 parameter count: 1
-bytecode array length: 362
+bytecode array length: 363
 bytecodes: [
   B(StackCheck),
   B(LdaConstant), U8(0),
@@ -240,9 +240,9 @@ bytecodes: [
   B(Star), R(11),
   B(LdaZero),
   B(Star), R(10),
-  B(Jump), U8(62),
+  B(Jump), U8(63),
   B(Jump), U8(-74),
-  B(Jump), U8(46),
+  B(Jump), U8(47),
   B(Star), R(15),
   B(LdaConstant), U8(5),
   B(Star), R(14),
@@ -250,8 +250,6 @@ bytecodes: [
   B(Star), R(16),
   B(CallRuntime), U16(Runtime::kPushCatchContext), R(14), U8(3),
   B(Star), R(13),
-  B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), R(0), U8(0),
-  B(Ldar), R(13),
   B(PushContext), R(9),
   B(Ldar), R(3),
   B(Star), R(14),
@@ -261,7 +259,9 @@ bytecodes: [
   B(LdaSmi), U8(1),
   B(Star), R(3),
   B(LdaContextSlot), R(context), U8(4),
-  B(Throw),
+  B(Star), R(14),
+  B(CallRuntime), U16(Runtime::kReThrow), R(14), U8(1),
+  B(PopContext), R(9),
   B(LdaSmi), U8(-1),
   B(Star), R(10),
   B(Jump), U8(8),
@@ -368,9 +368,9 @@ constant pool: [
   kInstanceTypeDontCare,
 ]
 handlers: [
-  [14, 157, 163],
+  [14, 158, 164],
   [17, 111, 113],
-  [256, 269, 271],
+  [257, 270, 272],
 ]
 
 ---
@@ -382,7 +382,7 @@ snippet: "
 "
 frame size: 16
 parameter count: 1
-bytecode array length: 368
+bytecode array length: 369
 bytecodes: [
   B(StackCheck),
   B(LdaUndefined),
@@ -439,7 +439,7 @@ bytecodes: [
   B(LdaZero),
   B(Star), R(3),
   B(Jump), U8(-92),
-  B(Jump), U8(46),
+  B(Jump), U8(47),
   B(Star), R(14),
   B(LdaConstant), U8(5),
   B(Star), R(13),
@@ -447,8 +447,6 @@ bytecodes: [
   B(Star), R(15),
   B(CallRuntime), U16(Runtime::kPushCatchContext), R(13), U8(3),
   B(Star), R(12),
-  B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), R(0), U8(0),
-  B(Ldar), R(12),
   B(PushContext), R(8),
   B(Ldar), R(3),
   B(Star), R(13),
@@ -458,7 +456,9 @@ bytecodes: [
   B(LdaSmi), U8(1),
   B(Star), R(3),
   B(LdaContextSlot), R(context), U8(4),
-  B(Throw),
+  B(Star), R(13),
+  B(CallRuntime), U16(Runtime::kReThrow), R(13), U8(1),
+  B(PopContext), R(8),
   B(LdaSmi), U8(-1),
   B(Star), R(9),
   B(Jump), U8(7),
@@ -560,9 +560,9 @@ constant pool: [
   kInstanceTypeDontCare,
 ]
 handlers: [
-  [10, 173, 179],
+  [10, 174, 180],
   [13, 127, 129],
-  [271, 284, 286],
+  [272, 285, 287],
 ]
 
 ---
@@ -572,7 +572,7 @@ snippet: "
 "
 frame size: 15
 parameter count: 1
-bytecode array length: 378
+bytecode array length: 379
 bytecodes: [
   B(StackCheck),
   B(CreateObjectLiteral), U8(0), U8(0), U8(5),
@@ -624,9 +624,9 @@ bytecodes: [
   B(Star), R(9),
   B(LdaZero),
   B(Star), R(8),
-  B(Jump), U8(62),
+  B(Jump), U8(63),
   B(Jump), U8(-84),
-  B(Jump), U8(46),
+  B(Jump), U8(47),
   B(Star), R(13),
   B(LdaConstant), U8(7),
   B(Star), R(12),
@@ -634,8 +634,6 @@ bytecodes: [
   B(Star), R(14),
   B(CallRuntime), U16(Runtime::kPushCatchContext), R(12), U8(3),
   B(Star), R(11),
-  B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), R(0), U8(0),
-  B(Ldar), R(11),
   B(PushContext), R(7),
   B(Ldar), R(2),
   B(Star), R(12),
@@ -645,7 +643,9 @@ bytecodes: [
   B(LdaSmi), U8(1),
   B(Star), R(2),
   B(LdaContextSlot), R(context), U8(4),
-  B(Throw),
+  B(Star), R(12),
+  B(CallRuntime), U16(Runtime::kReThrow), R(12), U8(1),
+  B(PopContext), R(7),
   B(LdaSmi), U8(-1),
   B(Star), R(8),
   B(Jump), U8(8),
@@ -754,8 +754,8 @@ constant pool: [
   kInstanceTypeDontCare,
 ]
 handlers: [
-  [18, 173, 179],
+  [18, 174, 180],
   [21, 127, 129],
-  [272, 285, 287],
+  [273, 286, 288],
 ]
 

test/message/for-of-throw-in-body.js

+// Copyright 2016 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.
+
+for (var x of [1, 2, 3]) { throw 42 }

test/message/for-of-throw-in-body.out

+# Copyright 2016 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.
+*%(basename)s:5: 42
+for (var x of [1, 2, 3]) { throw 42 }
+                           ^