From e6ff16d6bfb87fe21ffbf9120afa2be703e3327d Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Fri, 6 Mar 2015 02:15:49 -0800 Subject: [PATCH] [turbofan] Preserve pending message while inside finally-block. This makes sure that any pending message is saved before entering and restored after exiting a finally block. It also makes sure that operand stacks are kept in sync to full-codegen. R=bmeurer@chromium.org TEST=cctest/test-run-jsexceptions/ThrowMessage Review URL: https://codereview.chromium.org/979173002 Cr-Commit-Position: refs/heads/master@{#27036} --- src/compiler/ast-graph-builder.cc | 58 +++++++++++++++++++-------- src/compiler/ast-graph-builder.h | 4 ++ src/compiler/simplified-lowering.cc | 4 +- test/cctest/compiler/test-run-jsexceptions.cc | 44 +++++++++++++++++++- 4 files changed, 91 insertions(+), 19 deletions(-) diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 6cf419f..c2e7bc8 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -1252,6 +1252,13 @@ void AstGraphBuilder::VisitTryCatchStatement(TryCatchStatement* stmt) { void AstGraphBuilder::VisitTryFinallyStatement(TryFinallyStatement* stmt) { TryFinallyBuilder try_control(this); + ExternalReference message_object = + ExternalReference::address_of_pending_message_obj(isolate()); + ExternalReference message_present = + ExternalReference::address_of_has_pending_message(isolate()); + ExternalReference message_script = + ExternalReference::address_of_pending_message_script(isolate()); + // 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 // finally-block have been evaluated. @@ -1280,23 +1287,29 @@ void AstGraphBuilder::VisitTryFinallyStatement(TryFinallyStatement* stmt) { // - BreakStatement/ContinueStatement: Filled with the hole. // - Falling through into finally-block: Filled with the hole. Node* result = try_control.GetResultValueNode(); + Node* token = try_control.GetDispatchTokenNode(); - // TODO(mstarzinger): See FullCodeGenerator::EnterFinallyBlock. - environment()->Push(jsgraph()->SmiConstant(Code::kHeaderSize)); + // The result value, dispatch token and message is expected on the operand + // stack (this is in sync with FullCodeGenerator::EnterFinallyBlock). + environment()->Push(token); // TODO(mstarzinger): Cook token! environment()->Push(result); - environment()->Push(jsgraph()->TheHoleConstant()); // pending_message_obj - environment()->Push(jsgraph()->SmiConstant(0)); // has_pending_message - environment()->Push(jsgraph()->TheHoleConstant()); // pending_message_script + environment()->Push(BuildLoadExternal(message_object, kMachAnyTagged)); + environment()->Push(BuildLoadExternal(message_present, kMachBool)); + environment()->Push(BuildLoadExternal(message_script, kMachAnyTagged)); // Evaluate the finally-block. Visit(stmt->finally_block()); try_control.EndFinally(); - // TODO(mstarzinger): See FullCodeGenerator::ExitFinallyBlock. - environment()->Drop(5); + // The result value, dispatch token and message is restored from the operand + // stack (this is in sync with FullCodeGenerator::ExitFinallyBlock). + BuildStoreExternal(message_script, kMachAnyTagged, environment()->Pop()); + BuildStoreExternal(message_present, kMachBool, environment()->Pop()); + BuildStoreExternal(message_object, kMachAnyTagged, environment()->Pop()); + result = environment()->Pop(); + token = environment()->Pop(); // TODO(mstarzinger): Uncook token! // Dynamic dispatch after the finally-block. - Node* token = try_control.GetDispatchTokenNode(); commands->ApplyDeferredCommands(token, result); // TODO(mstarzinger): Remove bailout once everything works. @@ -2776,9 +2789,8 @@ Node* AstGraphBuilder::BuildVariableAssignment( Node* AstGraphBuilder::BuildLoadObjectField(Node* object, int offset) { - Node* field_load = NewNode(jsgraph()->machine()->Load(kMachAnyTagged), object, - jsgraph()->Int32Constant(offset - kHeapObjectTag)); - return field_load; + return NewNode(jsgraph()->machine()->Load(kMachAnyTagged), object, + jsgraph()->IntPtrConstant(offset - kHeapObjectTag)); } @@ -2805,6 +2817,23 @@ Node* AstGraphBuilder::BuildLoadGlobalProxy() { } +Node* AstGraphBuilder::BuildLoadExternal(ExternalReference reference, + MachineType type) { + return NewNode(jsgraph()->machine()->Load(type), + jsgraph()->ExternalConstant(reference), + jsgraph()->IntPtrConstant(0)); +} + + +Node* AstGraphBuilder::BuildStoreExternal(ExternalReference reference, + MachineType type, Node* value) { + StoreRepresentation representation(type, kNoWriteBarrier); + return NewNode(jsgraph()->machine()->Store(representation), + jsgraph()->ExternalConstant(reference), + jsgraph()->IntPtrConstant(0), value); +} + + Node* AstGraphBuilder::BuildToBoolean(Node* input) { // TODO(titzer): This should be in a JSOperatorReducer. switch (input->opcode()) { @@ -2940,11 +2969,8 @@ Node* AstGraphBuilder::BuildBinaryOp(Node* left, Node* right, Token::Value op) { Node* AstGraphBuilder::BuildStackCheck() { IfBuilder stack_check(this); - Node* limit = - NewNode(jsgraph()->machine()->Load(kMachPtr), - jsgraph()->ExternalConstant( - ExternalReference::address_of_stack_limit(isolate())), - jsgraph()->ZeroConstant()); + Node* limit = BuildLoadExternal( + ExternalReference::address_of_stack_limit(isolate()), kMachPtr); Node* stack = NewNode(jsgraph()->machine()->LoadStackPointer()); Node* tag = NewNode(jsgraph()->machine()->UintLessThan(), limit, stack); stack_check.If(tag, BranchHint::kTrue); diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h index 9ebefb5..b26abba 100644 --- a/src/compiler/ast-graph-builder.h +++ b/src/compiler/ast-graph-builder.h @@ -231,6 +231,10 @@ class AstGraphBuilder : public AstVisitor { Node* BuildLoadClosure(); Node* BuildLoadObjectField(Node* object, int offset); + // Builders for accessing external references. + Node* BuildLoadExternal(ExternalReference ref, MachineType type); + Node* BuildStoreExternal(ExternalReference ref, MachineType type, Node* val); + // Builders for automatic type conversion. Node* BuildToBoolean(Node* value); Node* BuildToName(Node* value, BailoutId bailout_id); diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index a757248..051d3fe 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc @@ -916,7 +916,7 @@ class RepresentationSelector { MachineTypeUnion tBase = kRepTagged | kMachPtr; LoadRepresentation rep = OpParameter(node); ProcessInput(node, 0, tBase); // pointer or object - ProcessInput(node, 1, kMachInt32); // index + ProcessInput(node, 1, kMachIntPtr); // index ProcessRemainingInputs(node, 2); SetOutput(node, rep); break; @@ -926,7 +926,7 @@ class RepresentationSelector { MachineTypeUnion tBase = kRepTagged | kMachPtr; StoreRepresentation rep = OpParameter(node); ProcessInput(node, 0, tBase); // pointer or object - ProcessInput(node, 1, kMachInt32); // index + ProcessInput(node, 1, kMachIntPtr); // index ProcessInput(node, 2, rep.machine_type()); ProcessRemainingInputs(node, 3); SetOutput(node, 0); diff --git a/test/cctest/compiler/test-run-jsexceptions.cc b/test/cctest/compiler/test-run-jsexceptions.cc index 5cafc34..9b31fe0 100644 --- a/test/cctest/compiler/test-run-jsexceptions.cc +++ b/test/cctest/compiler/test-run-jsexceptions.cc @@ -18,7 +18,7 @@ TEST(Throw) { } -TEST(ThrowSourcePosition) { +TEST(ThrowMessagePosition) { i::FLAG_turbo_exceptions = true; static const char* src = "(function(a, b) { \n" @@ -47,6 +47,48 @@ TEST(ThrowSourcePosition) { } +TEST(ThrowMessageDirectly) { + i::FLAG_turbo_exceptions = true; + static const char* src = + "(function(a, b) {" + " if (a) { throw b; } else { throw new Error(b); }" + "})"; + FunctionTester T(src); + v8::Handle message; + + message = T.CheckThrowsReturnMessage(T.false_value(), T.Val("Wat?")); + CHECK(!message.IsEmpty()); + CHECK(message->Get()->Equals(v8_str("Uncaught Error: Wat?"))); + + message = T.CheckThrowsReturnMessage(T.true_value(), T.Val("Kaboom!")); + CHECK(!message.IsEmpty()); + CHECK(message->Get()->Equals(v8_str("Uncaught Kaboom!"))); +} + + +TEST(ThrowMessageIndirectly) { + i::FLAG_turbo_exceptions = true; + static const char* src = + "(function(a, b) {" + " try {" + " if (a) { throw b; } else { throw new Error(b); }" + " } finally {" + " try { throw 'clobber'; } catch (e) { 'unclobber'; }" + " }" + "})"; + FunctionTester T(src); + v8::Handle message; + + message = T.CheckThrowsReturnMessage(T.false_value(), T.Val("Wat?")); + CHECK(!message.IsEmpty()); + CHECK(message->Get()->Equals(v8_str("Uncaught Error: Wat?"))); + + message = T.CheckThrowsReturnMessage(T.true_value(), T.Val("Kaboom!")); + CHECK(!message.IsEmpty()); + CHECK(message->Get()->Equals(v8_str("Uncaught Kaboom!"))); +} + + // TODO(mstarzinger): Increase test coverage by having similar tests within the // mjsunit suite to also test integration with other components (e.g. OSR). -- 2.7.4