ART: Add zero-padding to allocation request for String.
Also make the signed-to-unsigned conversion of utf16_length
in String::Alloc() explicit and make the overflow check take
into account the oveflow in the multiplication by
sizeof(uint16_t), i.e. being passed a negative utf16_length.
Bug: 23528461
Change-Id: Ia5844ea79f8f12e897f52fa27b0984358e3dea0b
diff --git a/runtime/mirror/string-inl.h b/runtime/mirror/string-inl.h
index eda6c9b..28a830d 100644
--- a/runtime/mirror/string-inl.h
+++ b/runtime/mirror/string-inl.h
@@ -146,8 +146,8 @@
inline size_t String::SizeOf() {
size_t size = sizeof(String) + (sizeof(uint16_t) * GetLength<kVerifyFlags>());
// String.equals() intrinsics assume zero-padding up to kObjectAlignment,
- // so make sure the padding is actually zero-initialized if the allocator
- // chooses to clear, or GC compaction chooses to copy, only SizeOf() bytes.
+ // so make sure the zero-padding is actually copied around if GC compaction
+ // chooses to copy only SizeOf() bytes.
// http://b/23528461
return RoundUp(size, kObjectAlignment);
}
@@ -155,21 +155,35 @@
template <bool kIsInstrumented, typename PreFenceVisitor>
inline String* String::Alloc(Thread* self, int32_t utf16_length, gc::AllocatorType allocator_type,
const PreFenceVisitor& pre_fence_visitor) {
- size_t header_size = sizeof(String);
- size_t data_size = sizeof(uint16_t) * utf16_length;
+ constexpr size_t header_size = sizeof(String);
+ static_assert(sizeof(utf16_length) <= sizeof(size_t),
+ "static_cast<size_t>(utf16_length) must not lose bits.");
+ size_t length = static_cast<size_t>(utf16_length);
+ size_t data_size = sizeof(uint16_t) * length;
size_t size = header_size + data_size;
+ // String.equals() intrinsics assume zero-padding up to kObjectAlignment,
+ // so make sure the allocator clears the padding as well.
+ // http://b/23528461
+ size_t alloc_size = RoundUp(size, kObjectAlignment);
Class* string_class = GetJavaLangString();
// Check for overflow and throw OutOfMemoryError if this was an unreasonable request.
- if (UNLIKELY(size < data_size)) {
+ // Do this by comparing with the maximum length that will _not_ cause an overflow.
+ constexpr size_t overflow_length = (-header_size) / sizeof(uint16_t); // Unsigned arithmetic.
+ constexpr size_t max_alloc_length = overflow_length - 1u;
+ static_assert(IsAligned<sizeof(uint16_t)>(kObjectAlignment),
+ "kObjectAlignment must be at least as big as Java char alignment");
+ constexpr size_t max_length = RoundDown(max_alloc_length, kObjectAlignment / sizeof(uint16_t));
+ if (UNLIKELY(length > max_length)) {
self->ThrowOutOfMemoryError(StringPrintf("%s of length %d would overflow",
PrettyDescriptor(string_class).c_str(),
utf16_length).c_str());
return nullptr;
}
+
gc::Heap* heap = Runtime::Current()->GetHeap();
return down_cast<String*>(
- heap->AllocObjectWithAllocator<kIsInstrumented, true>(self, string_class, size,
+ heap->AllocObjectWithAllocator<kIsInstrumented, true>(self, string_class, alloc_size,
allocator_type, pre_fence_visitor));
}