Merge cherrypicks of [2467803, 2467786, 2467717, 2467598, 2467823, 2467734, 2467671, 2467718, 2467599, 2467753, 2467672, 2467600, 2467735, 2467841, 2467638, 2467824, 2467736, 2467754, 2467737, 2467674] into oc-release

Change-Id: If8a17206ca029906dd71a1412cb92726a0d4e301

12 files changed, 249 insertions, 27 deletions

diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 7de8916ad5..67038b4c4b 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -403,15 +403,19 @@ bool ArtMethod::IsOverridableByDefaultMethod() {
 
 bool ArtMethod::IsAnnotatedWithFastNative() {
   return IsAnnotatedWith(WellKnownClasses::dalvik_annotation_optimization_FastNative,
-                         DexFile::kDexVisibilityBuild);
+                         DexFile::kDexVisibilityBuild,
+                         /* lookup_in_resolved_boot_classes */ true);
 }
 
 bool ArtMethod::IsAnnotatedWithCriticalNative() {
   return IsAnnotatedWith(WellKnownClasses::dalvik_annotation_optimization_CriticalNative,
-                         DexFile::kDexVisibilityBuild);
+                         DexFile::kDexVisibilityBuild,
+                         /* lookup_in_resolved_boot_classes */ true);
 }
 
