Fix bugs in the x86 and arm versions of generic JNI.

Also fix the run script of 116-nodex2oat to use the non-prebuild
script for target.

Bug: 17000769

(cherry-picked from commit 54accbca0b549b1b1ad3ef09655dad438bc1e104)

Change-Id: I439fcd710fb8bb408f3288ff8fb34fef23890adb

diff --git a/compiler/jni/jni_compiler_test.cc b/compiler/jni/jni_compiler_test.cc
index 995ea46..4a8a8f8 100644 (file)
--- a/compiler/jni/jni_compiler_test.cc
+++ b/compiler/jni/jni_compiler_test.cc
@@ -16,6 +16,8 @@
 
 #include <memory>
 
+#include <math.h>
+
 #include "class_linker.h"
 #include "common_compiler_test.h"
 #include "dex_file.h"
@@ -47,7 +49,7 @@ namespace art {
 class JniCompilerTest : public CommonCompilerTest {
  protected:
   void CompileForTest(jobject class_loader, bool direct,
-                      const char* method_name, const char* method_sig) {
+                      const char* method_name, const char* method_sig, bool generic = false) {
     ScopedObjectAccess soa(Thread::Current());
     StackHandleScope<1> hs(soa.Self());
     Handle<mirror::ClassLoader> loader(
@@ -61,25 +63,29 @@ class JniCompilerTest : public CommonCompilerTest {
       method = c->FindVirtualMethod(method_name, method_sig);
     }
     ASSERT_TRUE(method != nullptr) << method_name << " " << method_sig;
-    if (method->GetEntryPointFromQuickCompiledCode() == nullptr ||
-        method->GetEntryPointFromQuickCompiledCode() == class_linker_->GetQuickGenericJniTrampoline()) {
-      CompileMethod(method);
-      ASSERT_TRUE(method->GetEntryPointFromQuickCompiledCode() != nullptr)
-          << method_name << " " << method_sig;
-      ASSERT_TRUE(method->GetEntryPointFromPortableCompiledCode() != nullptr)
-          << method_name << " " << method_sig;
+    if (generic) {
+      method->SetEntryPointFromQuickCompiledCode(class_linker_->GetQuickGenericJniTrampoline());
+    } else {
+      if (method->GetEntryPointFromQuickCompiledCode() == nullptr ||
+          method->GetEntryPointFromQuickCompiledCode() == class_linker_->GetQuickGenericJniTrampoline()) {
+        CompileMethod(method);
+        ASSERT_TRUE(method->GetEntryPointFromQuickCompiledCode() != nullptr)
+            << method_name << " " << method_sig;
+        ASSERT_TRUE(method->GetEntryPointFromPortableCompiledCode() != nullptr)
+            << method_name << " " << method_sig;
+      }
     }
   }
 
   void SetUpForTest(bool direct, const char* method_name, const char* method_sig,
-                    void* native_fnptr) {
+                    void* native_fnptr, bool generic = false) {
     // Initialize class loader and compile method when runtime not started.
     if (!runtime_->IsStarted()) {
       {
         ScopedObjectAccess soa(Thread::Current());
         class_loader_ = LoadDex("MyClassNatives");
       }
-      CompileForTest(class_loader_, direct, method_name, method_sig);
+      CompileForTest(class_loader_, direct, method_name, method_sig, generic);
       // Start runtime.
       Thread::Current()->TransitionFromSuspendedToRunnable();
       bool started = runtime_->Start();
@@ -432,6 +438,80 @@ TEST_F(JniCompilerTest, CompileAndRunStaticDoubleDoubleMethod) {
   EXPECT_EQ(2, gJava_MyClassNatives_fooSDD_calls);
 }
 
+// The x86 generic JNI code had a bug where it assumed a floating
+// point return value would be in xmm0. We use log, to somehow ensure
+// the compiler will use the floating point stack.
+
+jdouble Java_MyClassNatives_logD(JNIEnv* env, jclass klass, jdouble x) {
+  return log(x);
+}
+
+TEST_F(JniCompilerTest, RunGenericStaticLogDoubleethod) {
+  TEST_DISABLED_FOR_PORTABLE();
+  TEST_DISABLED_FOR_MIPS();
+  SetUpForTest(true, "logD", "(D)D",
+               reinterpret_cast<void*>(&Java_MyClassNatives_logD), true);
+
+  jdouble result = env_->CallStaticDoubleMethod(jklass_, jmethod_, 2.0);
+  EXPECT_EQ(log(2.0), result);
+}
+
+jfloat Java_MyClassNatives_logF(JNIEnv* env, jclass klass, jfloat x) {
+  return logf(x);
+}
+
+TEST_F(JniCompilerTest, RunGenericStaticLogFloatMethod) {
+  TEST_DISABLED_FOR_PORTABLE();
+  TEST_DISABLED_FOR_MIPS();
+  SetUpForTest(true, "logF", "(F)F",
+               reinterpret_cast<void*>(&Java_MyClassNatives_logF), true);
+
+  jfloat result = env_->CallStaticFloatMethod(jklass_, jmethod_, 2.0);
+  EXPECT_EQ(logf(2.0), result);
+}
+
+jboolean Java_MyClassNatives_returnTrue(JNIEnv* env, jclass klass) {
+  return JNI_TRUE;
+}
+
+jboolean Java_MyClassNatives_returnFalse(JNIEnv* env, jclass klass) {
+  return JNI_FALSE;
+}
+
+jint Java_MyClassNatives_returnInt(JNIEnv* env, jclass klass) {
+  return 42;
+}
+
+TEST_F(JniCompilerTest, RunGenericStaticReturnTrue) {
+  TEST_DISABLED_FOR_PORTABLE();
+  TEST_DISABLED_FOR_MIPS();
+  SetUpForTest(true, "returnTrue", "()Z",
+               reinterpret_cast<void*>(&Java_MyClassNatives_returnTrue), true);
+
+  jboolean result = env_->CallStaticBooleanMethod(jklass_, jmethod_);
+  EXPECT_TRUE(result);
+}
+
+TEST_F(JniCompilerTest, RunGenericStaticReturnFalse) {
+  TEST_DISABLED_FOR_PORTABLE();
+  TEST_DISABLED_FOR_MIPS();
+  SetUpForTest(true, "returnFalse", "()Z",
+               reinterpret_cast<void*>(&Java_MyClassNatives_returnFalse), true);
+
+  jboolean result = env_->CallStaticBooleanMethod(jklass_, jmethod_);
+  EXPECT_FALSE(result);
+}
+
+TEST_F(JniCompilerTest, RunGenericStaticReturnInt) {
+  TEST_DISABLED_FOR_PORTABLE();
+  TEST_DISABLED_FOR_MIPS();
+  SetUpForTest(true, "returnInt", "()I",
+               reinterpret_cast<void*>(&Java_MyClassNatives_returnInt), true);
+
+  jint result = env_->CallStaticIntMethod(jklass_, jmethod_);
+  EXPECT_EQ(42, result);
+}
+
 int gJava_MyClassNatives_fooSIOO_calls = 0;
 jobject Java_MyClassNatives_fooSIOO(JNIEnv* env, jclass klass, jint x, jobject y,
                              jobject z) {

diff --git a/runtime/arch/arm/quick_entrypoints_arm.S b/runtime/arch/arm/quick_entrypoints_arm.S
index dd1f04a..9f0db8c 100644 (file)
--- a/runtime/arch/arm/quick_entrypoints_arm.S
+++ b/runtime/arch/arm/quick_entrypoints_arm.S
@@ -1034,14 +1034,13 @@ ENTRY art_quick_generic_jni_trampoline
     // result sign extension is handled in C code
     // prepare for artQuickGenericJniEndTrampoline call
     // (Thread*, result, result_f)
-    //    r0      r1,r2    r3,stack       <= C calling convention
+    //    r0      r2,r3    stack       <= C calling convention
     //    r11     r0,r1    r0,r1          <= where they are
-    sub sp, sp, #12 // Stack alignment.
+    sub sp, sp, #8 // Stack alignment.
 
-    push {r1}
-    mov r3, r0
-    mov r2, r1
-    mov r1, r0
+    push {r0-r1}
+    mov r3, r1
+    mov r2, r0
     mov r0, r11
 
     blx artQuickGenericJniEndTrampoline
@@ -1058,7 +1057,18 @@ ENTRY art_quick_generic_jni_trampoline
     cbnz r2, .Lexception_in_native
 
     // Tear down the callee-save frame.
-    RESTORE_REF_AND_ARGS_CALLEE_SAVE_FRAME
+    add  sp, #12                      @ rewind sp
+    // Do not pop r0 and r1, they contain the return value.
+    pop {r2-r3, r5-r8, r10-r11, lr}  @ 9 words of callee saves
+    .cfi_restore r2
+    .cfi_restore r3
+    .cfi_restore r5
+    .cfi_restore r6
+    .cfi_restore r7
+    .cfi_restore r8
+    .cfi_restore r10
+    .cfi_restore r11
+    .cfi_adjust_cfa_offset -48
 
     bx lr      // ret
 

diff --git a/runtime/arch/x86/quick_entrypoints_x86.S b/runtime/arch/x86/quick_entrypoints_x86.S
index 75ec49d..084846a 100644 (file)
--- a/runtime/arch/x86/quick_entrypoints_x86.S
+++ b/runtime/arch/x86/quick_entrypoints_x86.S
@@ -1168,10 +1168,10 @@ DEFINE_FUNCTION art_quick_generic_jni_trampoline
     // prepare for artQuickGenericJniEndTrampoline call
     // (Thread*, result, result_f)
     //  (esp)    4(esp)  12(esp)    <= C calling convention
-    //  fs:...  eax:edx   xmm0      <= where they are
+    //  fs:...  eax:edx   fp0      <= where they are
 
     subl LITERAL(20), %esp         // Padding & pass float result.
-    movsd %xmm0, (%esp)
+    fstpl (%esp)
     pushl %edx                    // Pass int result.
     pushl %eax
     pushl %fs:THREAD_SELF_OFFSET  // Pass Thread::Current().
@@ -1196,7 +1196,7 @@ DEFINE_FUNCTION art_quick_generic_jni_trampoline
     POP ebp  // Restore callee saves
     POP esi
     POP edi
-    // store into fpr, for when it's a fpr return...
+    // Quick expects the return value to be in xmm0.
     movd %eax, %xmm0
     movd %edx, %xmm1
     punpckldq %xmm1, %xmm0

diff --git a/runtime/common_runtime_test.h b/runtime/common_runtime_test.h
index 86b2de8..5b014b3 100644 (file)
--- a/runtime/common_runtime_test.h
+++ b/runtime/common_runtime_test.h
@@ -155,6 +155,12 @@ class CheckJniAbortCatcher {
     return; \
   }
 
+#define TEST_DISABLED_FOR_MIPS() \
+  if (kRuntimeISA == kMips || kRuntimeISA == kMips64) { \
+    printf("WARNING: TEST DISABLED FOR MIPS\n"); \
+    return; \
+  }
+
 }  // namespace art
 
 namespace std {

diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 4730701..dfd2e11 100644 (file)
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1699,7 +1699,15 @@ extern "C" uint64_t artQuickGenericJniEndTrampoline(Thread* self, jvalue result,
     artQuickGenericJniEndJNINonRef(self, cookie, lock);
 
     switch (return_shorty_char) {
-      case 'F':  // Fall-through.
+      case 'F': {
+        if (kRuntimeISA == kX86) {
+          // Convert back the result to float.
+          double d = bit_cast<uint64_t, double>(result_f);
+          return bit_cast<float, uint32_t>(static_cast<float>(d));
+        } else {
+          return result_f;
+        }
+      }
       case 'D':
         return result_f;
       case 'Z':

diff --git a/test/116-nodex2oat/run b/test/116-nodex2oat/run
index 5ffeecd..2df6705 100755 (executable)
--- a/test/116-nodex2oat/run
+++ b/test/116-nodex2oat/run
@@ -17,6 +17,9 @@
 # Remove prebuild from the flags, this test is for testing not having oat files.
 flags="${@/--prebuild/}"
 
+# Use the non-prebuild script.
+RUN="${RUN/push-and-run-prebuilt-test-jar/push-and-run-test-jar}"
+
 # Make sure we can run without an oat file,
 echo "Run -Xnodex2oat"
 ${RUN} ${flags} --runtime-option -Xnodex2oat

diff --git a/test/MyClassNatives/MyClassNatives.java b/test/MyClassNatives/MyClassNatives.java
index b5e0204..fab153b 100644 (file)
--- a/test/MyClassNatives/MyClassNatives.java
+++ b/test/MyClassNatives/MyClassNatives.java
@@ -80,16 +80,22 @@ class MyClassNatives {
         Object o248, Object o249, Object o250, Object o251, Object o252, Object o253);
 
     native void withoutImplementation();
-    
+
     native static void stackArgsIntsFirst(int i1, int i2, int i3, int i4, int i5, int i6, int i7,
         int i8, int i9, int i10, float f1, float f2, float f3, float f4, float f5, float f6,
         float f7, float f8, float f9, float f10);
-    
+
     native static void stackArgsFloatsFirst(float f1, float f2, float f3, float f4, float f5,
         float f6, float f7, float f8, float f9, float f10, int i1, int i2, int i3, int i4, int i5,
         int i6, int i7, int i8, int i9, int i10);
-    
+
     native static void stackArgsMixed(int i1, float f1, int i2, float f2, int i3, float f3, int i4,
         float f4, int i5, float f5, int i6, float f6, int i7, float f7, int i8, float f8, int i9,
         float f9, int i10, float f10);
+
+    static native double logD(double d);
+    static native float logF(float f);
+    static native boolean returnTrue();
+    static native boolean returnFalse();
+    static native int returnInt();
 }