Fixes to instrumentation for debug build and interpreter.

- Stub uninstall will put back the interpreter entry point if Xint
  mode is specified.
- Copy method entry and exit listeners before iterating over them
  to prevent problems that occur when they are modified during
  iteration.
- Corrected checks from WalkStack and AssertPcIsWithinCode to
  handle instrumented code since they are used to remove the stubs.

Change-Id: Ib0e2b421e6b56d520e4643699624dd80ee5148e3

4 files changed, 40 insertions, 17 deletions

diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 8598d6d..091f66a 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -59,7 +59,9 @@ bool Instrumentation::InstallStubsForClass(mirror::Class* klass) {
     if (!method->IsAbstract()) {
       const void* new_code;
       if (uninstall) {
-        if (is_initialized || !method->IsStatic() || method->IsConstructor()) {
+        if (forced_interpret_only_ && !method->IsNative() && !method->IsProxyMethod()) {
+          new_code = GetInterpreterEntryPoint();
+        } else if (is_initialized || !method->IsStatic() || method->IsConstructor()) {
           new_code = class_linker->GetOatCodeFor(method);
         } else {
           new_code = GetResolutionTrampoline(class_linker);
@@ -79,7 +81,11 @@ bool Instrumentation::InstallStubsForClass(mirror::Class* klass) {
     if (!method->IsAbstract()) {
       const void* new_code;
       if (uninstall) {
-        new_code = class_linker->GetOatCodeFor(method);
+        if (forced_interpret_only_ && !method->IsNative() && !method->IsProxyMethod()) {
+          new_code = GetInterpreterEntryPoint();
+        } else {
+          new_code = class_linker->GetOatCodeFor(method);
+        }
       } else {  // !uninstall
         if (!interpreter_stubs_installed_ || method->IsNative()) {
           new_code = GetInstrumentationEntryPoint();
@@ -376,6 +382,12 @@ void Instrumentation::ConfigureStubs(bool require_entry_exit_stubs, bool require
 void Instrumentation::UpdateMethodsCode(mirror::AbstractMethod* method, const void* code) const {
   if (LIKELY(!instrumentation_stubs_installed_)) {
     method->SetEntryPointFromCompiledCode(code);
+  } else {
+    if (!interpreter_stubs_installed_ || method->IsNative()) {
+      method->SetEntryPointFromCompiledCode(GetInstrumentationEntryPoint());
+    } else {
+      method->SetEntryPointFromCompiledCode(GetInterpreterEntryPoint());
+    }
   }
 }
 
@@ -396,9 +408,14 @@ void Instrumentation::MethodEnterEventImpl(Thread* thread, mirror::Object* this_
                                            const mirror::AbstractMethod* method,
                                            uint32_t dex_pc) const {
   typedef std::list<InstrumentationListener*>::const_iterator It; // TODO: C++0x auto
-  for (It it = method_entry_listeners_.begin(), end = method_entry_listeners_.end(); it != end;
-      ++it) {
-    (*it)->MethodEntered(thread, this_object, method, dex_pc);
+  It it = method_entry_listeners_.begin();
+  bool is_end = (it == method_entry_listeners_.end());
+  // Implemented this way to prevent problems caused by modification of the list while iterating.
+  while (!is_end) {
+    InstrumentationListener* cur = *it;
+    ++it;
+    is_end = (it == method_entry_listeners_.end());
+    cur->MethodEntered(thread, this_object, method, dex_pc);
   }
 }
 
@@ -406,9 +423,14 @@ void Instrumentation::MethodExitEventImpl(Thread* thread, mirror::Object* this_o
                                           const mirror::AbstractMethod* method,
                                           uint32_t dex_pc, const JValue& return_value) const {
   typedef std::list<InstrumentationListener*>::const_iterator It; // TODO: C++0x auto
-  for (It it = method_exit_listeners_.begin(), end = method_exit_listeners_.end(); it != end;
-      ++it) {
-    (*it)->MethodExited(thread, this_object, method, dex_pc, return_value);
+  It it = method_exit_listeners_.begin();
+  bool is_end = (it == method_exit_listeners_.end());
+  // Implemented this way to prevent problems caused by modification of the list while iterating.
+  while (!is_end) {
+    InstrumentationListener* cur = *it;
+    ++it;
+    is_end = (it == method_exit_listeners_.end());
+    cur->MethodExited(thread, this_object, method, dex_pc, return_value);
   }
 }
 
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 5fea34f..384dfb2 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -284,7 +284,7 @@ struct InstrumentationStackFrame {
   mirror::AbstractMethod* method_;
   const uintptr_t return_pc_;
   const size_t frame_id_;
-  bool interpreter_entry_;
+  const bool interpreter_entry_;
 };
 
 }  // namespace instrumentation
diff --git a/runtime/mirror/abstract_method-inl.h b/runtime/mirror/abstract_method-inl.h
index a823886..39fc89e 100644
--- a/runtime/mirror/abstract_method-inl.h
+++ b/runtime/mirror/abstract_method-inl.h
@@ -114,17 +114,21 @@ inline void AbstractMethod::AssertPcIsWithinCode(uintptr_t pc) const {
   if (IsNative() || IsRuntimeMethod() || IsProxyMethod()) {
     return;
   }
-  if (GetEntryPointFromCompiledCode() == GetInterpreterEntryPoint()) {
+  if (pc == GetInstrumentationExitPc()) {
+    return;
+  }
+  const void* code = GetEntryPointFromCompiledCode();
+  if (code == GetInterpreterEntryPoint() || code == GetInstrumentationEntryPoint()) {
     return;
   }
   ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
-  if (GetEntryPointFromCompiledCode() == GetResolutionTrampoline(class_linker)) {
-      return;
+  if (code == GetResolutionTrampoline(class_linker)) {
+    return;
   }
   DCHECK(IsWithinCode(pc))
       << PrettyMethod(this)
       << " pc=" << std::hex << pc
-      << " code=" << GetEntryPointFromCompiledCode()
+      << " code=" << code
       << " size=" << GetCodeSize();
 }
 
diff --git a/runtime/stack.cc b/runtime/stack.cc
index fcd0f2d..f4ae81d 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -289,7 +289,6 @@ void StackVisitor::WalkStack(bool include_transitions) {
       DCHECK(current_fragment->GetTopShadowFrame() == NULL);
       mirror::AbstractMethod* method = *cur_quick_frame_;
       while (method != NULL) {
-        DCHECK(cur_quick_frame_pc_ != GetInstrumentationExitPc());
         SanityCheckFrame();
         bool should_continue = VisitFrame();
         if (UNLIKELY(!should_continue)) {
@@ -312,9 +311,7 @@ void StackVisitor::WalkStack(bool include_transitions) {
             instrumentation_stack_depth++;
             if (instrumentation_frame.interpreter_entry_) {
               mirror::AbstractMethod* callee = Runtime::Current()->GetCalleeSaveMethod(Runtime::kRefsAndArgs);
-              if (GetMethod() != callee) {
-                LOG(FATAL) << "Expected: " << callee << " Found: " << PrettyMethod(GetMethod());
-              }
+              CHECK_EQ(GetMethod(), callee);
             } else if (instrumentation_frame.method_ != GetMethod()) {
               LOG(FATAL)  << "Expected: " << PrettyMethod(instrumentation_frame.method_)
                 << " Found: " << PrettyMethod(GetMethod());