Revert "Revert^2 "JVMTI PopFrame support""
This reverts commit 1c7b1fcf0ff29d83d13d38d0451a54474ccf5964.
Bug: 73255278
Bug: 111357976
bug: 117533193
Reason for revert: Test failures
Change-Id: I9da863fd95264007c4efeb85539e704e83499dcf
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 2ae95dc..df66061 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -261,12 +261,6 @@
shadow_frame.GetThisObject(accessor.InsSize()),
method,
0);
- if (UNLIKELY(shadow_frame.GetForcePopFrame())) {
- // The caller will retry this invoke. Just return immediately without any value.
- DCHECK(Runtime::Current()->AreNonStandardExitsEnabled());
- DCHECK(PrevFrameWillRetry(self, shadow_frame));
- return JValue();
- }
if (UNLIKELY(self->IsExceptionPending())) {
instrumentation->MethodUnwindEvent(self,
shadow_frame.GetThisObject(accessor.InsSize()),
@@ -500,8 +494,8 @@
JValue value;
// Set value to last known result in case the shadow frame chain is empty.
value.SetJ(ret_val->GetJ());
- // How many frames we have executed.
- size_t frame_cnt = 0;
+ // Are we executing the first shadow frame?
+ bool first = true;
while (shadow_frame != nullptr) {
// We do not want to recover lock state for lock counting when deoptimizing. Currently,
// the compiler should not have compiled a method that failed structured-locking checks.
@@ -516,30 +510,24 @@
// the instrumentation. To prevent from reporting it a second time, we simply pass a
// null Instrumentation*.
const instrumentation::Instrumentation* const instrumentation =
- frame_cnt == 0 ? nullptr : Runtime::Current()->GetInstrumentation();
+ first ? nullptr : Runtime::Current()->GetInstrumentation();
new_dex_pc = MoveToExceptionHandler(
self, *shadow_frame, instrumentation) ? shadow_frame->GetDexPC() : dex::kDexNoIndex;
} else if (!from_code) {
// Deoptimization is not called from code directly.
const Instruction* instr = &accessor.InstructionAt(dex_pc);
- if (deopt_method_type == DeoptimizationMethodType::kKeepDexPc ||
- shadow_frame->GetForceRetryInstruction()) {
- DCHECK(frame_cnt == 0 || (frame_cnt == 1 && shadow_frame->GetForceRetryInstruction()))
- << "frame_cnt: " << frame_cnt
- << " force-retry: " << shadow_frame->GetForceRetryInstruction();
+ if (deopt_method_type == DeoptimizationMethodType::kKeepDexPc) {
+ DCHECK(first);
// Need to re-execute the dex instruction.
// (1) An invocation might be split into class initialization and invoke.
// In this case, the invoke should not be skipped.
// (2) A suspend check should also execute the dex instruction at the
// corresponding dex pc.
- // If the ForceRetryInstruction bit is set this must be the second frame (the first being
- // the one that is being popped).
DCHECK_EQ(new_dex_pc, dex_pc);
- shadow_frame->SetForceRetryInstruction(false);
} else if (instr->Opcode() == Instruction::MONITOR_ENTER ||
instr->Opcode() == Instruction::MONITOR_EXIT) {
DCHECK(deopt_method_type == DeoptimizationMethodType::kDefault);
- DCHECK_EQ(frame_cnt, 0u);
+ DCHECK(first);
// Non-idempotent dex instruction should not be re-executed.
// On the other hand, if a MONITOR_ENTER is at the dex_pc of a suspend
// check, that MONITOR_ENTER should be executed. That case is handled
@@ -565,7 +553,7 @@
DCHECK_EQ(new_dex_pc, dex_pc);
} else {
DCHECK(deopt_method_type == DeoptimizationMethodType::kDefault);
- DCHECK_EQ(frame_cnt, 0u);
+ DCHECK(first);
// By default, we re-execute the dex instruction since if they are not
// an invoke, so that we don't have to decode the dex instruction to move
// result into the right vreg. All slow paths have been audited to be
@@ -578,7 +566,7 @@
} else {
// Nothing to do, the dex_pc is the one at which the code requested
// the deoptimization.
- DCHECK_EQ(frame_cnt, 0u);
+ DCHECK(first);
DCHECK_EQ(new_dex_pc, dex_pc);
}
if (new_dex_pc != dex::kDexNoIndex) {
@@ -597,7 +585,7 @@
// and should advance dex pc past the invoke instruction.
from_code = false;
deopt_method_type = DeoptimizationMethodType::kDefault;
- frame_cnt++;
+ first = false;
}
ret_val->SetJ(value.GetJ());
}
@@ -669,18 +657,5 @@
InitMterpTls(self);
}
-bool PrevFrameWillRetry(Thread* self, const ShadowFrame& frame) {
- ShadowFrame* prev_frame = frame.GetLink();
- if (prev_frame == nullptr) {
- NthCallerVisitor vis(self, 1, false);
- vis.WalkStack();
- prev_frame = vis.GetCurrentShadowFrame();
- if (prev_frame == nullptr) {
- prev_frame = self->FindDebuggerShadowFrame(vis.GetFrameId());
- }
- }
- return prev_frame != nullptr && prev_frame->GetForceRetryInstruction();
-}
-
} // namespace interpreter
} // namespace art
diff --git a/runtime/interpreter/interpreter.h b/runtime/interpreter/interpreter.h
index d7e69a6..0d43b90 100644
--- a/runtime/interpreter/interpreter.h
+++ b/runtime/interpreter/interpreter.h
@@ -69,12 +69,6 @@
void InitInterpreterTls(Thread* self);
-// Returns true if the previous frame has the ForceRetryInstruction bit set. This is required for
-// ForPopFrame to work correctly since that will cause the java function return with null/0 which
-// might not be expected by the code being run.
-bool PrevFrameWillRetry(Thread* self, const ShadowFrame& frame)
- REQUIRES_SHARED(Locks::mutator_lock_);
-
} // namespace interpreter
} // namespace art
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index dc3cc45..92d4731 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -371,12 +371,6 @@
if (UNLIKELY(self->IsExceptionPending())) {
return false;
}
- if (UNLIKELY(shadow_frame.GetForcePopFrame())) {
- // Don't actually set the field. The next instruction will force us to pop.
- DCHECK(Runtime::Current()->AreNonStandardExitsEnabled());
- DCHECK(PrevFrameWillRetry(self, shadow_frame));
- return true;
- }
}
// Note: iput-x-quick instructions are only for non-volatile fields.
switch (field_type) {
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index cb64ff4..04935cf 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -24,39 +24,16 @@
#include "interpreter_common.h"
#include "jit/jit.h"
#include "jvalue-inl.h"
-#include "nth_caller_visitor.h"
#include "safe_math.h"
#include "shadow_frame-inl.h"
-#include "thread.h"
namespace art {
namespace interpreter {
-#define CHECK_FORCE_RETURN() \
- do { \
- if (UNLIKELY(shadow_frame.GetForcePopFrame())) { \
- DCHECK(PrevFrameWillRetry(self, shadow_frame)) \
- << "Pop frame forced without previous frame ready to retry instruction!"; \
- DCHECK(Runtime::Current()->AreNonStandardExitsEnabled()); \
- if (UNLIKELY(NeedsMethodExitEvent(instrumentation))) { \
- SendMethodExitEvents(self, \
- instrumentation, \
- shadow_frame, \
- shadow_frame.GetThisObject(accessor.InsSize()), \
- shadow_frame.GetMethod(), \
- inst->GetDexPc(insns), \
- JValue()); \
- } \
- ctx->result = JValue(); /* Handled in caller. */ \
- return; \
- } \
- } while (false)
-
#define HANDLE_PENDING_EXCEPTION_WITH_INSTRUMENTATION(instr) \
do { \
DCHECK(self->IsExceptionPending()); \
self->AllowThreadSuspension(); \
- CHECK_FORCE_RETURN(); \
if (!MoveToExceptionHandler(self, shadow_frame, instr)) { \
/* Structured locking is to be enforced for abnormal termination, too. */ \
DoMonitorCheckOnExit<do_assignability_check>(self, &shadow_frame); \
@@ -67,7 +44,6 @@
ctx->result = JValue(); /* Handled in caller. */ \
return; \
} else { \
- CHECK_FORCE_RETURN(); \
int32_t displacement = \
static_cast<int32_t>(shadow_frame.GetDexPC()) - static_cast<int32_t>(dex_pc); \
inst = inst->RelativeAt(displacement); \
@@ -76,39 +52,8 @@
#define HANDLE_PENDING_EXCEPTION() HANDLE_PENDING_EXCEPTION_WITH_INSTRUMENTATION(instrumentation)
-#define POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE_IMPL(_is_exception_pending, _next_function) \
- do { \
- if (UNLIKELY(shadow_frame.GetForceRetryInstruction())) { \
- /* Don't need to do anything except clear the flag and exception. We leave the */ \
- /* instruction the same so it will be re-executed on the next go-around. */ \
- DCHECK(inst->IsInvoke()); \
- shadow_frame.SetForceRetryInstruction(false); \
- if (UNLIKELY(_is_exception_pending)) { \
- DCHECK(self->IsExceptionPending()); \
- if (kIsDebugBuild) { \
- LOG(WARNING) << "Suppressing exception for instruction-retry: " \
- << self->GetException()->Dump(); \
- } \
- self->ClearException(); \
- } \
- } else if (UNLIKELY(_is_exception_pending)) { \
- /* Should have succeeded. */ \
- DCHECK(!shadow_frame.GetForceRetryInstruction()); \
- HANDLE_PENDING_EXCEPTION(); \
- } else { \
- inst = inst->_next_function(); \
- } \
- } while (false)
-
-#define POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE_POLYMORPHIC(_is_exception_pending) \
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE_IMPL(_is_exception_pending, Next_4xx)
-#define POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(_is_exception_pending) \
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE_IMPL(_is_exception_pending, Next_3xx)
-
#define POSSIBLY_HANDLE_PENDING_EXCEPTION(_is_exception_pending, _next_function) \
do { \
- /* Should only be on invoke instructions. */ \
- DCHECK(!shadow_frame.GetForceRetryInstruction()); \
if (UNLIKELY(_is_exception_pending)) { \
HANDLE_PENDING_EXCEPTION(); \
} else { \
@@ -122,22 +67,17 @@
}
// Code to run before each dex instruction.
-#define PREAMBLE_SAVE(save_ref) \
+#define PREAMBLE_SAVE(save_ref) \
{ \
- /* We need to put this before & after the instrumentation to avoid having to put in a */ \
- /* post-script macro. */ \
- CHECK_FORCE_RETURN(); \
- if (UNLIKELY(instrumentation->HasDexPcListeners())) { \
- if (UNLIKELY(!DoDexPcMoveEvent(self, \
- accessor, \
- shadow_frame, \
- dex_pc, \
- instrumentation, \
- save_ref))) { \
- HANDLE_PENDING_EXCEPTION(); \
- break; \
- } \
- CHECK_FORCE_RETURN(); \
+ if (UNLIKELY(instrumentation->HasDexPcListeners()) && \
+ UNLIKELY(!DoDexPcMoveEvent(self, \
+ accessor, \
+ shadow_frame, \
+ dex_pc, \
+ instrumentation, \
+ save_ref))) { \
+ HANDLE_PENDING_EXCEPTION(); \
+ break; \
} \
} \
do {} while (false)
@@ -241,8 +181,7 @@
const JValue& result)
REQUIRES_SHARED(Locks::mutator_lock_) {
bool had_event = false;
- // We don't send method-exit if it's a pop-frame. We still send frame_popped though.
- if (UNLIKELY(instrumentation->HasMethodExitListeners() && !frame.GetForcePopFrame())) {
+ if (UNLIKELY(instrumentation->HasMethodExitListeners())) {
had_event = true;
instrumentation->MethodExitEvent(self, thiz.Ptr(), method, dex_pc, result);
}
@@ -282,9 +221,6 @@
uint16_t inst_data;
jit::Jit* jit = Runtime::Current()->GetJit();
- DCHECK(!shadow_frame.GetForceRetryInstruction())
- << "Entered interpreter from invoke without retry instruction being handled!";
-
do {
dex_pc = inst->GetDexPc(insns);
shadow_frame.SetDexPC(dex_pc);
@@ -1671,84 +1607,84 @@
PREAMBLE();
bool success = DoInvoke<kVirtual, false, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_VIRTUAL_RANGE: {
PREAMBLE();
bool success = DoInvoke<kVirtual, true, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_SUPER: {
PREAMBLE();
bool success = DoInvoke<kSuper, false, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_SUPER_RANGE: {
PREAMBLE();
bool success = DoInvoke<kSuper, true, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_DIRECT: {
PREAMBLE();
bool success = DoInvoke<kDirect, false, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_DIRECT_RANGE: {
PREAMBLE();
bool success = DoInvoke<kDirect, true, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_INTERFACE: {
PREAMBLE();
bool success = DoInvoke<kInterface, false, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_INTERFACE_RANGE: {
PREAMBLE();
bool success = DoInvoke<kInterface, true, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_STATIC: {
PREAMBLE();
bool success = DoInvoke<kStatic, false, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_STATIC_RANGE: {
PREAMBLE();
bool success = DoInvoke<kStatic, true, do_access_check>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_VIRTUAL_QUICK: {
PREAMBLE();
bool success = DoInvokeVirtualQuick<false>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_VIRTUAL_RANGE_QUICK: {
PREAMBLE();
bool success = DoInvokeVirtualQuick<true>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_POLYMORPHIC: {
@@ -1756,7 +1692,7 @@
DCHECK(Runtime::Current()->IsMethodHandlesEnabled());
bool success = DoInvokePolymorphic<false /* is_range */>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE_POLYMORPHIC(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_4xx);
break;
}
case Instruction::INVOKE_POLYMORPHIC_RANGE: {
@@ -1764,7 +1700,7 @@
DCHECK(Runtime::Current()->IsMethodHandlesEnabled());
bool success = DoInvokePolymorphic<true /* is_range */>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE_POLYMORPHIC(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_4xx);
break;
}
case Instruction::INVOKE_CUSTOM: {
@@ -1772,7 +1708,7 @@
DCHECK(Runtime::Current()->IsMethodHandlesEnabled());
bool success = DoInvokeCustom<false /* is_range */>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::INVOKE_CUSTOM_RANGE: {
@@ -1780,7 +1716,7 @@
DCHECK(Runtime::Current()->IsMethodHandlesEnabled());
bool success = DoInvokeCustom<true /* is_range */>(
self, shadow_frame, inst, inst_data, &result_register);
- POSSIBLY_HANDLE_PENDING_EXCEPTION_ON_INVOKE(!success);
+ POSSIBLY_HANDLE_PENDING_EXCEPTION(!success, Next_3xx);
break;
}
case Instruction::NEG_INT:
diff --git a/runtime/interpreter/mterp/mterp.cc b/runtime/interpreter/mterp/mterp.cc
index c385fb9..fbc96f7 100644
--- a/runtime/interpreter/mterp/mterp.cc
+++ b/runtime/interpreter/mterp/mterp.cc
@@ -152,11 +152,6 @@
const instrumentation::Instrumentation* const instrumentation = runtime->GetInstrumentation();
return instrumentation->NonJitProfilingActive() ||
Dbg::IsDebuggerActive() ||
- // mterp only knows how to deal with the normal exits. It cannot handle any of the
- // non-standard force-returns.
- // TODO We really only need to switch interpreters if a PopFrame has actually happened. We
- // should check this here.
- UNLIKELY(runtime->AreNonStandardExitsEnabled()) ||
// An async exception has been thrown. We need to go to the switch interpreter. MTerp doesn't
// know how to deal with these so we could end up never dealing with it if we are in an
// infinite loop. Since this can be called in a tight loop and getting the current thread
diff --git a/runtime/interpreter/shadow_frame.h b/runtime/interpreter/shadow_frame.h
index 9854f8f..91371d1 100644
--- a/runtime/interpreter/shadow_frame.h
+++ b/runtime/interpreter/shadow_frame.h
@@ -49,17 +49,6 @@
// - interpreter - separate VRegs and reference arrays. References are in the reference array.
// - JNI - just VRegs, but where every VReg holds a reference.
class ShadowFrame {
- private:
- // Used to keep track of extra state the shadowframe has.
- enum class FrameFlags : uint32_t {
- // We have been requested to notify when this frame gets popped.
- kNotifyFramePop = 1 << 0,
- // We have been asked to pop this frame off the stack as soon as possible.
- kForcePopFrame = 1 << 1,
- // We have been asked to re-execute the last instruction.
- kForceRetryInst = 1 << 2,
- };
-
public:
// Compute size of ShadowFrame in bytes assuming it has a reference array.
static size_t ComputeSize(uint32_t num_vregs) {
@@ -356,27 +345,11 @@
}
bool NeedsNotifyPop() const {
- return GetFrameFlag(FrameFlags::kNotifyFramePop);
+ return needs_notify_pop_;
}
void SetNotifyPop(bool notify) {
- UpdateFrameFlag(notify, FrameFlags::kNotifyFramePop);
- }
-
- bool GetForcePopFrame() const {
- return GetFrameFlag(FrameFlags::kForcePopFrame);
- }
-
- void SetForcePopFrame(bool enable) {
- UpdateFrameFlag(enable, FrameFlags::kForcePopFrame);
- }
-
- bool GetForceRetryInstruction() const {
- return GetFrameFlag(FrameFlags::kForceRetryInst);
- }
-
- void SetForceRetryInstruction(bool enable) {
- UpdateFrameFlag(enable, FrameFlags::kForceRetryInst);
+ needs_notify_pop_ = notify;
}
private:
@@ -391,7 +364,7 @@
dex_pc_(dex_pc),
cached_hotness_countdown_(0),
hotness_countdown_(0),
- frame_flags_(0) {
+ needs_notify_pop_(0) {
// TODO(iam): Remove this parameter, it's an an artifact of portable removal
DCHECK(has_reference_array);
if (has_reference_array) {
@@ -401,18 +374,6 @@
}
}
- void UpdateFrameFlag(bool enable, FrameFlags flag) {
- if (enable) {
- frame_flags_ |= static_cast<uint32_t>(flag);
- } else {
- frame_flags_ &= ~static_cast<uint32_t>(flag);
- }
- }
-
- bool GetFrameFlag(FrameFlags flag) const {
- return (frame_flags_ & static_cast<uint32_t>(flag)) != 0;
- }
-
const StackReference<mirror::Object>* References() const {
DCHECK(HasReferenceArray());
const uint32_t* vreg_end = &vregs_[NumberOfVRegs()];
@@ -436,11 +397,9 @@
uint32_t dex_pc_;
int16_t cached_hotness_countdown_;
int16_t hotness_countdown_;
-
- // This is a set of ShadowFrame::FrameFlags which denote special states this frame is in.
- // NB alignment requires that this field takes 4 bytes no matter its size. Only 3 bits are
- // currently used.
- uint32_t frame_flags_;
+ // TODO Might be worth it to try to bit-pack this into some other field to reduce stack usage.
+ // NB alignment requires that this field takes 4 bytes. Only 1 bit is actually ever used.
+ bool needs_notify_pop_;
// This is a two-part array:
// - [0..number_of_vregs) holds the raw virtual registers, and each element here is always 4