From e814457675d615aaebb14ae723a42c756492f732 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Wed, 3 Feb 2016 05:52:30 -0800 Subject: [PATCH] [interpreter] Clear pending message object on handler entry. This clears the currently pending message object whenever a try-block or a finally-block is being entered in interpreted code. The intention is to avoid memory leaks introduced by the message object. Also the message object is being restored when a finally-block exits. R=rmcilroy@chromium.org TEST=cctest/test-heap/MessageObjectLeak BUG=v8:4674 LOG=n Review URL: https://codereview.chromium.org/1651993002 Cr-Commit-Position: refs/heads/master@{#33704} --- src/interpreter/bytecode-generator.cc | 21 +- src/runtime/runtime-interpreter.cc | 16 + src/runtime/runtime.h | 4 +- test/cctest/cctest.status | 5 +- .../interpreter/test-bytecode-generator.cc | 313 ++++++++++-------- 5 files changed, 213 insertions(+), 146 deletions(-) diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 6c7ca344cb..42aa0f7c9f 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -1108,6 +1108,7 @@ void BytecodeGenerator::VisitForOfStatement(ForOfStatement* stmt) { void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { TryCatchBuilder try_control_builder(builder()); + Register no_reg; // Preserve the context in a dedicated register, so that it can be restored // when the handler is entered by the stack-unwinding machinery. @@ -1123,11 +1124,15 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { } try_control_builder.EndTry(); - // Clear message object as we enter the catch block. - // TODO(mstarzinger): Implement this! - // Create a catch scope that binds the exception. VisitNewLocalCatchContext(stmt->variable()); + builder()->StoreAccumulatorInRegister(context); + + // Clear message object as we enter the catch block. + builder()->CallRuntime(Runtime::kInterpreterClearPendingMessage, no_reg, 0); + + // Load the catch context into the accumulator. + builder()->LoadAccumulatorWithRegister(context); // Evaluate the catch-block. VisitInScope(stmt->catch_block(), stmt->scope()); @@ -1137,6 +1142,7 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { void BytecodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) { TryFinallyBuilder try_control_builder(builder()); + Register no_reg; // 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 @@ -1177,15 +1183,22 @@ void BytecodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) { try_control_builder.BeginHandler(); commands.RecordHandlerReThrowPath(); + // Pending message object is saved on entry. try_control_builder.BeginFinally(); + Register message = context; // Reuse register. // Clear message object as we enter the finally block. - // TODO(mstarzinger): Implement this! + builder() + ->CallRuntime(Runtime::kInterpreterClearPendingMessage, no_reg, 0) + .StoreAccumulatorInRegister(message); // Evaluate the finally-block. Visit(stmt->finally_block()); try_control_builder.EndFinally(); + // Pending message object is restored on exit. + builder()->CallRuntime(Runtime::kInterpreterSetPendingMessage, message, 1); + // Dynamic dispatch after the finally-block. commands.ApplyDeferredCommands(); } diff --git a/src/runtime/runtime-interpreter.cc b/src/runtime/runtime-interpreter.cc index 726190805c..9d65061ba6 100644 --- a/src/runtime/runtime-interpreter.cc +++ b/src/runtime/runtime-interpreter.cc @@ -253,5 +253,21 @@ RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeExit) { return isolate->heap()->undefined_value(); } +RUNTIME_FUNCTION(Runtime_InterpreterClearPendingMessage) { + SealHandleScope shs(isolate); + DCHECK_EQ(0, args.length()); + Object* message = isolate->thread_local_top()->pending_message_obj_; + isolate->clear_pending_message(); + return message; +} + +RUNTIME_FUNCTION(Runtime_InterpreterSetPendingMessage) { + SealHandleScope shs(isolate); + DCHECK_EQ(1, args.length()); + CONVERT_ARG_HANDLE_CHECKED(Object, message, 0); + isolate->thread_local_top()->pending_message_obj_ = *message; + return isolate->heap()->undefined_value(); +} + } // namespace internal } // namespace v8 diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 51cda541f0..dae44aeeb1 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -221,7 +221,9 @@ namespace internal { F(InterpreterTypeOf, 1, 1) \ F(InterpreterNewClosure, 2, 1) \ F(InterpreterTraceBytecodeEntry, 3, 1) \ - F(InterpreterTraceBytecodeExit, 3, 1) + F(InterpreterTraceBytecodeExit, 3, 1) \ + F(InterpreterClearPendingMessage, 0, 1) \ + F(InterpreterSetPendingMessage, 1, 1) #define FOR_EACH_INTRINSIC_FUNCTION(F) \ F(FunctionGetName, 1, 1) \ diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 3adbc64832..e448c05ec6 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -513,11 +513,8 @@ 'test-profile-generator/BailoutReason': [FAIL], 'test-api/Regress385349': [FAIL], - # TODO(mstarzinger,4674): Message object is not properly cleared. - 'test-heap/MessageObjectLeak': [FAIL], - # TODO(mstarzinger,4674): Support exception handlers in BytecodeGraphBuilder. - 'test-run-deopt/DeoptExceptionHandlerCatch': [FAIL], + 'test-run-deopt/DeoptExceptionHandlerCatch': [PASS, FAIL], # TODO(rmcilroy,4680): Check failed: toplevel_test_code_event_found. 'test-serialize/SerializeToplevelIsolates': [FAIL], diff --git a/test/cctest/interpreter/test-bytecode-generator.cc b/test/cctest/interpreter/test-bytecode-generator.cc index af83799c50..f56b4dca7d 100644 --- a/test/cctest/interpreter/test-bytecode-generator.cc +++ b/test/cctest/interpreter/test-bytecode-generator.cc @@ -4266,20 +4266,24 @@ TEST(TryCatch) { {"try { return 1; } catch(e) { return 2; }", 5 * kPointerSize, 1, - 27, + 36, { - B(LdaSmi8), U8(1), // - B(Return), // - B(Star), R(3), // - B(LdaConstant), U8(0), // - B(Star), R(2), // - B(Ldar), R(closure), // - B(Star), R(4), // - B(CallRuntime), U16(Runtime::kPushCatchContext), R(2), U8(3), // - B(PushContext), R(0), // - B(LdaSmi8), U8(2), // - B(PopContext), R(0), // - B(Return), // + B(LdaSmi8), U8(1), // + B(Return), // + B(Star), R(3), // + B(LdaConstant), U8(0), // + B(Star), R(2), // + B(Ldar), R(closure), // + B(Star), R(4), // + B(CallRuntime), U16(Runtime::kPushCatchContext), R(2), U8(3), // + B(Star), R(1), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Ldar), R(1), // + B(PushContext), R(0), // + B(LdaSmi8), U8(2), // + B(PopContext), R(0), // + B(Return), // // TODO(mstarzinger): Potential optimization, elide next bytes. B(LdaUndefined), // B(Return), // @@ -4291,39 +4295,47 @@ TEST(TryCatch) { {"var a; try { a = 1 } catch(e1) {}; try { a = 2 } catch(e2) { a = 3 }", 6 * kPointerSize, 1, - 56, + 74, { - B(LdaSmi8), U8(1), // - B(Star), R(0), // - B(Jump), U8(21), // - B(Star), R(4), // - B(LdaConstant), U8(0), // - B(Star), R(3), // - B(Ldar), R(closure), // - B(Star), R(5), // - B(CallRuntime), U16(Runtime::kPushCatchContext), R(3), U8(3), // - B(PushContext), R(1), // - B(PopContext), R(1), // - B(LdaSmi8), U8(2), // - B(Star), R(0), // - B(Jump), U8(25), // - B(Star), R(4), // - B(LdaConstant), U8(1), // - B(Star), R(3), // - B(Ldar), R(closure), // - B(Star), R(5), // - B(CallRuntime), U16(Runtime::kPushCatchContext), R(3), U8(3), // - B(PushContext), R(1), // - B(LdaSmi8), U8(3), // - B(Star), R(0), // - B(PopContext), R(1), // - B(LdaUndefined), // - B(Return), // + B(LdaSmi8), U8(1), // + B(Star), R(0), // + B(Jump), U8(30), // + B(Star), R(4), // + B(LdaConstant), U8(0), // + B(Star), R(3), // + B(Ldar), R(closure), // + B(Star), R(5), // + B(CallRuntime), U16(Runtime::kPushCatchContext), R(3), U8(3), // + B(Star), R(2), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Ldar), R(2), // + B(PushContext), R(1), // + B(PopContext), R(1), // + B(LdaSmi8), U8(2), // + B(Star), R(0), // + B(Jump), U8(34), // + B(Star), R(4), // + B(LdaConstant), U8(1), // + B(Star), R(3), // + B(Ldar), R(closure), // + B(Star), R(5), // + B(CallRuntime), U16(Runtime::kPushCatchContext), R(3), U8(3), // + B(Star), R(2), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Ldar), R(2), // + B(PushContext), R(1), // + B(LdaSmi8), U8(3), // + B(Star), R(0), // + B(PopContext), R(1), // + B(LdaUndefined), // + B(Return), // }, 2, {"e1", "e2"}, 2, - {{0, 4, 6}, {25, 29, 31}}}, + {{0, 4, 6}, {34, 38, 40}}}, }; for (size_t i = 0; i < arraysize(snippets); i++) { @@ -4344,28 +4356,33 @@ TEST(TryFinally) { {"var a = 1; try { a = 2; } finally { a = 3; }", 4 * kPointerSize, 1, - 35, + 47, { - B(LdaSmi8), U8(1), // - B(Star), R(0), // - B(LdaSmi8), U8(2), // - B(Star), R(0), // - B(LdaSmi8), U8(-1), // - B(Star), R(1), // - B(Jump), U8(7), // - B(Star), R(2), // - B(LdaZero), // - B(Star), R(1), // - B(LdaSmi8), U8(3), // - B(Star), R(0), // - B(LdaZero), // - B(TestEqualStrict), R(1), // - B(JumpIfTrue), U8(4), // - B(Jump), U8(5), // - B(Ldar), R(2), // - B(ReThrow), // - B(LdaUndefined), // - B(Return), // + B(LdaSmi8), U8(1), // + B(Star), R(0), // + B(LdaSmi8), U8(2), // + B(Star), R(0), // + B(LdaSmi8), U8(-1), // + B(Star), R(1), // + B(Jump), U8(7), // + B(Star), R(2), // + B(LdaZero), // + B(Star), R(1), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Star), R(3), // + B(LdaSmi8), U8(3), // + B(Star), R(0), // + B(CallRuntime), U16(Runtime::kInterpreterSetPendingMessage), // + /* */ R(3), U8(1), // + B(LdaZero), // + B(TestEqualStrict), R(1), // + B(JumpIfTrue), U8(4), // + B(Jump), U8(5), // + B(Ldar), R(2), // + B(ReThrow), // + B(LdaUndefined), // + B(Return), // }, 0, {}, @@ -4374,96 +4391,118 @@ TEST(TryFinally) { {"var a = 1; try { a = 2; } catch(e) { a = 20 } finally { a = 3; }", 9 * kPointerSize, 1, - 60, + 81, { - B(LdaSmi8), U8(1), // - B(Star), R(0), // - B(LdaSmi8), U8(2), // - B(Star), R(0), // - B(Jump), U8(25), // - B(Star), R(7), // - B(LdaConstant), U8(0), // - B(Star), R(6), // - B(Ldar), R(closure), // - B(Star), R(8), // - B(CallRuntime), U16(Runtime::kPushCatchContext), R(6), U8(3), // - B(PushContext), R(1), // - B(LdaSmi8), U8(20), // - B(Star), R(0), // - B(PopContext), R(1), // - B(LdaSmi8), U8(-1), // - B(Star), R(2), // - B(Jump), U8(7), // - B(Star), R(3), // - B(LdaZero), // - B(Star), R(2), // - B(LdaSmi8), U8(3), // - B(Star), R(0), // - B(LdaZero), // - B(TestEqualStrict), R(2), // - B(JumpIfTrue), U8(4), // - B(Jump), U8(5), // - B(Ldar), R(3), // - B(ReThrow), // - B(LdaUndefined), // - B(Return), // + B(LdaSmi8), U8(1), // + B(Star), R(0), // + B(LdaSmi8), U8(2), // + B(Star), R(0), // + B(Jump), U8(34), // + B(Star), R(7), // + B(LdaConstant), U8(0), // + B(Star), R(6), // + B(Ldar), R(closure), // + B(Star), R(8), // + B(CallRuntime), U16(Runtime::kPushCatchContext), R(6), U8(3), // + B(Star), R(5), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Ldar), R(5), // + B(PushContext), R(1), // + B(LdaSmi8), U8(20), // + B(Star), R(0), // + B(PopContext), R(1), // + B(LdaSmi8), U8(-1), // + B(Star), R(2), // + B(Jump), U8(7), // + B(Star), R(3), // + B(LdaZero), // + B(Star), R(2), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Star), R(4), // + B(LdaSmi8), U8(3), // + B(Star), R(0), // + B(CallRuntime), U16(Runtime::kInterpreterSetPendingMessage), // + /* */ R(4), U8(1), // + B(LdaZero), // + B(TestEqualStrict), R(2), // + B(JumpIfTrue), U8(4), // + B(Jump), U8(5), // + B(Ldar), R(3), // + B(ReThrow), // + B(LdaUndefined), // + B(Return), // }, 1, {"e"}, 2, - {{4, 33, 39}, {4, 8, 10}}}, + {{4, 42, 48}, {4, 8, 10}}}, {"var a; try {" " try { a = 1 } catch(e) { a = 2 }" "} catch(e) { a = 20 } finally { a = 3; }", 10 * kPointerSize, 1, - 81, + 111, { - B(LdaSmi8), U8(1), // - B(Star), R(0), // - B(Jump), U8(25), // - B(Star), R(8), // - B(LdaConstant), U8(0), // - B(Star), R(7), // - B(Ldar), R(closure), // - B(Star), R(9), // - B(CallRuntime), U16(Runtime::kPushCatchContext), R(7), U8(3), // - B(PushContext), R(1), // - B(LdaSmi8), U8(2), // - B(Star), R(0), // - B(PopContext), R(1), // - B(Jump), U8(25), // - B(Star), R(7), // - B(LdaConstant), U8(0), // - B(Star), R(6), // - B(Ldar), R(closure), // - B(Star), R(8), // - B(CallRuntime), U16(Runtime::kPushCatchContext), R(6), U8(3), // - B(PushContext), R(1), // - B(LdaSmi8), U8(20), // - B(Star), R(0), // - B(PopContext), R(1), // - B(LdaSmi8), U8(-1), // - B(Star), R(2), // - B(Jump), U8(7), // - B(Star), R(3), // - B(LdaZero), // - B(Star), R(2), // - B(LdaSmi8), U8(3), // - B(Star), R(0), // - B(LdaZero), // - B(TestEqualStrict), R(2), // - B(JumpIfTrue), U8(4), // - B(Jump), U8(5), // - B(Ldar), R(3), // - B(ReThrow), // - B(LdaUndefined), // - B(Return), // + B(LdaSmi8), U8(1), // + B(Star), R(0), // + B(Jump), U8(34), // + B(Star), R(8), // + B(LdaConstant), U8(0), // + B(Star), R(7), // + B(Ldar), R(closure), // + B(Star), R(9), // + B(CallRuntime), U16(Runtime::kPushCatchContext), R(7), U8(3), // + B(Star), R(6), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Ldar), R(6), // + B(PushContext), R(1), // + B(LdaSmi8), U8(2), // + B(Star), R(0), // + B(PopContext), R(1), // + B(Jump), U8(34), // + B(Star), R(7), // + B(LdaConstant), U8(0), // + B(Star), R(6), // + B(Ldar), R(closure), // + B(Star), R(8), // + B(CallRuntime), U16(Runtime::kPushCatchContext), R(6), U8(3), // + B(Star), R(5), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Ldar), R(5), // + B(PushContext), R(1), // + B(LdaSmi8), U8(20), // + B(Star), R(0), // + B(PopContext), R(1), // + B(LdaSmi8), U8(-1), // + B(Star), R(2), // + B(Jump), U8(7), // + B(Star), R(3), // + B(LdaZero), // + B(Star), R(2), // + B(CallRuntime), U16(Runtime::kInterpreterClearPendingMessage), // + /* */ R(0), U8(0), // + B(Star), R(4), // + B(LdaSmi8), U8(3), // + B(Star), R(0), // + B(CallRuntime), U16(Runtime::kInterpreterSetPendingMessage), // + /* */ R(4), U8(1), // + B(LdaZero), // + B(TestEqualStrict), R(2), // + B(JumpIfTrue), U8(4), // + B(Jump), U8(5), // + B(Ldar), R(3), // + B(ReThrow), // + B(LdaUndefined), // + B(Return), // }, 1, {"e"}, 3, - {{0, 54, 60}, {0, 29, 31}, {0, 4, 6}}}, + {{0, 72, 78}, {0, 38, 40}, {0, 4, 6}}}, }; for (size_t i = 0; i < arraysize(snippets); i++) {