Revert "Revert "Revert "Make class redefinition work with native methods on stack."""

This reverts commit 4493f92dfc70e01d95ad57bc4c0c38f59a616f70.

Reason for revert: Fails interpreter-ac and relocate-npatchoat

Change-Id: I15eb4964c9101fa04e34b52acf9ffe25e6d4e60d
Test: revert

12 files changed, 5 insertions, 250 deletions

diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 5a93e29663..6cb8544617 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -442,51 +442,12 @@ static uint32_t GetOatMethodIndexFromMethodIndex(const DexFile& dex_file,
   UNREACHABLE();
 }
 
-// We use the method's DexFile and declaring class name to find the OatMethod for an obsolete
-// method.  This is extremely slow but we need it if we want to be able to have obsolete native
-// methods since we need this to find the size of it's stack frames.
-static const OatFile::OatMethod FindOatMethodFromDexFileFor(ArtMethod* method, bool* found)
-    REQUIRES_SHARED(Locks::mutator_lock_) {
-  DCHECK(method->IsObsolete() && method->IsNative());
-  const DexFile* dex_file = method->GetDexFile();
-
-  // recreate the class_def_index from the descriptor.
-  std::string descriptor_storage;
-  const DexFile::TypeId* declaring_class_type_id =
-      dex_file->FindTypeId(method->GetDeclaringClass()->GetDescriptor(&descriptor_storage));
-  CHECK(declaring_class_type_id != nullptr);
-  dex::TypeIndex declaring_class_type_index = dex_file->GetIndexForTypeId(*declaring_class_type_id);
-  const DexFile::ClassDef* declaring_class_type_def =
-      dex_file->FindClassDef(declaring_class_type_index);
-  CHECK(declaring_class_type_def != nullptr);
-  uint16_t declaring_class_def_index = dex_file->GetIndexForClassDef(*declaring_class_type_def);
-
-  size_t oat_method_index = GetOatMethodIndexFromMethodIndex(*dex_file,
-                                                             declaring_class_def_index,
-                                                             method->GetDexMethodIndex());
-
-  OatFile::OatClass oat_class = OatFile::FindOatClass(*dex_file,
-                                                      declaring_class_def_index,
-                                                      found);
-  if (!(*found)) {
-    return OatFile::OatMethod::Invalid();
-  }
-  return oat_class.GetOatMethod(oat_method_index);
-}
-
 static const OatFile::OatMethod FindOatMethodFor(ArtMethod* method,
                                                  PointerSize pointer_size,
                                                  bool* found)
     REQUIRES_SHARED(Locks::mutator_lock_) {
-  if (UNLIKELY(method->IsObsolete())) {
-    // We shouldn't be calling this with obsolete methods except for native obsolete methods for
-    // which we need to use the oat method to figure out how large the quick frame is.
-    DCHECK(method->IsNative()) << "We should only be finding the OatMethod of obsolete methods in "
-                               << "order to allow stack walking. Other obsolete methods should "
-                               << "never need to access this information.";
-    DCHECK_EQ(pointer_size, kRuntimePointerSize) << "Obsolete method in compiler!";
-    return FindOatMethodFromDexFileFor(method, found);
-  }
+  // We shouldn't be calling this with obsolete methods.
+  DCHECK(!method->IsObsolete());
   // Although we overwrite the trampoline of non-static methods, we may get here via the resolution
   // method for direct methods (or virtual methods made direct).
   mirror::Class* declaring_class = method->GetDeclaringClass();
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index c35419cef5..f0c0dbcbfc 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -452,11 +452,6 @@ void Redefiner::ClassRedefinition::FindAndAllocateObsoleteMethods(art::mirror::C
   CallbackCtx ctx(linker->GetAllocatorForClassLoader(art_klass->GetClassLoader()));
   // Add all the declared methods to the map
   for (auto& m : art_klass->GetDeclaredMethods(art::kRuntimePointerSize)) {
-    // TODO It should be possible to simply filter out some methods where they cannot really become
-    // obsolete, such as native methods and keep their original (possibly optimized)
-    // implementations. We don't do this, however, since we would need to mark these functions
-    // (still in the classes declared_methods array) as obsolete so we will find the correct dex
-    // file to get meta-data from (for example about stack-frame size).
     ctx.obsolete_methods.insert(&m);
     // TODO Allow this or check in IsModifiableClass.
     DCHECK(!m.IsIntrinsic());
diff --git a/runtime/stack.cc b/runtime/stack.cc
index 96fc664584..d7ba1d75d8 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -874,11 +874,9 @@ void StackVisitor::WalkStack(bool include_transitions) {
               CHECK_EQ(GetMethod(), callee) << "Expected: " << ArtMethod::PrettyMethod(callee)
                                             << " Found: " << ArtMethod::PrettyMethod(GetMethod());
             } else {
-              // Instrumentation generally doesn't distinguish between a method's obsolete and
-              // non-obsolete version.
-              CHECK_EQ(instrumentation_frame.method_, GetMethod()->GetNonObsoleteMethod())
+              CHECK_EQ(instrumentation_frame.method_, GetMethod())
                   << "Expected: " << ArtMethod::PrettyMethod(instrumentation_frame.method_)
-                  << " Found: " << ArtMethod::PrettyMethod(GetMethod()->GetNonObsoleteMethod());
+                  << " Found: " << ArtMethod::PrettyMethod(GetMethod());
             }
             if (num_frames_ != 0) {
               // Check agreement of frame Ids only if num_frames_ is computed to avoid infinite
@@ -905,7 +903,7 @@ void StackVisitor::WalkStack(bool include_transitions) {
               << " native=" << method->IsNative()
               << std::noboolalpha
               << " entrypoints=" << method->GetEntryPointFromQuickCompiledCode()
-              << "," << (method->IsNative() ? method->GetEntryPointFromJni() : nullptr)
+              << "," << method->GetEntryPointFromJni()
               << " next=" << *cur_quick_frame_;
         }
 
diff --git a/test/945-obsolete-native/build b/test/945-obsolete-native/build
deleted file mode 100755
index 898e2e54a2..0000000000
--- a/test/945-obsolete-native/build
+++ /dev/null
@@ -1,17 +0,0 @@
-#!/bin/bash
-#
-# Copyright 2016 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-./default-build "$@" --experimental agents
diff --git a/test/945-obsolete-native/expected.txt b/test/945-obsolete-native/expected.txt
deleted file mode 100644
index 83efda144d..0000000000
--- a/test/945-obsolete-native/expected.txt
+++ /dev/null
@@ -1,9 +0,0 @@
-hello
-Not doing anything here
-goodbye
-hello
-transforming calling function
-goodbye
-Hello - Transformed
-Not doing anything here
-Goodbye - Transformed
diff --git a/test/945-obsolete-native/info.txt b/test/945-obsolete-native/info.txt
deleted file mode 100644
index c8b892cedd..0000000000
--- a/test/945-obsolete-native/info.txt
+++ /dev/null
@@ -1 +0,0 @@
-Tests basic obsolete method support
diff --git a/test/945-obsolete-native/obsolete_native.cc b/test/945-obsolete-native/obsolete_native.cc
deleted file mode 100644
index 061e7afbbc..0000000000
--- a/test/945-obsolete-native/obsolete_native.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <inttypes.h>
-#include <memory>
-#include <stdio.h>
-
-#include "android-base/stringprintf.h"
-
-#include "android-base/stringprintf.h"
-#include "base/logging.h"
-#include "base/macros.h"
-#include "jni.h"
-#include "openjdkjvmti/jvmti.h"
-#include "ScopedLocalRef.h"
-#include "ti-agent/common_helper.h"
-#include "ti-agent/common_load.h"
-
-namespace art {
-namespace Test945ObsoleteNative {
-
-extern "C" JNIEXPORT void JNICALL Java_Main_bindTest945ObsoleteNative(
-    JNIEnv* env, jclass klass ATTRIBUTE_UNUSED) {
-  BindFunctions(jvmti_env, env, "Transform");
-}
-
-extern "C" JNIEXPORT void JNICALL Java_Transform_doExecute(JNIEnv* env,
-                                                           jclass klass ATTRIBUTE_UNUSED,
-                                                           jobject runnable) {
-  jclass runnable_klass = env->FindClass("java/lang/Runnable");
-  DCHECK(runnable_klass != nullptr);
-  jmethodID run_method = env->GetMethodID(runnable_klass, "run", "()V");
-  env->CallVoidMethod(runnable, run_method);
-}
-
-
-}  // namespace Test945ObsoleteNative
-}  // namespace art
diff --git a/test/945-obsolete-native/run b/test/945-obsolete-native/run
deleted file mode 100755
index c6e62ae6cd..0000000000
--- a/test/945-obsolete-native/run
+++ /dev/null
@@ -1,17 +0,0 @@
-#!/bin/bash
-#
-# Copyright 2016 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-./default-run "$@" --jvmti
diff --git a/test/945-obsolete-native/src/Main.java b/test/945-obsolete-native/src/Main.java
deleted file mode 100644
index 5e2154e9a3..0000000000
--- a/test/945-obsolete-native/src/Main.java
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import java.util.Base64;
-
-public class Main {
-  // class Transform {
-  //   public void sayHi(Runnable r) {
-  //     System.out.println("Hello - Transformed");
-  //     doExecute(r);
-  //     System.out.println("Goodbye - Transformed");
-  //   }
-  //
-  //   private static native void doExecute(Runnable r);
-  // }
-  private static final byte[] CLASS_BYTES = Base64.getDecoder().decode(
-    "yv66vgAAADQAIgoACAASCQATABQIABUKABYAFwoABwAYCAAZBwAaBwAbAQAGPGluaXQ+AQADKClW" +
-    "AQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEABXNheUhpAQAXKExqYXZhL2xhbmcvUnVubmFibGU7" +
-    "KVYBAAlkb0V4ZWN1dGUBAApTb3VyY2VGaWxlAQAOVHJhbnNmb3JtLmphdmEMAAkACgcAHAwAHQAe" +
-    "AQATSGVsbG8gLSBUcmFuc2Zvcm1lZAcAHwwAIAAhDAAPAA4BABVHb29kYnllIC0gVHJhbnNmb3Jt" +
-    "ZWQBAAlUcmFuc2Zvcm0BABBqYXZhL2xhbmcvT2JqZWN0AQAQamF2YS9sYW5nL1N5c3RlbQEAA291" +
-    "dAEAFUxqYXZhL2lvL1ByaW50U3RyZWFtOwEAE2phdmEvaW8vUHJpbnRTdHJlYW0BAAdwcmludGxu" +
-    "AQAVKExqYXZhL2xhbmcvU3RyaW5nOylWACAABwAIAAAAAAADAAAACQAKAAEACwAAAB0AAQABAAAA" +
-    "BSq3AAGxAAAAAQAMAAAABgABAAAAEQABAA0ADgABAAsAAAA5AAIAAgAAABWyAAISA7YABCu4AAWy" +
-    "AAISBrYABLEAAAABAAwAAAASAAQAAAATAAgAFAAMABUAFAAWAQoADwAOAAAAAQAQAAAAAgAR");
-  private static final byte[] DEX_BYTES = Base64.getDecoder().decode(
-    "ZGV4CjAzNQB1fZcJR/opPuXacK8mIla5shH0LSg72qJYAwAAcAAAAHhWNBIAAAAAAAAAALgCAAAR" +
-    "AAAAcAAAAAcAAAC0AAAAAwAAANAAAAABAAAA9AAAAAUAAAD8AAAAAQAAACQBAAAUAgAARAEAAKIB" +
-    "AACqAQAAwQEAANYBAADjAQAA+gEAAA4CAAAkAgAAOAIAAEwCAABcAgAAXwIAAGMCAABuAgAAggIA" +
-    "AIcCAACQAgAAAwAAAAQAAAAFAAAABgAAAAcAAAAIAAAACgAAAAoAAAAGAAAAAAAAAAsAAAAGAAAA" +
-    "lAEAAAsAAAAGAAAAnAEAAAUAAQAOAAAAAAAAAAAAAAAAAAEADAAAAAAAAQAQAAAAAQACAA8AAAAC" +
-    "AAAAAAAAAAAAAAAAAAAAAgAAAAAAAAAJAAAAAAAAAKUCAAAAAAAAAQABAAEAAACXAgAABAAAAHAQ" +
-    "BAAAAA4ABAACAAIAAACcAgAAFAAAAGIAAAAbAQIAAABuIAMAEABxEAEAAwBiAAAAGwEBAAAAbiAD" +
-    "ABAADgABAAAAAwAAAAEAAAAEAAY8aW5pdD4AFUdvb2RieWUgLSBUcmFuc2Zvcm1lZAATSGVsbG8g" +
-    "LSBUcmFuc2Zvcm1lZAALTFRyYW5zZm9ybTsAFUxqYXZhL2lvL1ByaW50U3RyZWFtOwASTGphdmEv" +
-    "bGFuZy9PYmplY3Q7ABRMamF2YS9sYW5nL1J1bm5hYmxlOwASTGphdmEvbGFuZy9TdHJpbmc7ABJM" +
-    "amF2YS9sYW5nL1N5c3RlbTsADlRyYW5zZm9ybS5qYXZhAAFWAAJWTAAJZG9FeGVjdXRlABJlbWl0" +
-    "dGVyOiBqYWNrLTQuMjUAA291dAAHcHJpbnRsbgAFc2F5SGkAEQAHDgATAQAHDoc8hwAAAAIBAICA" +
-    "BMQCAYoCAAIB3AIADQAAAAAAAAABAAAAAAAAAAEAAAARAAAAcAAAAAIAAAAHAAAAtAAAAAMAAAAD" +
-    "AAAA0AAAAAQAAAABAAAA9AAAAAUAAAAFAAAA/AAAAAYAAAABAAAAJAEAAAEgAAACAAAARAEAAAEQ" +
-    "AAACAAAAlAEAAAIgAAARAAAAogEAAAMgAAACAAAAlwIAAAAgAAABAAAApQIAAAAQAAABAAAAuAIA" +
-    "AA==");
-
-  public static void main(String[] args) {
-    bindTest945ObsoleteNative();
-    doTest(new Transform());
-  }
-
-  public static void doTest(Transform t) {
-    t.sayHi(() -> { System.out.println("Not doing anything here"); });
-    t.sayHi(() -> {
-      System.out.println("transforming calling function");
-      doCommonClassRedefinition(Transform.class, CLASS_BYTES, DEX_BYTES);
-    });
-    t.sayHi(() -> { System.out.println("Not doing anything here"); });
-  }
-
-  // Transforms the class
-  private static native void doCommonClassRedefinition(Class<?> target,
-                                                       byte[] classfile,
-                                                       byte[] dexfile);
-
-  private static native void bindTest945ObsoleteNative();
-}
diff --git a/test/945-obsolete-native/src/Transform.java b/test/945-obsolete-native/src/Transform.java
deleted file mode 100644
index 2b7cc1b3a1..0000000000
--- a/test/945-obsolete-native/src/Transform.java
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-class Transform {
-  public void sayHi(Runnable r) {
-    System.out.println("hello");
-    doExecute(r);
-    System.out.println("goodbye");
-  }
-
-  private static native void doExecute(Runnable r);
-}
diff --git a/test/Android.bp b/test/Android.bp
index 00c890a834..d3244a683a 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -274,7 +274,6 @@ art_cc_defaults {
         "933-misc-events/misc_events.cc",
         "936-search-onload/search_onload.cc",
         "944-transform-classloaders/classloader.cc",
-        "945-obsolete-native/obsolete_native.cc",
     ],
     shared_libs: [
         "libbase",
diff --git a/test/ti-agent/common_load.cc b/test/ti-agent/common_load.cc
index 351857d1d9..c5a93568c6 100644
--- a/test/ti-agent/common_load.cc
+++ b/test/ti-agent/common_load.cc
@@ -122,7 +122,6 @@ static AgentLib agents[] = {
   { "942-private-recursive", common_redefine::OnLoad, nullptr },
   { "943-private-recursive-jit", common_redefine::OnLoad, nullptr },
   { "944-transform-classloaders", common_redefine::OnLoad, nullptr },
-  { "945-obsolete-native", common_redefine::OnLoad, nullptr },
 };
 
 static AgentLib* FindAgent(char* name) {