From a7e5abff3424024f8938ec5c23467f481ec34bd3 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Wed, 5 Jul 2017 10:13:00 -0400 Subject: [PATCH] [ignition] handle catch prediction tracking entirely in BytecodeGenerator Remove catch prediction tracking from AstNumbering, and replace it with a similar mechanism in the BytecodeGenerator visitor. BUG=v8:4483, v8:5855 Change-Id: I6351ba311716102fa55cd9ef29b9955ab4b11027 Reviewed-on: https://chromium-review.googlesource.com/559006 Reviewed-by: Georg Neis Reviewed-by: Ross McIlroy Commit-Queue: Caitlin Potter Cr-Commit-Position: refs/heads/master@{#46419} --- src/ast/ast-numbering.cc | 46 +-------------- src/ast/ast.h | 85 +++++++++++++++------------ src/ast/prettyprinter.cc | 25 +++----- src/ast/prettyprinter.h | 1 - src/interpreter/bytecode-generator.cc | 54 +++++++++++++++-- src/interpreter/bytecode-generator.h | 11 ++++ 6 files changed, 119 insertions(+), 103 deletions(-) diff --git a/src/ast/ast-numbering.cc b/src/ast/ast-numbering.cc index ba4bcfe4d6..c5f2683443 100644 --- a/src/ast/ast-numbering.cc +++ b/src/ast/ast-numbering.cc @@ -26,7 +26,6 @@ class AstNumberingVisitor final : public AstVisitor { slot_cache_(zone), disable_fullcodegen_reason_(kNoReason), dont_optimize_reason_(kNoReason), - catch_prediction_(HandlerTable::UNCAUGHT), collect_type_profile_(collect_type_profile) { InitializeAstVisitor(stack_limit); } @@ -102,7 +101,6 @@ class AstNumberingVisitor final : public AstVisitor { FeedbackSlotCache slot_cache_; BailoutReason disable_fullcodegen_reason_; BailoutReason dont_optimize_reason_; - HandlerTable::CatchPrediction catch_prediction_; bool collect_type_profile_; DEFINE_AST_VISITOR_SUBCLASS_MEMBERS(); @@ -296,34 +294,6 @@ void AstNumberingVisitor::VisitFunctionDeclaration(FunctionDeclaration* node) { void AstNumberingVisitor::VisitCallRuntime(CallRuntime* node) { IncrementNodeCount(); VisitArguments(node->arguments()); - // To support catch prediction within async/await: - // - // The AstNumberingVisitor is when catch prediction currently occurs, and it - // is the only common point that has access to this information. The parser - // just doesn't know yet. Take the following two cases of catch prediction: - // - // try { await fn(); } catch (e) { } - // try { await fn(); } finally { } - // - // When parsing the await that we want to mark as caught or uncaught, it's - // not yet known whether it will be followed by a 'finally' or a 'catch. - // The AstNumberingVisitor is what learns whether it is caught. To make - // the information available later to the runtime, the AstNumberingVisitor - // has to stash it somewhere. Changing the runtime function into another - // one in ast-numbering seemed like a simple and straightforward solution to - // that problem. - if (node->is_jsruntime() && catch_prediction_ == HandlerTable::ASYNC_AWAIT) { - switch (node->context_index()) { - case Context::ASYNC_FUNCTION_AWAIT_CAUGHT_INDEX: - node->set_context_index(Context::ASYNC_FUNCTION_AWAIT_UNCAUGHT_INDEX); - break; - case Context::ASYNC_GENERATOR_AWAIT_CAUGHT: - node->set_context_index(Context::ASYNC_GENERATOR_AWAIT_UNCAUGHT); - break; - default: - break; - } - } } @@ -361,18 +331,7 @@ void AstNumberingVisitor::VisitTryCatchStatement(TryCatchStatement* node) { DCHECK(node->scope() == nullptr || !node->scope()->HasBeenRemoved()); IncrementNodeCount(); DisableFullCodegen(kTryCatchStatement); - { - const HandlerTable::CatchPrediction old_prediction = catch_prediction_; - // This node uses its own prediction, unless it's "uncaught", in which case - // we adopt the prediction of the outer try-block. - HandlerTable::CatchPrediction catch_prediction = node->catch_prediction(); - if (catch_prediction != HandlerTable::UNCAUGHT) { - catch_prediction_ = catch_prediction; - } - node->set_catch_prediction(catch_prediction_); - Visit(node->try_block()); - catch_prediction_ = old_prediction; - } + Visit(node->try_block()); Visit(node->catch_block()); } @@ -380,9 +339,6 @@ void AstNumberingVisitor::VisitTryCatchStatement(TryCatchStatement* node) { void AstNumberingVisitor::VisitTryFinallyStatement(TryFinallyStatement* node) { IncrementNodeCount(); DisableFullCodegen(kTryFinallyStatement); - // We can't know whether the finally block will override ("catch") an - // exception thrown in the try block, so we just adopt the outer prediction. - node->set_catch_prediction(catch_prediction_); Visit(node->try_block()); Visit(node->finally_block()); } diff --git a/src/ast/ast.h b/src/ast/ast.h index 1eff5b2386..6ea5d09ba9 100644 --- a/src/ast/ast.h +++ b/src/ast/ast.h @@ -971,30 +971,9 @@ class TryStatement : public Statement { Block* try_block() const { return try_block_; } void set_try_block(Block* b) { try_block_ = b; } - // Prediction of whether exceptions thrown into the handler for this try block - // will be caught. - // - // This is set in ast-numbering and later compiled into the code's handler - // table. The runtime uses this information to implement a feature that - // notifies the debugger when an uncaught exception is thrown, _before_ the - // exception propagates to the top. - // - // Since it's generally undecidable whether an exception will be caught, our - // prediction is only an approximation. - HandlerTable::CatchPrediction catch_prediction() const { - return catch_prediction_; - } - void set_catch_prediction(HandlerTable::CatchPrediction prediction) { - catch_prediction_ = prediction; - } - protected: TryStatement(Block* try_block, int pos, NodeType type) - : Statement(pos, type), - catch_prediction_(HandlerTable::UNCAUGHT), - try_block_(try_block) {} - - HandlerTable::CatchPrediction catch_prediction_; + : Statement(pos, type), try_block_(try_block) {} private: Block* try_block_; @@ -1007,18 +986,52 @@ class TryCatchStatement final : public TryStatement { Block* catch_block() const { return catch_block_; } void set_catch_block(Block* b) { catch_block_ = b; } - // 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. + // Prediction of whether exceptions thrown into the handler for this try block + // will be caught. + // + // BytecodeGenerator tracks the state of catch prediction, which can change + // with each TryCatchStatement encountered. The tracked catch prediction is + // later compiled into the code's handler table. The runtime uses this + // information to implement a feature that notifies the debugger when an + // uncaught exception is thrown, _before_ the exception propagates to the top. + // + // If this try/catch statement is meant to rethrow (HandlerTable::UNCAUGHT), + // the catch prediction value is set to the same value as the surrounding + // catch prediction. + // + // Since it's generally undecidable whether an exception will be caught, our + // prediction is only an approximation. + // --------------------------------------------------------------------------- + inline HandlerTable::CatchPrediction GetCatchPrediction( + HandlerTable::CatchPrediction outer_catch_prediction) const { + if (catch_prediction_ == HandlerTable::UNCAUGHT) { + return outer_catch_prediction; + } + return catch_prediction_; + } + + // Indicates whether or not code should be generated to clear the pending + // exception. The pending exception is cleared for cases where the exception + // is not guaranteed to be rethrown, indicated by the value + // HandlerTable::UNCAUGHT. If both the current and surrounding catch handler's + // are predicted uncaught, the exception is not cleared. + // + // If this handler is not going to simply rethrow the exception, this method + // indicates that the isolate's pending exception message should be cleared + // 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.) - bool clear_pending_message() const { - return catch_prediction_ != HandlerTable::UNCAUGHT; + // ordinary throw, not clearing the old message is safe but not very + // useful.) + inline bool ShouldClearPendingException( + HandlerTable::CatchPrediction outer_catch_prediction) const { + return catch_prediction_ != HandlerTable::UNCAUGHT || + outer_catch_prediction != HandlerTable::UNCAUGHT; } private: @@ -1028,12 +1041,12 @@ class TryCatchStatement final : public TryStatement { HandlerTable::CatchPrediction catch_prediction, int pos) : TryStatement(try_block, pos, kTryCatchStatement), scope_(scope), - catch_block_(catch_block) { - catch_prediction_ = catch_prediction; - } + catch_block_(catch_block), + catch_prediction_(catch_prediction) {} Scope* scope_; Block* catch_block_; + HandlerTable::CatchPrediction catch_prediction_; }; diff --git a/src/ast/prettyprinter.cc b/src/ast/prettyprinter.cc index 040305c6a2..7222328835 100644 --- a/src/ast/prettyprinter.cc +++ b/src/ast/prettyprinter.cc @@ -875,24 +875,9 @@ void AstPrinter::VisitForOfStatement(ForOfStatement* node) { void AstPrinter::VisitTryCatchStatement(TryCatchStatement* node) { IndentedScope indent(this, "TRY CATCH", node->position()); - PrintTryStatement(node); - PrintLiteralWithModeIndented("CATCHVAR", node->scope()->catch_variable(), - node->scope()->catch_variable()->name()); - PrintIndentedVisit("CATCH", node->catch_block()); -} - - -void AstPrinter::VisitTryFinallyStatement(TryFinallyStatement* node) { - IndentedScope indent(this, "TRY FINALLY", node->position()); - PrintTryStatement(node); - PrintIndentedVisit("FINALLY", node->finally_block()); -} - -void AstPrinter::PrintTryStatement(TryStatement* node) { - PrintIndentedVisit("TRY", node->try_block()); PrintIndented("CATCH PREDICTION"); const char* prediction = ""; - switch (node->catch_prediction()) { + switch (node->GetCatchPrediction(HandlerTable::UNCAUGHT)) { case HandlerTable::UNCAUGHT: prediction = "UNCAUGHT"; break; @@ -911,6 +896,14 @@ void AstPrinter::PrintTryStatement(TryStatement* node) { UNREACHABLE(); } Print(" %s\n", prediction); + PrintLiteralWithModeIndented("CATCHVAR", node->scope()->catch_variable(), + node->scope()->catch_variable()->name()); + PrintIndentedVisit("CATCH", node->catch_block()); +} + +void AstPrinter::VisitTryFinallyStatement(TryFinallyStatement* node) { + IndentedScope indent(this, "TRY FINALLY", node->position()); + PrintIndentedVisit("FINALLY", node->finally_block()); } void AstPrinter::VisitDebuggerStatement(DebuggerStatement* node) { diff --git a/src/ast/prettyprinter.h b/src/ast/prettyprinter.h index 847d9c86c0..8f5e4ae061 100644 --- a/src/ast/prettyprinter.h +++ b/src/ast/prettyprinter.h @@ -96,7 +96,6 @@ class AstPrinter final : public AstVisitor { void PrintLabelsIndented(ZoneList* labels); void PrintObjectProperties(ZoneList* properties); void PrintClassProperties(ZoneList* properties); - void PrintTryStatement(TryStatement* try_statement); void inc_indent() { indent_++; } void dec_indent() { indent_--; } diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 07025d24e3..b2528e73bb 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -798,7 +798,8 @@ BytecodeGenerator::BytecodeGenerator(CompilationInfo* info) generator_jump_table_(nullptr), generator_object_(), generator_state_(), - loop_depth_(0) { + loop_depth_(0), + catch_prediction_(HandlerTable::UNCAUGHT) { DCHECK_EQ(closure_scope(), closure_scope()->GetClosureScope()); if (info->is_block_coverage_enabled()) { DCHECK(FLAG_block_coverage); @@ -1576,7 +1577,13 @@ void BytecodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { } void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { - TryCatchBuilder try_control_builder(builder(), stmt->catch_prediction()); + // Update catch prediction tracking. The updated catch_prediction value lasts + // until the end of the try_block in the AST node, and does not apply to the + // catch_block. + HandlerTable::CatchPrediction outer_catch_prediction = catch_prediction(); + set_catch_prediction(stmt->GetCatchPrediction(outer_catch_prediction)); + + TryCatchBuilder try_control_builder(builder(), catch_prediction()); // Preserve the context in a dedicated register, so that it can be restored // when the handler is entered by the stack-unwinding machinery. @@ -1590,6 +1597,7 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { { ControlScopeForTryCatch scope(this, &try_control_builder); Visit(stmt->try_block()); + set_catch_prediction(outer_catch_prediction); } try_control_builder.EndTry(); @@ -1598,7 +1606,7 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { builder()->StoreAccumulatorInRegister(context); // If requested, clear message object as we enter the catch block. - if (stmt->clear_pending_message()) { + if (stmt->ShouldClearPendingException(outer_catch_prediction)) { builder()->LoadTheHole().SetPendingMessage(); } @@ -1611,7 +1619,9 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { } void BytecodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) { - TryFinallyBuilder try_control_builder(builder(), stmt->catch_prediction()); + // We can't know whether the finally block will override ("catch") an + // exception thrown in the try bblock, so we just adopt the outer prediction. + TryFinallyBuilder try_control_builder(builder(), catch_prediction()); // We keep a record of all paths that enter the finally-block to be able to // dispatch to the correct continuation point after the statements in the @@ -3249,15 +3259,49 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) { } } +int BytecodeGenerator::UpdateRuntimeFunctionForAsyncAwait(int context_index) { + // To support catch prediction within async/await: + // + // BytecodeGenerator models catch prediction (see VisitTryCatchstatement). + // Certain runtime calls need to be rewritten to invoke different functions, + // depending on the currently tracked catch prediction state. Take the + // following two cases of catch prediction: + // + // try { await fn(); } catch (e) { } + // try { await fn(); } finally { } + // + // When parsing the await that we want to mark as caught or uncaught, it's + // not yet known whether it will be followed by a 'finally' or a 'catch. + // The BytecodeGenerator has learned whether or not this Await is caught or + // not, and is responsible for invoking the correct function depending on + // those findings. It does that here. + if (catch_prediction() == HandlerTable::ASYNC_AWAIT) { + switch (context_index) { + case Context::ASYNC_FUNCTION_AWAIT_CAUGHT_INDEX: + context_index = Context::ASYNC_FUNCTION_AWAIT_UNCAUGHT_INDEX; + break; + case Context::ASYNC_GENERATOR_AWAIT_CAUGHT: + context_index = Context::ASYNC_GENERATOR_AWAIT_UNCAUGHT; + break; + default: + break; + } + } + return context_index; +} + void BytecodeGenerator::VisitCallRuntime(CallRuntime* expr) { if (expr->is_jsruntime()) { + int context_index = + UpdateRuntimeFunctionForAsyncAwait(expr->context_index()); + RegisterList args = register_allocator()->NewGrowableRegisterList(); // Allocate a register for the receiver and load it with undefined. // TODO(leszeks): If CallJSRuntime always has an undefined receiver, use the // same mechanism as CallUndefinedReceiver. BuildPushUndefinedIntoRegisterList(&args); VisitArguments(expr->arguments(), &args); - builder()->CallJSRuntime(expr->context_index(), args); + builder()->CallJSRuntime(context_index, args); } else { // Evaluate all arguments to the runtime call. RegisterList args = register_allocator()->NewGrowableRegisterList(); diff --git a/src/interpreter/bytecode-generator.h b/src/interpreter/bytecode-generator.h index 4cfa4734dc..bb59f01556 100644 --- a/src/interpreter/bytecode-generator.h +++ b/src/interpreter/bytecode-generator.h @@ -214,6 +214,8 @@ class BytecodeGenerator final : public AstVisitor { Register* out_register)); void VisitInSameTestExecutionScope(Expression* expr); + int UpdateRuntimeFunctionForAsyncAwait(int context_index); + // Returns the runtime function id for a store to super for the function's // language mode. inline Runtime::FunctionId StoreToSuperRuntimeId(); @@ -258,6 +260,13 @@ class BytecodeGenerator final : public AstVisitor { inline LanguageMode language_mode() const; int feedback_index(FeedbackSlot slot) const; + inline HandlerTable::CatchPrediction catch_prediction() const { + return catch_prediction_; + } + inline void set_catch_prediction(HandlerTable::CatchPrediction value) { + catch_prediction_ = value; + } + Zone* zone_; BytecodeArrayBuilder* builder_; CompilationInfo* info_; @@ -282,6 +291,8 @@ class BytecodeGenerator final : public AstVisitor { Register generator_object_; Register generator_state_; int loop_depth_; + + HandlerTable::CatchPrediction catch_prediction_; }; } // namespace interpreter