From: Andreas Gampe Date: Wed, 9 Dec 2015 23:14:04 +0000 (-0800) Subject: ART: Make trampoline compiler pointer-size-safe X-Git-Tag: android-x86-7.1-r1~944^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=4d98c84e6e2a47caf279909edae2b55f9f032288;p=android-x86%2Fart.git ART: Make trampoline compiler pointer-size-safe The trampoline compiler uses offsets of runtime structures which may change with the pointer size. Add offset tests to jni_internal_test. Bug: 26071368 (cherry picked from commit da9b763abc712fd6d1e24170a194abfbe795b8cd) Change-Id: I01d1a3727f46b3015ac677afb5427337c3093402 --- diff --git a/compiler/trampolines/trampoline_compiler.cc b/compiler/trampolines/trampoline_compiler.cc index 39e5259f0..48465e64a 100644 --- a/compiler/trampolines/trampoline_compiler.cc +++ b/compiler/trampolines/trampoline_compiler.cc @@ -57,7 +57,7 @@ static const std::vector* CreateTrampoline(EntryPointCallingConvention __ LoadFromOffset(kLoadWord, PC, R0, offset.Int32Value()); break; case kJniAbi: // Load via Thread* held in JNIEnv* in first argument (R0). - __ LoadFromOffset(kLoadWord, IP, R0, JNIEnvExt::SelfOffset().Int32Value()); + __ LoadFromOffset(kLoadWord, IP, R0, JNIEnvExt::SelfOffset(4).Int32Value()); __ LoadFromOffset(kLoadWord, PC, IP, offset.Int32Value()); break; case kQuickAbi: // R9 holds Thread*. @@ -91,7 +91,7 @@ static const std::vector* CreateTrampoline(EntryPointCallingConvention case kJniAbi: // Load via Thread* held in JNIEnv* in first argument (X0). __ LoadRawPtr(Arm64ManagedRegister::FromXRegister(IP1), Arm64ManagedRegister::FromXRegister(X0), - Offset(JNIEnvExt::SelfOffset().Int32Value())); + Offset(JNIEnvExt::SelfOffset(8).Int32Value())); __ JumpTo(Arm64ManagedRegister::FromXRegister(IP1), Offset(offset.Int32Value()), Arm64ManagedRegister::FromXRegister(IP0)); @@ -126,7 +126,7 @@ static const std::vector* CreateTrampoline(EntryPointCallingConvention __ LoadFromOffset(kLoadWord, T9, A0, offset.Int32Value()); break; case kJniAbi: // Load via Thread* held in JNIEnv* in first argument (A0). - __ LoadFromOffset(kLoadWord, T9, A0, JNIEnvExt::SelfOffset().Int32Value()); + __ LoadFromOffset(kLoadWord, T9, A0, JNIEnvExt::SelfOffset(4).Int32Value()); __ LoadFromOffset(kLoadWord, T9, T9, offset.Int32Value()); break; case kQuickAbi: // S1 holds Thread*. @@ -158,7 +158,7 @@ static const std::vector* CreateTrampoline(EntryPointCallingConvention __ LoadFromOffset(kLoadDoubleword, T9, A0, offset.Int32Value()); break; case kJniAbi: // Load via Thread* held in JNIEnv* in first argument (A0). - __ LoadFromOffset(kLoadDoubleword, T9, A0, JNIEnvExt::SelfOffset().Int32Value()); + __ LoadFromOffset(kLoadDoubleword, T9, A0, JNIEnvExt::SelfOffset(8).Int32Value()); __ LoadFromOffset(kLoadDoubleword, T9, T9, offset.Int32Value()); break; case kQuickAbi: // Fall-through. diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h index d13526b22..2d0ae63b2 100644 --- a/runtime/indirect_reference_table.h +++ b/runtime/indirect_reference_table.h @@ -344,8 +344,11 @@ class IndirectReferenceTable { segment_state_.all = new_state; } - static Offset SegmentStateOffset() { - return Offset(OFFSETOF_MEMBER(IndirectReferenceTable, segment_state_)); + static Offset SegmentStateOffset(size_t pointer_size ATTRIBUTE_UNUSED) { + // Note: Currently segment_state_ is at offset 0. We're testing the expected value in + // jni_internal_test to make sure it stays correct. It is not OFFSETOF_MEMBER, as that + // is not pointer-size-safe. + return Offset(0); } // Release pages past the end of the table that may have previously held references. diff --git a/runtime/jni_env_ext.cc b/runtime/jni_env_ext.cc index dab10403a..aa25f67ba 100644 --- a/runtime/jni_env_ext.cc +++ b/runtime/jni_env_ext.cc @@ -105,9 +105,32 @@ void JNIEnvExt::PopFrame() { stacked_local_ref_cookies.pop_back(); } -Offset JNIEnvExt::SegmentStateOffset() { - return Offset(OFFSETOF_MEMBER(JNIEnvExt, locals) + - IndirectReferenceTable::SegmentStateOffset().Int32Value()); +// Note: the offset code is brittle, as we can't use OFFSETOF_MEMBER or offsetof easily. Thus, there +// are tests in jni_internal_test to match the results against the actual values. + +// This is encoding the knowledge of the structure and layout of JNIEnv fields. +static size_t JNIEnvSize(size_t pointer_size) { + // A single pointer. + return pointer_size; +} + +Offset JNIEnvExt::SegmentStateOffset(size_t pointer_size) { + size_t locals_offset = JNIEnvSize(pointer_size) + + 2 * pointer_size + // Thread* self + JavaVMExt* vm. + 4 + // local_ref_cookie. + (pointer_size - 4); // Padding. + size_t irt_segment_state_offset = + IndirectReferenceTable::SegmentStateOffset(pointer_size).Int32Value(); + return Offset(locals_offset + irt_segment_state_offset); +} + +Offset JNIEnvExt::LocalRefCookieOffset(size_t pointer_size) { + return Offset(JNIEnvSize(pointer_size) + + 2 * pointer_size); // Thread* self + JavaVMExt* vm +} + +Offset JNIEnvExt::SelfOffset(size_t pointer_size) { + return Offset(JNIEnvSize(pointer_size)); } // Use some defining part of the caller's frame as the identifying mark for the JNI segment. diff --git a/runtime/jni_env_ext.h b/runtime/jni_env_ext.h index 3828ff045..2f8decf98 100644 --- a/runtime/jni_env_ext.h +++ b/runtime/jni_env_ext.h @@ -50,15 +50,9 @@ struct JNIEnvExt : public JNIEnv { T AddLocalReference(mirror::Object* obj) SHARED_REQUIRES(Locks::mutator_lock_); - static Offset SegmentStateOffset(); - - static Offset LocalRefCookieOffset() { - return Offset(OFFSETOF_MEMBER(JNIEnvExt, local_ref_cookie)); - } - - static Offset SelfOffset() { - return Offset(OFFSETOF_MEMBER(JNIEnvExt, self)); - } + static Offset SegmentStateOffset(size_t pointer_size); + static Offset LocalRefCookieOffset(size_t pointer_size); + static Offset SelfOffset(size_t pointer_size); jobject NewLocalRef(mirror::Object* obj) SHARED_REQUIRES(Locks::mutator_lock_); void DeleteLocalRef(jobject obj) SHARED_REQUIRES(Locks::mutator_lock_); diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc index d1687d787..b41d16b29 100644 --- a/runtime/jni_internal_test.cc +++ b/runtime/jni_internal_test.cc @@ -18,7 +18,9 @@ #include "art_method-inl.h" #include "common_compiler_test.h" +#include "indirect_reference_table.h" #include "java_vm_ext.h" +#include "jni_env_ext.h" #include "mirror/string-inl.h" #include "scoped_thread_state_change.h" #include "ScopedLocalRef.h" @@ -2261,4 +2263,41 @@ TEST_F(JniInternalTest, DetachThreadUnlockJNIMonitors) { env_->DeleteGlobalRef(global_ref); } +// Test the offset computation of IndirectReferenceTable offsets. b/26071368. +TEST_F(JniInternalTest, IndirectReferenceTableOffsets) { + // The segment_state_ field is private, and we want to avoid friend declaration. So we'll check + // by modifying memory. + // The parameters don't really matter here. + IndirectReferenceTable irt(5, 5, IndirectRefKind::kGlobal, true); + uint32_t old_state = irt.GetSegmentState(); + + // Write some new state directly. We invert parts of old_state to ensure a new value. + uint32_t new_state = old_state ^ 0x07705005; + ASSERT_NE(old_state, new_state); + + uint8_t* base = reinterpret_cast(&irt); + int32_t segment_state_offset = + IndirectReferenceTable::SegmentStateOffset(sizeof(void*)).Int32Value(); + *reinterpret_cast(base + segment_state_offset) = new_state; + + // Read and compare. + EXPECT_EQ(new_state, irt.GetSegmentState()); +} + +// Test the offset computation of JNIEnvExt offsets. b/26071368. +TEST_F(JniInternalTest, JNIEnvExtOffsets) { + EXPECT_EQ(OFFSETOF_MEMBER(JNIEnvExt, local_ref_cookie), + JNIEnvExt::LocalRefCookieOffset(sizeof(void*)).Int32Value()); + + EXPECT_EQ(OFFSETOF_MEMBER(JNIEnvExt, self), JNIEnvExt::SelfOffset(sizeof(void*)).Int32Value()); + + // segment_state_ is private in the IndirectReferenceTable. So this test isn't as good as we'd + // hope it to be. + int32_t segment_state_now = + OFFSETOF_MEMBER(JNIEnvExt, locals) + + IndirectReferenceTable::SegmentStateOffset(sizeof(void*)).Int32Value(); + int32_t segment_state_computed = JNIEnvExt::SegmentStateOffset(sizeof(void*)).Int32Value(); + EXPECT_EQ(segment_state_now, segment_state_computed); +} + } // namespace art