Fix the stack at the beginning of the Generic JNI trampoline.

Fix up a callee-save frame at the bottom of the stack while we
check for optimization annotations, thus allowing stack walking
until the completion of the JNI frame creation.

Test: art/test/testrunner/testrunner.py -t 656-annotation-lookup-generic-jni
Bug: 38454151
Bug: 34659969
Change-Id: I70dc3d40139198f29457dba5282b45a9d0d09e49
Merged-In: I70dc3d40139198f29457dba5282b45a9d0d09e49
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 2b349e3..c1016aa 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -2075,11 +2075,34 @@
     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);
   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 0000000..4519c7e
--- /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 0000000..ddc1930
--- /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 0000000..6caac66
--- /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 0000000..838b4fe
--- /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 0000000..01b288a
--- /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 0000000..c8aa2af
--- /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 1679669..2d682ed 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -393,6 +393,7 @@
         "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",