-bool ArtMethod::IsAnnotatedWith(jclass klass, uint32_t visibility) {
+bool ArtMethod::IsAnnotatedWith(jclass klass,
+                                uint32_t visibility,
+                                bool lookup_in_resolved_boot_classes) {
   Thread* self = Thread::Current();
   ScopedObjectAccess soa(self);
   StackHandleScope<1> shs(self);
@@ -420,10 +424,8 @@ bool ArtMethod::IsAnnotatedWith(jclass klass, uint32_t visibility) {
   DCHECK(annotation->IsAnnotation());
   Handle<mirror::Class> annotation_handle(shs.NewHandle(annotation));
 
-  // Note: Resolves any method annotations' classes as a side-effect.
-  // -- This seems allowed by the spec since it says we can preload any classes
-  //    referenced by another classes's constant pool table.
-  return annotations::IsMethodAnnotationPresent(this, annotation_handle, visibility);
+  return annotations::IsMethodAnnotationPresent(
+      this, annotation_handle, visibility, lookup_in_resolved_boot_classes);
 }
 
 static uint32_t GetOatMethodIndexFromMethodIndex(const DexFile& dex_file,
diff --git a/runtime/art_method.h b/runtime/art_method.h
index 856bfd23e5..9ed056a16b 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -716,7 +716,10 @@ class ArtMethod FINAL {
  private:
   uint16_t FindObsoleteDexClassDefIndex() REQUIRES_SHARED(Locks::mutator_lock_);
 
-  bool IsAnnotatedWith(jclass klass, uint32_t visibility);
+  // If `lookup_in_resolved_boot_classes` is true, look up any of the
+  // method's annotations' classes in the bootstrap class loader's
+  // resolved types; otherwise, resolve them as a side effect.
+  bool IsAnnotatedWith(jclass klass, uint32_t visibility, bool lookup_in_resolved_boot_classes);
 
   static constexpr size_t PtrSizedFieldsOffset(PointerSize pointer_size) {
     // Round up to pointer size for padding field. Tested in art_method.cc.
diff --git a/runtime/dex_file_annotations.cc b/runtime/dex_file_annotations.cc
index 005bb4a160..ebacb27f52 100644
--- a/runtime/dex_file_annotations.cc
+++ b/runtime/dex_file_annotations.cc
@@ -751,7 +751,8 @@ const DexFile::AnnotationItem* GetAnnotationItemFromAnnotationSet(
     const ClassData& klass,
     const DexFile::AnnotationSetItem* annotation_set,
     uint32_t visibility,
-    Handle<mirror::Class> annotation_class)
+    Handle<mirror::Class> annotation_class,
+    bool lookup_in_resolved_boot_classes = false)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   const DexFile& dex_file = klass.GetDexFile();
   for (uint32_t i = 0; i < annotation_set->size_; ++i) {
@@ -761,19 +762,37 @@ const DexFile::AnnotationItem* GetAnnotationItemFromAnnotationSet(
     }
     const uint8_t* annotation = annotation_item->annotation_;
     uint32_t type_index = DecodeUnsignedLeb128(&annotation);
-    StackHandleScope<2> hs(Thread::Current());
-    mirror::Class* resolved_class = Runtime::Current()->GetClassLinker()->ResolveType(
-        klass.GetDexFile(),
-        dex::TypeIndex(type_index),
-        hs.NewHandle(klass.GetDexCache()),
-        hs.NewHandle(klass.GetClassLoader()));
-    if (resolved_class == nullptr) {
-      std::string temp;
-      LOG(WARNING) << StringPrintf("Unable to resolve %s annotation class %d",
-                                   klass.GetRealClass()->GetDescriptor(&temp), type_index);
-      CHECK(Thread::Current()->IsExceptionPending());
-      Thread::Current()->ClearException();
-      continue;
+    mirror::Class* resolved_class;
+    if (lookup_in_resolved_boot_classes) {
+      ObjPtr<mirror::Class> looked_up_class =
+          Runtime::Current()->GetClassLinker()->LookupResolvedType(
+              klass.GetDexFile(),
+              dex::TypeIndex(type_index),
+              klass.GetDexCache(),
+              // Force the use of the bootstrap class loader.
+              static_cast<mirror::ClassLoader*>(nullptr));
+      resolved_class = looked_up_class.Ptr();
+      if (resolved_class == nullptr) {
+        // If `resolved_class` is null, this is fine: just ignore that
+        // annotation item. We expect this to happen, as we do not
+        // attempt to resolve the annotation's class in this code path.
+        continue;
+      }
+    } else {
+      StackHandleScope<2> hs(Thread::Current());
+      resolved_class = Runtime::Current()->GetClassLinker()->ResolveType(
+          klass.GetDexFile(),
+          dex::TypeIndex(type_index),
+          hs.NewHandle(klass.GetDexCache()),
+          hs.NewHandle(klass.GetClassLoader()));
+      if (resolved_class == nullptr) {
+        std::string temp;
+        LOG(WARNING) << StringPrintf("Unable to resolve %s annotation class %d",
+                                     klass.GetRealClass()->GetDescriptor(&temp), type_index);
+        CHECK(Thread::Current()->IsExceptionPending());
+        Thread::Current()->ClearException();
+        continue;
+      }
     }
     if (resolved_class == annotation_class.Get()) {
       return annotation_item;
@@ -1200,15 +1219,20 @@ mirror::ObjectArray<mirror::String>* GetSignatureAnnotationForMethod(ArtMethod*
   return GetSignatureValue(ClassData(method), annotation_set);
 }
 
-bool IsMethodAnnotationPresent(ArtMethod* method, Handle<mirror::Class> annotation_class,
-                               uint32_t visibility /* = DexFile::kDexVisibilityRuntime */) {
+bool IsMethodAnnotationPresent(ArtMethod* method,
+                               Handle<mirror::Class> annotation_class,
+                               uint32_t visibility /* = DexFile::kDexVisibilityRuntime */,
+                               bool lookup_in_resolved_boot_classes /* = false */) {
   const DexFile::AnnotationSetItem* annotation_set = FindAnnotationSetForMethod(method);
   if (annotation_set == nullptr) {
     return false;
   }
   const DexFile::AnnotationItem* annotation_item =
       GetAnnotationItemFromAnnotationSet(ClassData(method),
-                                         annotation_set, visibility, annotation_class);
+                                         annotation_set,
+                                         visibility,
+                                         annotation_class,
+                                         lookup_in_resolved_boot_classes);
   return annotation_item != nullptr;
 }
 
diff --git a/runtime/dex_file_annotations.h b/runtime/dex_file_annotations.h
index 651c9844eb..e1088823c3 100644
--- a/runtime/dex_file_annotations.h
+++ b/runtime/dex_file_annotations.h
@@ -65,8 +65,15 @@ bool GetParametersMetadataForMethod(ArtMethod* method,
     REQUIRES_SHARED(Locks::mutator_lock_);
 mirror::ObjectArray<mirror::String>* GetSignatureAnnotationForMethod(ArtMethod* method)
     REQUIRES_SHARED(Locks::mutator_lock_);
-bool IsMethodAnnotationPresent(ArtMethod* method, Handle<mirror::Class> annotation_class,
-                               uint32_t visibility = DexFile::kDexVisibilityRuntime)
+// Check whether `method` is annotated with `annotation_class`.
+// If `lookup_in_resolved_boot_classes` is true, look up any of the
+// method's annotations' classes in the bootstrap class loader's
+// resolved types; if it is false (default value), resolve them as a
+// side effect.
+bool IsMethodAnnotationPresent(ArtMethod* method,
+                               Handle<mirror::Class> annotation_class,
+                               uint32_t visibility = DexFile::kDexVisibilityRuntime,
+                               bool lookup_in_resolved_boot_classes = false)
     REQUIRES_SHARED(Locks::mutator_lock_);
 
 // Class annotations.
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 2b349e39a0..629148e75b 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -2075,11 +2075,39 @@ extern "C" TwoWordReturn artQuickGenericJniTrampoline(Thread* self, ArtMethod**
     REQUIRES_SHARED(Locks::mutator_lock_) {
   ArtMethod* called = *sp;
   DCHECK(called->IsNative()) << called->PrettyMethod(true);
+  // Fix up a callee-save frame at the bottom of the stack (at `*sp`,
+  // above the alloca region) while we check for optimization
+  // annotations, thus allowing stack walking until the completion of
+  // the JNI frame creation.
+  //
+  // Note however that the Generic JNI trampoline does not expect
+  // exception being thrown at that stage.
+  *sp = Runtime::Current()->GetCalleeSaveMethod(Runtime::CalleeSaveType::kSaveRefsAndArgs);
+  self->SetTopOfStack(sp);
   uint32_t shorty_len = 0;
   const char* shorty = called->GetShorty(&shorty_len);
+  // Optimization annotations lookup does not try to resolve classes,
+  // as this may throw an exception, which is not supported by the
+  // Generic JNI trampoline at this stage; instead, method's
+  // annotations' classes are looked up in the bootstrap class
+  // loader's resolved types (which won't trigger an exception).
   bool critical_native = called->IsAnnotatedWithCriticalNative();
+  // ArtMethod::IsAnnotatedWithCriticalNative should not throw
+  // an exception; clear it if it happened anyway.
+  // TODO: Revisit this code path and turn this into a CHECK(!self->IsExceptionPending()).
+  if (self->IsExceptionPending()) {
+    self->ClearException();
+  }
   bool fast_native = called->IsAnnotatedWithFastNative();
+  // ArtMethod::IsAnnotatedWithFastNative should not throw
+  // an exception; clear it if it happened anyway.
+  // TODO: Revisit this code path and turn this into a CHECK(!self->IsExceptionPending()).
+  if (self->IsExceptionPending()) {
+    self->ClearException();
+  }
   bool normal_native = !critical_native && !fast_native;
+  // Restore the initial ArtMethod pointer at `*sp`.
+  *sp = called;
 
   // Run the visitor and update sp.
   BuildGenericJniFrameVisitor visitor(self,
diff --git a/test/656-annotation-lookup-generic-jni/expected.txt b/test/656-annotation-lookup-generic-jni/expected.txt
new file mode 100644
index 0000000000..4519c7e442
--- /dev/null
+++ b/test/656-annotation-lookup-generic-jni/expected.txt
@@ -0,0 +1,3 @@
+JNI_OnLoad called
+Java_Test_nativeMethodWithAnnotation
+passed
diff --git a/test/656-annotation-lookup-generic-jni/info.txt b/test/656-annotation-lookup-generic-jni/info.txt
new file mode 100644
index 0000000000..ddc19300ce
--- /dev/null
+++ b/test/656-annotation-lookup-generic-jni/info.txt
@@ -0,0 +1,5 @@
+Non-regression test for b/38454151, where the invocation of a native
+method with an annotatation through Generic JNI would crash the
+Generic JNI trampoline because it would throw and exception when
+trying to resolve the annotation (during the CriticalNative/FastNative
+optimization annotation lookup).
diff --git a/test/656-annotation-lookup-generic-jni/src-ex/DummyAnnotation.java b/test/656-annotation-lookup-generic-jni/src-ex/DummyAnnotation.java
new file mode 100644
index 0000000000..6caac6685e
--- /dev/null
+++ b/test/656-annotation-lookup-generic-jni/src-ex/DummyAnnotation.java
@@ -0,0 +1,17 @@
+/*
+ * 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.
+ */
+
+public @interface DummyAnnotation {}
diff --git a/test/656-annotation-lookup-generic-jni/src-ex/Test.java b/test/656-annotation-lookup-generic-jni/src-ex/Test.java
new file mode 100644
index 0000000000..838b4fe0d6
--- /dev/null
+++ b/test/656-annotation-lookup-generic-jni/src-ex/Test.java
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+public class Test {
+
+  public static void initialize(String libname) {
+    // Load test native library to get access to the implementation of
+    // Test.nativeMethodWithAnnotation.
+    System.loadLibrary(libname);
+  }
+
+  @DummyAnnotation
+  public static native void nativeMethodWithAnnotation();
+
+}
diff --git a/test/656-annotation-lookup-generic-jni/src/Main.java b/test/656-annotation-lookup-generic-jni/src/Main.java
new file mode 100644
index 0000000000..01b288a900
--- /dev/null
+++ b/test/656-annotation-lookup-generic-jni/src/Main.java
@@ -0,0 +1,76 @@
+/*
+ * 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.
+ */
+
+import dalvik.system.InMemoryDexClassLoader;
+
+import java.io.InputStream;
+import java.lang.reflect.Method;
+import java.nio.ByteBuffer;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
+public class Main {
+
+  public static void main(String[] args) throws Exception {
+    // Extract Dex file contents from the secondary Jar file.
+    String jarFilename =
+        System.getenv("DEX_LOCATION") + "/656-annotation-lookup-generic-jni-ex.jar";
+    ZipFile zipFile = new ZipFile(jarFilename);
+    ZipEntry zipEntry = zipFile.getEntry("classes.dex");
+    InputStream inputStream = zipFile.getInputStream(zipEntry);
+    int dexFileSize = (int) zipEntry.getSize();
+    byte[] dexFileContents = new byte[dexFileSize];
+    inputStream.read(dexFileContents, 0, dexFileSize);
+
+    // Create class loader from secondary Dex file.
+    ByteBuffer dexBuffer = ByteBuffer.wrap(dexFileContents);
+    ClassLoader classLoader = createUnquickenedDexClassLoader(dexBuffer);
+
+    // Load and initialize the Test class.
+    Class<?> testClass = classLoader.loadClass("Test");
+    Method initialize = testClass.getMethod("initialize", String.class);
+    initialize.invoke(null, args[0]);
+
+    // Invoke Test.nativeMethodWithAnnotation().
+    Method nativeMethodWithAnnotation = testClass.getMethod("nativeMethodWithAnnotation");
+    // Invoking the native method Test.nativeMethodWithAnnotation used
+    // to crash the Generic JNI trampoline during the resolution of
+    // the method's annotations (DummyAnnotation) (see b/38454151).
+    nativeMethodWithAnnotation.invoke(null);
+
+    zipFile.close();
+    System.out.println("passed");
+  }
+
+  // Create a class loader loading a Dex file in memory
+  // *without creating an Oat file*. This way, the Dex file won't be
+  // quickened and JNI stubs won't be compiled, thus forcing the use
+  // of Generic JNI when invoking the native method
+  // Test.nativeMethodWithAnnotation.
+  static ClassLoader createUnquickenedDexClassLoader(ByteBuffer dexBuffer) {
+    InMemoryDexClassLoader cl = new InMemoryDexClassLoader(dexBuffer, getBootClassLoader());
+    return cl;
+  }
+
+  static ClassLoader getBootClassLoader() {
+    ClassLoader cl = Main.class.getClassLoader();
+    while (cl.getParent() != null) {
+      cl = cl.getParent();
+    }
+    return cl;
+  }
+
+}
diff --git a/test/656-annotation-lookup-generic-jni/test.cc b/test/656-annotation-lookup-generic-jni/test.cc
new file mode 100644
index 0000000000..c8aa2af921
--- /dev/null
+++ b/test/656-annotation-lookup-generic-jni/test.cc
@@ -0,0 +1,28 @@
+/*
+ * 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 "jni.h"
+
+#include <iostream>
+
+namespace art {
+
+// Native method annotated with `DummyAnnotation` in Java source.
+extern "C" JNIEXPORT void JNICALL Java_Test_nativeMethodWithAnnotation(JNIEnv*, jclass) {
+  std::cout << "Java_Test_nativeMethodWithAnnotation" << std::endl;
+}
+
+}  // namespace art
diff --git a/test/Android.bp b/test/Android.bp
index 1679669056..2d682ed0e0 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -393,6 +393,7 @@ cc_defaults {
         "626-const-class-linking/clear_dex_cache_types.cc",
         "642-fp-callees/fp_callees.cc",
         "647-jni-get-field-id/get_field_id.cc",
+        "656-annotation-lookup-generic-jni/test.cc"
     ],
     shared_libs: [
         "libbacktrace",