From 442371b9b2c52548acacaed6687ae2fa5d21c51e Mon Sep 17 00:00:00 2001 From: Mythri Alle Date: Mon, 6 Jun 2022 08:10:48 +0000 Subject: Skip method unwind callbacks if exception was thrown by method exit When a method exit callback is called the method is still on the stack. This is required because we should allow the method being exited to be force popped and re-execute the caller frame. The way it is implemented currently requires the method that is being exited on the stack. However, method exit callbacks could also throw which triggers a stack unwind. We should not call method unwind events for methods that already called the method exit event. This CL makes sure method unwind events are not called when an exception is thrown from method exit callbacks. Currently, method entry / exit callbacks force interpreter entry points so this isn't a problem. This is in preperation for future CLs that change this. Change-Id: I6124898acf0c20b8c9944c0e1e5b9f23c5633b14 --- runtime/quick_exception_handler.cc | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) (limited to 'runtime/quick_exception_handler.cc') diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc index f8bfb5a0ad..8adc3b3e49 100644 --- a/runtime/quick_exception_handler.cc +++ b/runtime/quick_exception_handler.cc @@ -68,12 +68,15 @@ class CatchBlockStackVisitor final : public StackVisitor { Context* context, Handle* exception, QuickExceptionHandler* exception_handler, - uint32_t skip_frames) + uint32_t skip_frames, + bool skip_top_unwind_callback) REQUIRES_SHARED(Locks::mutator_lock_) : StackVisitor(self, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames), exception_(exception), exception_handler_(exception_handler), - skip_frames_(skip_frames) { + skip_frames_(skip_frames), + skip_unwind_callback_(skip_top_unwind_callback) { + DCHECK_IMPLIES(skip_unwind_callback_, skip_frames_ == 0); } bool VisitFrame() override REQUIRES_SHARED(Locks::mutator_lock_) { @@ -101,9 +104,14 @@ class CatchBlockStackVisitor final : public StackVisitor { // can potentially throw, so we want to call these after we find the catch block. // We stop the stack walk when we find the catch block. If we are ending the stack walk we don't // have to unwind this method so don't record it. - if (continue_stack_walk) { + if (continue_stack_walk && !skip_unwind_callback_) { + // Skip unwind callback is only used when method exit callback has thrown an exception. In + // that case, we should have runtime method (artMethodExitHook) on top of stack and the + // second should be the method for which method exit was called. + DCHECK_IMPLIES(skip_unwind_callback_, GetFrameDepth() == 2); unwound_methods_.push(method); } + skip_unwind_callback_ = false; return continue_stack_walk; } @@ -155,13 +163,18 @@ class CatchBlockStackVisitor final : public StackVisitor { // The list of methods we would skip to reach the catch block. We record these to call // MethodUnwind callbacks. std::queue unwound_methods_; + // Specifies if the unwind callback should be ignored for method at the top of the stack. + bool skip_unwind_callback_; DISALLOW_COPY_AND_ASSIGN(CatchBlockStackVisitor); }; // Finds the appropriate exception catch after calling all method exit instrumentation functions. -// Note that this might change the exception being thrown. -void QuickExceptionHandler::FindCatch(ObjPtr exception) { +// Note that this might change the exception being thrown. If is_method_exit_exception is true +// skip the method unwind call for the method on top of the stack as the exception was thrown by +// method exit callback. +void QuickExceptionHandler::FindCatch(ObjPtr exception, + bool is_method_exit_exception) { DCHECK(!is_deoptimization_); instrumentation::Instrumentation* instr = Runtime::Current()->GetInstrumentation(); // The number of total frames we have so far popped. @@ -169,6 +182,7 @@ void QuickExceptionHandler::FindCatch(ObjPtr exception) { bool popped_to_top = true; StackHandleScope<1> hs(self_); MutableHandle exception_ref(hs.NewHandle(exception)); + bool skip_top_unwind_callback = is_method_exit_exception; // Sending the instrumentation events (done by the InstrumentationStackPopper) can cause new // exceptions to be thrown which will override the current exception. Therefore we need to perform // the search for a catch in a loop until we have successfully popped all the way to a catch or @@ -182,11 +196,15 @@ void QuickExceptionHandler::FindCatch(ObjPtr exception) { } // Walk the stack to find catch handler. - CatchBlockStackVisitor visitor(self_, context_, + CatchBlockStackVisitor visitor(self_, + context_, &exception_ref, this, - /*skip_frames=*/already_popped); + /*skip_frames=*/already_popped, + skip_top_unwind_callback); visitor.WalkStack(true); + skip_top_unwind_callback = false; + uint32_t new_pop_count = handler_frame_depth_; DCHECK_GE(new_pop_count, already_popped); already_popped = new_pop_count; -- cgit v1.2.3-59-g8ed1b