Fix unintended sign-extension. am: 4d0f795aaa
Change-Id: Ibb356db9a16af923c6b1a57e638998e98dd67367
diff --git a/runtime/mirror/array-inl.h b/runtime/mirror/array-inl.h
index b75aa5d..6d52d44 100644
--- a/runtime/mirror/array-inl.h
+++ b/runtime/mirror/array-inl.h
@@ -228,22 +228,30 @@
template<typename T, PointerSize kPointerSize, VerifyObjectFlags kVerifyFlags>
inline T PointerArray::GetElementPtrSize(uint32_t idx) {
- // C style casts here since we sometimes have T be a pointer, or sometimes an integer
- // (for stack traces).
if (kPointerSize == PointerSize::k64) {
- return (T)static_cast<uintptr_t>(AsLongArray<kVerifyFlags>()->GetWithoutChecks(idx));
+ DCHECK(IsLongArray<kVerifyFlags>());
+ } else {
+ DCHECK(IsIntArray<kVerifyFlags>());
}
- return (T)static_cast<uintptr_t>(AsIntArray<kVerifyFlags>()->GetWithoutChecks(idx));
+ return GetElementPtrSizeUnchecked<T, kPointerSize, kVerifyFlags>(idx);
}
+
template<typename T, PointerSize kPointerSize, VerifyObjectFlags kVerifyFlags>
inline T PointerArray::GetElementPtrSizeUnchecked(uint32_t idx) {
// C style casts here since we sometimes have T be a pointer, or sometimes an integer
// (for stack traces).
+ using ConversionType = typename std::conditional_t<std::is_pointer_v<T>, uintptr_t, T>;
if (kPointerSize == PointerSize::k64) {
- return (T)static_cast<uintptr_t>(AsLongArrayUnchecked<kVerifyFlags>()->GetWithoutChecks(idx));
+ uint64_t value =
+ static_cast<uint64_t>(AsLongArrayUnchecked<kVerifyFlags>()->GetWithoutChecks(idx));
+ return (T) dchecked_integral_cast<ConversionType>(value);
+ } else {
+ uint32_t value =
+ static_cast<uint32_t>(AsIntArrayUnchecked<kVerifyFlags>()->GetWithoutChecks(idx));
+ return (T) dchecked_integral_cast<ConversionType>(value);
}
- return (T)static_cast<uintptr_t>(AsIntArrayUnchecked<kVerifyFlags>()->GetWithoutChecks(idx));
}
+
template<typename T, VerifyObjectFlags kVerifyFlags>
inline T PointerArray::GetElementPtrSize(uint32_t idx, PointerSize ptr_size) {
if (ptr_size == PointerSize::k64) {
diff --git a/runtime/mirror/object_test.cc b/runtime/mirror/object_test.cc
index ee137f0..8ef7025 100644
--- a/runtime/mirror/object_test.cc
+++ b/runtime/mirror/object_test.cc
@@ -253,6 +253,48 @@
TestPrimitiveArray<ShortArray>(class_linker_);
}
+TEST_F(ObjectTest, PointerArrayWriteRead) {
+ ScopedObjectAccess soa(Thread::Current());
+ StackHandleScope<2> hs(soa.Self());
+
+ Handle<PointerArray> a32 =
+ hs.NewHandle(ObjPtr<PointerArray>::DownCast<Array>(IntArray::Alloc(soa.Self(), 1)));
+ ASSERT_TRUE(a32 != nullptr);
+ ASSERT_EQ(1, a32->GetLength());
+ EXPECT_EQ(0u, (a32->GetElementPtrSize<uint32_t, PointerSize::k32>(0u)));
+ EXPECT_EQ(0u, (a32->GetElementPtrSizeUnchecked<uint32_t, PointerSize::k32>(0u)));
+ for (uint32_t value : { 0u, 1u, 0x7fffffffu, 0x80000000u, 0xffffffffu }) {
+ a32->SetElementPtrSize(0u, value, PointerSize::k32);
+ EXPECT_EQ(value, (a32->GetElementPtrSize<uint32_t, PointerSize::k32>(0u)));
+ EXPECT_EQ(value, (a32->GetElementPtrSizeUnchecked<uint32_t, PointerSize::k32>(0u)));
+ // Check that the value matches also when retrieved as `uint64_t`.
+ // This is a regression test for unintended sign-extension. b/155780442
+ // (Using `uint64_t` rather than `uintptr_t`, so that the 32-bit test checks this too.)
+ EXPECT_EQ(value, (a32->GetElementPtrSize<uint64_t, PointerSize::k32>(0u)));
+ EXPECT_EQ(value, (a32->GetElementPtrSizeUnchecked<uint64_t, PointerSize::k32>(0u)));
+ }
+
+ Handle<PointerArray> a64 =
+ hs.NewHandle(ObjPtr<PointerArray>::DownCast<Array>(LongArray::Alloc(soa.Self(), 1)));
+ ASSERT_TRUE(a64 != nullptr);
+ ASSERT_EQ(1, a64->GetLength());
+ EXPECT_EQ(0u, (a64->GetElementPtrSize<uint32_t, PointerSize::k64>(0u)));
+ EXPECT_EQ(0u, (a64->GetElementPtrSizeUnchecked<uint32_t, PointerSize::k64>(0u)));
+ for (uint64_t value : { UINT64_C(0),
+ UINT64_C(1),
+ UINT64_C(0x7fffffff),
+ UINT64_C(0x80000000),
+ UINT64_C(0xffffffff),
+ UINT64_C(0x100000000),
+ UINT64_C(0x7fffffffffffffff),
+ UINT64_C(0x8000000000000000),
+ UINT64_C(0xffffffffffffffff) }) {
+ a64->SetElementPtrSize(0u, value, PointerSize::k64);
+ EXPECT_EQ(value, (a64->GetElementPtrSize<uint64_t, PointerSize::k64>(0u)));
+ EXPECT_EQ(value, (a64->GetElementPtrSizeUnchecked<uint64_t, PointerSize::k64>(0u)));
+ }
+}
+
TEST_F(ObjectTest, PrimitiveArray_Double_Alloc) {
using ArrayT = DoubleArray;
ScopedObjectAccess soa(Thread::Current());