8346916: [REDO] align_up has potential overflow

src/hotspot/share/cds/metaspaceShared.cpp

@@ -214,27 +214,6 @@ void MetaspaceShared::dump_loaded_classes(const char* file_name, TRAPS) {
   }
 }
 
-// If p is not aligned, move it up to the next address that's aligned with alignment.
-// If this is not possible (because p is too high), return nullptr. Example:
-//     p = 0xffffffffffff0000, alignment= 0x10000    => return nullptr.
-static char* align_up_or_null(char* p, size_t alignment) {
-  assert(p != nullptr, "sanity");
-  if (is_aligned(p, alignment)) {
-    return p;
-  }
-
-  char* down = align_down(p, alignment);
-  if (max_uintx - uintx(down) < uintx(alignment)) {
-    // Run out of address space to align up.
-    return nullptr;
-  }
-
-  char* aligned = align_up(p, alignment);
-  assert(aligned >= p, "sanity");
-  assert(aligned != nullptr, "sanity");
-  return aligned;
-}
-
 static bool shared_base_too_high(char* specified_base, char* aligned_base, size_t cds_max) {
   // Caller should have checked if align_up_or_null( returns nullptr (comparing specified_base
   // with nullptr is UB).
@@ -262,6 +241,7 @@ static char* compute_shared_base(size_t cds_max) {
   }
 
   char* aligned_base = align_up_or_null(specified_base, alignment);
+  assert(is_aligned(aligned_base, alignment), "sanity");
 
   if (aligned_base != specified_base) {
     log_info(cds)("SharedBaseAddress (" INTPTR_FORMAT ") aligned up to " INTPTR_FORMAT,

src/hotspot/share/gc/parallel/psOldGen.cpp

@@ -190,7 +190,7 @@ bool PSOldGen::expand(size_t bytes) {
   assert(bytes > 0, "precondition");
 #endif
   const size_t alignment = virtual_space()->alignment();
-  size_t aligned_bytes  = align_up(bytes, alignment);
+  size_t aligned_bytes  = align_up_or_min(bytes, alignment);
   size_t aligned_expand_bytes = align_up(MinHeapDeltaBytes, alignment);
 
   if (UseNUMA) {

src/hotspot/share/gc/shared/gcArguments.cpp

@@ -142,13 +142,13 @@ void GCArguments::initialize_heap_flags_and_sizes() {
   // User inputs from -Xmx and -Xms must be aligned
   // Write back to flags if the values changed
   if (!is_aligned(MinHeapSize, HeapAlignment)) {
-    FLAG_SET_ERGO(MinHeapSize, align_up(MinHeapSize, HeapAlignment));
+    FLAG_SET_ERGO(MinHeapSize, align_up_or_min(MinHeapSize, HeapAlignment));
   }
   if (!is_aligned(InitialHeapSize, HeapAlignment)) {
-    FLAG_SET_ERGO(InitialHeapSize, align_up(InitialHeapSize, HeapAlignment));
+    FLAG_SET_ERGO(InitialHeapSize, align_up_or_min(InitialHeapSize, HeapAlignment));
   }
   if (!is_aligned(MaxHeapSize, HeapAlignment)) {
-    FLAG_SET_ERGO(MaxHeapSize, align_up(MaxHeapSize, HeapAlignment));
+    FLAG_SET_ERGO(MaxHeapSize, align_up_or_min(MaxHeapSize, HeapAlignment));
   }
 
   if (!FLAG_IS_DEFAULT(InitialHeapSize) && InitialHeapSize > MaxHeapSize) {
@@ -164,7 +164,7 @@ void GCArguments::initialize_heap_flags_and_sizes() {
     FLAG_SET_ERGO(SoftMaxHeapSize, MaxHeapSize);
   }
 
-  FLAG_SET_ERGO(MinHeapDeltaBytes, align_up(MinHeapDeltaBytes, SpaceAlignment));
+  FLAG_SET_ERGO(MinHeapDeltaBytes, align_up_or_min(MinHeapDeltaBytes, SpaceAlignment));
 
   if (checked_cast<uint>(ObjectAlignmentInBytes) > GCCardSizeInBytes) {
     err_msg message("ObjectAlignmentInBytes %u is larger than GCCardSizeInBytes %u",

src/hotspot/share/runtime/os.cpp

@@ -2011,7 +2011,7 @@ char* os::attempt_reserve_memory_between(char* min, char* max, size_t bytes, siz
   const size_t alignment_adjusted = MAX2(alignment, system_allocation_granularity);
 
   // Calculate first and last possible attach points:
-  char* const lo_att = align_up(MAX2(absolute_min, min), alignment_adjusted);
+  char* const lo_att = align_up_or_null(MAX2(absolute_min, min), alignment_adjusted);
   if (lo_att == nullptr) {
     return nullptr; // overflow
   }

src/hotspot/share/utilities/align.hpp

@@ -72,7 +72,19 @@ constexpr T align_down(T size, A alignment) {
 
 template<typename T, typename A, ENABLE_IF(std::is_integral<T>::value)>
 constexpr T align_up(T size, A alignment) {
-  T adjusted = checked_cast<T>(size + alignment_mask(alignment));
+  T mask = checked_cast<T>(alignment_mask(alignment));
+  assert(size <= std::numeric_limits<T>::max() - mask, "overflow");
+  T adjusted = size + mask;
+  return align_down(adjusted, alignment);
+}
+
+template<typename T, typename A, ENABLE_IF(std::is_integral<T>::value)>
+constexpr T align_up_or_min(T size, A alignment) {
+  T mask = checked_cast<T>(alignment_mask(alignment));
+  if (size > std::numeric_limits<T>::max() - mask) {
+    return std::numeric_limits<T>::min();
+  }
+  T adjusted = size + mask;
   return align_down(adjusted, alignment);
 }
 
@@ -90,6 +102,15 @@ inline T* align_up(T* ptr, A alignment) {
   return (T*)align_up((uintptr_t)ptr, alignment);
 }
 
+template <typename T, typename A>
+inline T* align_up_or_null(T* ptr, A alignment) {
+  uintptr_t up = align_up_or_min((uintptr_t)ptr, alignment);
+  if (up < (uintptr_t)ptr) { // we overflowed
+    return nullptr;
+  }
+  return (T*)up;
+}
+
 template <typename T, typename A>
 inline T* align_down(T* ptr, A alignment) {
   return (T*)align_down((uintptr_t)ptr, alignment);

test/hotspot/gtest/utilities/test_align.cpp

@@ -29,7 +29,7 @@
 #include <limits>
 
 // A few arbitrarily chosen values to test the align functions on.
-static constexpr uint64_t values[] = {1, 3, 10, 345, 1023, 1024, 1025, 23909034, INT_MAX, uint64_t(-1) / 2, uint64_t(-1) / 2 + 100, uint64_t(-1)};
+static constexpr uint64_t values[] = {1, 3, 10, 345, 1023, 1024, 1025, 23909034, INT_MAX, uint64_t(-1) / 2, uint64_t(-1) / 2 + 100, ~(uint64_t(1) << 62)};
 
 template <typename T>
 static constexpr T max_alignment() {
@@ -119,6 +119,10 @@ static void test_alignments() {
       if (0 < up && up <= (uint64_t)std::numeric_limits<T>::max()) {
         log("Testing align_up:   alignment: " UINT64_FORMAT_X " value: " UINT64_FORMAT_X " expected: " UINT64_FORMAT_X "\n", (uint64_t)alignment, values[i], up);
 
+        // Check against `align_up_or_min`
+        const uint64_t up2 = align_up_or_min(values[i], alignment);
+        ASSERT_EQ(up, up2);
+
         T value = T(values[i]);
 
         // Check against uint64_t version
@@ -194,3 +198,35 @@ TEST(Align, alignments) {
 
   test_alignments<int8_t, int8_t>();
 }
+
+#ifdef ASSERT
+template <typename T, typename A>
+static void test_fail_alignment() {
+  A alignment = max_alignment<A>();
+  T value = align_down(std::numeric_limits<T>::max(), alignment) + 1;
+  // Aligning value to alignment would now overflow.
+  ASSERT_EQ(align_up_or_min(value, alignment), std::numeric_limits<T>::min());
+  // Assert inside align_up expected.
+  T aligned = align_up(value, alignment);
+}
+
+TEST_VM_ASSERT(Align, fail_alignments_same_size) {
+  test_fail_alignment<uint64_t, uint64_t>();
+}
+
+TEST_VM_ASSERT(Align, fail_alignments_unsigned_signed) {
+  test_fail_alignment<uint32_t, int32_t>();
+}
+
+TEST_VM_ASSERT(Align, fail_alignments_signed_unsigned) {
+  test_fail_alignment<int64_t, uint32_t>();
+}
+
+TEST_VM_ASSERT(Align, fail_alignments_small_large) {
+  test_fail_alignment<uint8_t, uint64_t>();
+}
+
+TEST_VM_ASSERT(Align, fail_alignments_large_small) {
+  test_fail_alignment<uint64_t, uint8_t>();
+}
+#endif // ASSERT