8295865: Several issues with os::realloc

Reviewed-by: dholmes, jsjolen

src/hotspot/share/runtime/os.cpp

@@ -705,21 +705,61 @@ void* os::realloc(void *memblock, size_t size, MEMFLAGS memflags, const NativeCa
     return NULL;
   }
 
-  const size_t new_outer_size = size + MemTracker::overhead_per_malloc();
+  if (MemTracker::enabled()) {
+    // NMT realloc handling
 
-  // If NMT is enabled, this checks for heap overwrites, then de-accounts the old block.
-  void* const old_outer_ptr = MemTracker::record_free(memblock);
+    const size_t new_outer_size = size + MemTracker::overhead_per_malloc();
+
+    // Handle size overflow.
+    if (new_outer_size < size) {
+      return NULL;
+    }
+
+    const size_t old_size = MallocTracker::malloc_header(memblock)->size();
+
+    // De-account the old block from NMT *before* calling the real realloc(3) since it
+    // may invalidate old block including its header. This will also perform integrity checks
+    // on the old block (e.g. overwriters) and mark the old header as dead.
+    void* const old_outer_ptr = MemTracker::record_free(memblock);
+
+    // the real realloc
+    ALLOW_C_FUNCTION(::realloc, void* const new_outer_ptr = ::realloc(old_outer_ptr, new_outer_size);)
+
+    if (new_outer_ptr == NULL) {
+      // If realloc(3) failed, the old block still exists. We must re-instantiate the old
+      // NMT header then, since we marked it dead already. Otherwise subsequent os::realloc()
+      // or os::free() calls would trigger block integrity asserts.
+      void* p = MemTracker::record_malloc(old_outer_ptr, old_size, memflags, stack);
+      assert(p == memblock, "sanity");
+      return NULL;
+    }
+
+    // After a successful realloc(3), we re-account the resized block with its new size
+    // to NMT. This re-instantiates the NMT header.
+    void* const new_inner_ptr = MemTracker::record_malloc(new_outer_ptr, size, memflags, stack);
+
+#ifdef ASSERT
+    if (old_size < size) {
+      // We also zap the newly extended region.
+      ::memset((char*)new_inner_ptr + old_size, uninitBlockPad, size - old_size);
+    }
+#endif
+
+    rc = new_inner_ptr;
+
+  } else {
+
+    // NMT disabled.
+    ALLOW_C_FUNCTION(::realloc, rc = ::realloc(memblock, size);)
+    if (rc == NULL) {
+      return NULL;
+    }
 
-  ALLOW_C_FUNCTION(::realloc, void* const new_outer_ptr = ::realloc(old_outer_ptr, new_outer_size);)
-  if (new_outer_ptr == NULL) {
-    return NULL;
   }
 
-  void* const new_inner_ptr = MemTracker::record_malloc(new_outer_ptr, size, memflags, stack);
+  DEBUG_ONLY(break_if_ptr_caught(rc);)
 
-  DEBUG_ONLY(break_if_ptr_caught(new_inner_ptr);)
-
-  return new_inner_ptr;
+  return rc;
 }
 
 void  os::free(void *memblock) {

src/hotspot/share/services/mallocTracker.hpp

@@ -313,7 +313,6 @@ class MallocTracker : AllStatic {
   // signals popping up, e.g. when writing an hs_err file.
   static bool print_pointer_information(const void* p, outputStream* st);
 
- private:
   static inline MallocHeader* malloc_header(void *memblock) {
     assert(memblock != NULL, "NULL pointer");
     return (MallocHeader*)((char*)memblock - sizeof(MallocHeader));

test/hotspot/gtest/nmt/test_nmt_cornercases.cpp

@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2022 SAP SE. All rights reserved.
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "precompiled.hpp"
+#include "memory/allocation.hpp"
+#include "runtime/os.hpp"
+#include "services/mallocHeader.inline.hpp"
+#include "services/mallocTracker.hpp"
+#include "services/memTracker.hpp"
+#include "testutils.hpp"
+#include "unittest.hpp"
+
+// Check NMT header for integrity, as well as expected type and size.
+static void check_expected_malloc_header(const void* payload, MEMFLAGS type, size_t size) {
+  const MallocHeader* hdr = MallocTracker::malloc_header(payload);
+  hdr->assert_block_integrity();
+  EXPECT_EQ(hdr->size(), size);
+  EXPECT_EQ(hdr->flags(), type);
+}
+
+// Check that a malloc with an overflowing size is rejected.
+TEST_VM(NMT, malloc_failure1) {
+  void* p = os::malloc(SIZE_MAX, mtTest);
+  EXPECT_NULL(p);
+}
+
+// Check that gigantic mallocs are rejected, even if no size overflow happens.
+TEST_VM(NMT, malloc_failure2) {
+  void* p = os::malloc(SIZE_MAX - M, mtTest);
+  EXPECT_NULL(p);
+}
+
+// Check correct handling of failing reallocs.
+static void check_failing_realloc(size_t failing_request_size) {
+
+  // We test this with both NMT enabled and disabled.
+  bool nmt_enabled = MemTracker::enabled();
+  const size_t first_size = 0x100;
+
+  void* p = os::malloc(first_size, mtTest);
+  EXPECT_NOT_NULL(p);
+  if (nmt_enabled) {
+    check_expected_malloc_header(p, mtTest, first_size);
+  }
+  GtestUtils::mark_range(p, first_size);
+
+  // should fail
+  void* p2 = os::realloc(p, failing_request_size, mtTest);
+  EXPECT_NULL(p2);
+
+  // original allocation should still be intact
+  GtestUtils::check_range(p, first_size);
+  if (nmt_enabled) {
+    check_expected_malloc_header(p, mtTest, first_size);
+  }
+
+  os::free(p);
+}
+
+TEST_VM(NMT, realloc_failure_overflowing_size) {
+  check_failing_realloc(SIZE_MAX);
+  check_failing_realloc(SIZE_MAX - MemTracker::overhead_per_malloc());
+}
+
+TEST_VM(NMT, realloc_failure_gigantic_size) {
+  check_failing_realloc(SIZE_MAX - M);
+}
+
+static void* do_realloc(void* p, size_t old_size, size_t new_size, uint8_t old_content, bool check_nmt_header) {
+
+  EXPECT_NOT_NULL(p);
+  if (check_nmt_header) {
+    check_expected_malloc_header(p, mtTest, old_size);
+  }
+
+  void* p2 = os::realloc(p, new_size, mtTest);
+
+  EXPECT_NOT_NULL(p2);
+  if (check_nmt_header) {
+    check_expected_malloc_header(p2, mtTest, new_size);
+  }
+
+  // Check old content, and possibly zapped area (if block grew)
+  if (old_size < new_size) {
+    GtestUtils::check_range((char*)p2, old_size, old_content);
+#ifdef ASSERT
+    GtestUtils::check_range((char*)p2 + old_size, new_size - old_size, uninitBlockPad);
+#endif
+  } else {
+    GtestUtils::check_range((char*)p2, new_size, old_content);
+  }
+
+  return p2;
+}
+
+// Check a random sequence of reallocs. For enlarging reallocs, we expect the
+// newly allocated memory to be zapped (in debug) while the old section should be
+// left intact.
+TEST_VM(NMT, random_reallocs) {
+
+  bool nmt_enabled = MemTracker::enabled();
+  size_t size = 256;
+  uint8_t content = 'A';
+
+  void* p = os::malloc(size, mtTest);
+  ASSERT_NOT_NULL(p);
+  if (nmt_enabled) {
+    check_expected_malloc_header(p, mtTest, size);
+  }
+  GtestUtils::mark_range_with(p, size, content);
+
+  for (int n = 0; n < 100; n ++) {
+    size_t new_size = (size_t)(os::random() % 512) + 1;
+    // LOG_HERE("reallocating " SIZE_FORMAT "->" SIZE_FORMAT, size, new_size);
+    p = do_realloc(p, size, new_size, content, nmt_enabled);
+    size = new_size;
+    content = (n % 26) + 'A';
+    GtestUtils::mark_range_with(p, size, content);
+  }
+
+  os::free(p);
+}

test/hotspot/gtest/testutils.hpp

@@ -50,9 +50,13 @@ public:
 #define ASSERT_RANGE_IS_MARKED_WITH(p, size, mark)  ASSERT_TRUE(GtestUtils::check_range(p, size, mark))
 #define ASSERT_RANGE_IS_MARKED(p, size)             ASSERT_TRUE(GtestUtils::check_range(p, size))
 
-// Convenience asserts
+// Mimicking the official ASSERT_xx and EXPECT_xx counterparts of the googletest suite.
+// (ASSERT|EXPECT)_NOT_NULL: check that the given pointer is not NULL
+// (ASSERT|EXPECT)_NULL: check that the given pointer is NULL
 #define ASSERT_NOT_NULL(p)  ASSERT_NE(p2i(p), 0)
 #define ASSERT_NULL(p)      ASSERT_EQ(p2i(p), 0)
+#define EXPECT_NOT_NULL(p)  EXPECT_NE(p2i(p), 0)
+#define EXPECT_NULL(p)      EXPECT_EQ(p2i(p), 0)
 
 #define ASSERT_ALIGN(p, n) ASSERT_TRUE(is_aligned(p, n))