Revert "Correctly handle instrumenting threads multiple times."
This CL is incorrect in multiple ways. First it is way too strict with
the checks about where additional methods can be. More importantly we
cannot bail-out early without breaking the instrumentation stack.
This is linked with the next CL that fixes this correctly.
This reverts commit 055407c8c0d7658d53fef595dec8ec8095797992.
Test: build.
Bug: 67384421
Bug: 74386428
Change-Id: I745a8e88a5789f68794faaa705c82ed81b8fb29f
(cherry picked from commit e9278663e0d6e6a0d2b0fd99f6d9f28b58773669)
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 7ddd173..b079598 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -209,9 +209,7 @@
: StackVisitor(thread_in, context, kInstrumentationStackWalk),
instrumentation_stack_(thread_in->GetInstrumentationStack()),
instrumentation_exit_pc_(instrumentation_exit_pc),
- reached_existing_instrumentation_frames_(false),
- should_be_at_top_(false),
- instrumentation_stack_depth_(0),
+ reached_existing_instrumentation_frames_(false), instrumentation_stack_depth_(0),
last_return_pc_(0) {
}
@@ -235,20 +233,6 @@
return true; // Continue.
}
uintptr_t return_pc = GetReturnPc();
- if (UNLIKELY(should_be_at_top_)) {
- std::string thread_name;
- GetThread()->GetThreadName(thread_name);
- uint32_t dex_pc = dex::kDexNoIndex;
- if (last_return_pc_ != 0 &&
- GetCurrentOatQuickMethodHeader() != nullptr) {
- dex_pc = GetCurrentOatQuickMethodHeader()->ToDexPc(m, last_return_pc_);
- }
- LOG(FATAL) << "While walking " << thread_name << " Reached unexpected frame above what "
- << "should have been top. Method is " << GetMethod()->PrettyMethod()
- << " return_pc: " << std::hex << return_pc
- << " dex pc: " << dex_pc;
- UNREACHABLE();
- }
if (kVerboseInstrumentation) {
LOG(INFO) << " Installing exit stub in " << DescribeLocation();
}
@@ -283,21 +267,22 @@
if (kVerboseInstrumentation) {
LOG(INFO) << "Ignoring already instrumented " << frame.Dump();
}
- } else if (UNLIKELY(reached_existing_instrumentation_frames_)) {
- // If tracing was enabled we might have had all methods have the instrumentation frame
- // except the runtime transition method at the very top of the stack. This isn't really a
- // problem since the transition method just goes back into the runtime and never leaves it
- // so it can be ignored.
- should_be_at_top_ = true;
- DCHECK(m->IsRuntimeMethod()) << "Expected method to be runtime method at start of thread "
- << "but was " << m->PrettyMethod();
- if (kVerboseInstrumentation) {
- LOG(INFO) << "reached expected top frame " << m->PrettyMethod();
- }
- // Don't bother continuing on the upcalls on non-debug builds.
- return kIsDebugBuild ? true : false;
} else {
CHECK_NE(return_pc, 0U);
+ if (UNLIKELY(reached_existing_instrumentation_frames_)) {
+ std::string thread_name;
+ GetThread()->GetThreadName(thread_name);
+ uint32_t dex_pc = dex::kDexNoIndex;
+ if (last_return_pc_ != 0 &&
+ GetCurrentOatQuickMethodHeader() != nullptr) {
+ dex_pc = GetCurrentOatQuickMethodHeader()->ToDexPc(m, last_return_pc_);
+ }
+ LOG(FATAL) << "While walking " << thread_name << " found existing instrumentation frames."
+ << " method is " << GetMethod()->PrettyMethod()
+ << " return_pc is " << std::hex << return_pc
+ << " dex pc: " << dex_pc;
+ UNREACHABLE();
+ }
InstrumentationStackFrame instrumentation_frame(
m->IsRuntimeMethod() ? nullptr : GetThisObject(),
m,
@@ -335,7 +320,6 @@
std::vector<uint32_t> dex_pcs_;
const uintptr_t instrumentation_exit_pc_;
bool reached_existing_instrumentation_frames_;
- bool should_be_at_top_;
size_t instrumentation_stack_depth_;
uintptr_t last_return_pc_;
};