Correctly handle instrumenting threads multiple times.
We had a problem where if threads were instrumented multiple times we
would have CHECK failures as the final non-upcall frame would have an
unexpected return-pc. This changes the check logic to ensure that we
are robust with respect to that. This could happen when a thread was
instrumented while tracing was enabled.
Test: ./test.py --host --trace --ntrace --stream -j50
Bug: 67384421
Change-Id: Ic5aadf9e7db964aea653ac57a4d36eccbeac699d
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 0ae6dbf..b6055cb 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -209,7 +209,9 @@
: StackVisitor(thread_in, context, kInstrumentationStackWalk),
instrumentation_stack_(thread_in->GetInstrumentationStack()),
instrumentation_exit_pc_(instrumentation_exit_pc),
- reached_existing_instrumentation_frames_(false), instrumentation_stack_depth_(0),
+ reached_existing_instrumentation_frames_(false),
+ should_be_at_top_(false),
+ instrumentation_stack_depth_(0),
last_return_pc_(0) {
}
@@ -233,6 +235,20 @@
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();
}
@@ -267,22 +283,21 @@
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,
@@ -320,6 +335,7 @@
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_;
};
diff --git a/test/knownfailures.json b/test/knownfailures.json
index fe1e31e..100c288 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1,11 +1,5 @@
[
{
- "tests": "1934-jvmti-signal-thread",
- "description": ["Disables 1934-jvmti-signal-thread in tracing configurations"],
- "variant": "trace | stream",
- "bug": "http://b/67384421"
- },
- {
"tests": "153-reference-stress",
"description": ["Disable 153-reference-stress temporarily until a fix",
"arrives."],