commit 434ce294296b41a962a4fd86c3deb1f0f462d3e6
author Ian Rogers <irogers@google.com> Sat Mar 03 15:48:38 2012 -0800
committer Android (Google) Code Review <android-gerrit@google.com> Sat Mar 03 15:48:38 2012 -0800
tree 00961755d4b53d178f8820febbf6f352778b77f3
parent 4c17dffabb0ad062574018ba1b2e760ec34a03ee
parent c77928475995d01e1d6d2b2d6672510d01a5fce4

Merge "Fix bug in JNI compiler AAPCS padding calculation" into dalvik-dev

src/calling_convention_arm.cc

diff --git a/src/calling_convention_arm.cc b/src/calling_convention_arm.cc
index 1a5b92f..0ada85a 100644
--- a/src/calling_convention_arm.cc
+++ b/src/calling_convention_arm.cc
@@ -115,15 +115,18 @@
 ArmJniCallingConvention::ArmJniCallingConvention(bool is_static, bool is_synchronized,
                                                  const char* shorty)
     : JniCallingConvention(is_static, is_synchronized, shorty) {
-  // Compute padding to ensure longs and doubles are not split in AAPCS
-  // TODO: in terms of outgoing argument size this may be overly generous
-  // due to padding appearing in the registers
+  // Compute padding to ensure longs and doubles are not split in AAPCS. Ignore the 'this' jobject
+  // or jclass for static methods and the JNIEnv. We start at the aligned register r2.
   size_t padding = 0;
-  size_t check = IsStatic() ? 1 : 0;
-  for (size_t i = 0; i < NumArgs(); i++) {
-    if (((i & 1) == check) && IsParamALongOrDouble(i)) {
-      padding += 4;
+  for (size_t cur_arg = IsStatic() ? 0 : 1, cur_reg = 2; cur_arg < NumArgs(); cur_arg++) {
+    if (IsParamALongOrDouble(cur_arg)) {
+      if ((cur_reg & 1) != 0) {
+        padding += 4;
+        cur_reg++;  // additional bump to ensure alignment
+      }
+      cur_reg++;  // additional bump to skip extra long word
     }
+    cur_reg++;  // bump the iterator for every argument
   }
   padding_ = padding;
 
@@ -205,8 +208,9 @@
 
 FrameOffset ArmJniCallingConvention::CurrentParamStackOffset() {
   CHECK_GE(itr_slots_, 4u);
-  return FrameOffset(displacement_.Int32Value() - OutArgSize()
-                     + ((itr_slots_ - 4) * kPointerSize));
+  size_t offset = displacement_.Int32Value() - OutArgSize() + ((itr_slots_ - 4) * kPointerSize);
+  CHECK_LT(offset, OutArgSize());
+  return FrameOffset(offset);
 }
 
 size_t ArmJniCallingConvention::NumberOfOutgoingStackArgs() {

src/jni_compiler_test.cc

diff --git a/src/jni_compiler_test.cc b/src/jni_compiler_test.cc
index 2166d37..fd7574d 100644
--- a/src/jni_compiler_test.cc
+++ b/src/jni_compiler_test.cc
@@ -622,4 +622,22 @@
   EXPECT_EQ(result, JNI_TRUE);
 }
 
+jint my_gettext(JNIEnv* env, jclass klass, jlong val1, jobject obj1, jlong val2, jobject obj2) {
+  EXPECT_TRUE(env->IsInstanceOf(JniCompilerTest::jobj_, klass));
+  EXPECT_TRUE(env->IsSameObject(JniCompilerTest::jobj_, obj1));
+  EXPECT_TRUE(env->IsSameObject(JniCompilerTest::jobj_, obj2));
+  EXPECT_EQ(0x12345678ABCDEF88ll, val1);
+  EXPECT_EQ(0x7FEDCBA987654321ll, val2);
+  return 42;
+}
+
+TEST_F(JniCompilerTest, GetText) {
+  SirtRef<ClassLoader> class_loader(LoadDex("MyClassNatives"));
+  SetupForTest(class_loader.get(), true, "getText", "(JLjava/lang/Object;JLjava/lang/Object;)I",
+               reinterpret_cast<void*>(&my_gettext));
+  jint result = env_->CallStaticIntMethod(jklass_, jmethod_, 0x12345678ABCDEF88ll, jobj_,
+                                          0x7FEDCBA987654321ll, jobj_);
+  EXPECT_EQ(result, 42);
+}
+
 }  // namespace art

test/MyClassNatives/MyClassNatives.java

diff --git a/test/MyClassNatives/MyClassNatives.java b/test/MyClassNatives/MyClassNatives.java
index 55ec0bd..4987373 100644
--- a/test/MyClassNatives/MyClassNatives.java
+++ b/test/MyClassNatives/MyClassNatives.java
@@ -31,4 +31,5 @@
     static synchronized native Object fooSSIOO(int x, Object y, Object z);
     static native void arraycopy(Object src, int src_pos, Object dst, int dst_pos, int length);
     native boolean compareAndSwapInt(Object obj, long offset, int expected, int newval);
+    static native int getText(long val1, Object obj1, long val2, Object obj2);
 }