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));
 }