8286876: NMT.test_unaliged_block_address_vm_assert fails if using clang toolchain

Reviewed-by: stuefe, gziemski

src/hotspot/share/runtime/os.cpp

@@ -726,8 +726,7 @@ void* os::realloc(void *memblock, size_t size, MEMFLAGS memflags, const NativeCa
 
     // Perform integrity checks on and mark the old block as dead *before* calling the real realloc(3) since it
     // may invalidate the old block, including its header.
-    MallocHeader* header = MallocTracker::malloc_header(memblock);
-    header->assert_block_integrity(); // Assert block hasn't been tampered with.
+    MallocHeader* header = MallocHeader::resolve_checked(memblock);
     const MallocHeader::FreeInfo free_info = header->free_info();
     header->mark_block_as_dead();
 

src/hotspot/share/services/mallocHeader.hpp

@@ -114,7 +114,10 @@ class MallocHeader {
   uint16_t get_footer() const       { return build_footer(footer_address()[0], footer_address()[1]); }
   void set_footer(uint16_t v)       { footer_address()[0] = v >> 8; footer_address()[1] = (uint8_t)v; }
 
- public:
+  template<typename InTypeParam, typename OutTypeParam>
+  inline static OutTypeParam resolve_checked_impl(InTypeParam memblock);
+
+public:
   // Contains all of the necessary data to to deaccount block with NMT.
   struct FreeInfo {
     const size_t size;
@@ -140,10 +143,15 @@ class MallocHeader {
   // an option pointer to the corruption in p_corruption, and return false.
   // Return true if block is fine.
   inline bool check_block_integrity(char* msg, size_t msglen, address* p_corruption) const;
+  // Check correct alignment and placement of pointer, fill in short descriptive text and return false
+  // if this is not the case.
+  // Returns true if the memblock looks OK.
+  inline static bool is_valid_malloced_pointer(const void* payload, char* msg, size_t msglen);
 
   // If block is broken, print out a report to tty (optionally with
   // hex dump surrounding the broken block), then trigger a fatal error
-  inline void assert_block_integrity() const;
+  inline static const MallocHeader* resolve_checked(const void* memblock);
+  inline static MallocHeader* resolve_checked(void* memblock);
 };
 
 // This needs to be true on both 64-bit and 32-bit platforms

src/hotspot/share/services/mallocHeader.inline.hpp

@@ -61,23 +61,13 @@ inline void MallocHeader::mark_block_as_dead() {
   set_footer(_footer_canary_dead_mark);
 }
 
-inline void MallocHeader::assert_block_integrity() const {
-  char msg[256];
-  address corruption = nullptr;
-  if (!check_block_integrity(msg, sizeof(msg), &corruption)) {
-    print_block_on_error(tty, corruption != nullptr ? corruption : (address)this);
-    fatal("NMT corruption: Block at " PTR_FORMAT ": %s", p2i(this), msg);
-  }
-}
-
-inline bool MallocHeader::check_block_integrity(char* msg, size_t msglen, address* p_corruption) const {
-  // Note: if you modify the error messages here, make sure you
-  // adapt the associated gtests too.
-
+inline bool MallocHeader::is_valid_malloced_pointer(const void* payload, char* msg, size_t msglen) {
+  // Handle the pointer as an integral type
+  uintptr_t ptr = reinterpret_cast<uintptr_t>(payload);
   // Weed out obviously wrong block addresses of null or very low
-  // values. Note that we should not call this for ::free(null),
+  // values. Note that we should not call this for ::free(nullptr),
   // which should be handled by os::free() above us.
-  if (((size_t)p2i(this)) < K) {
+  if (ptr < K) {
     jio_snprintf(msg, msglen, "invalid block address");
     return false;
   }
@@ -97,11 +87,38 @@ inline bool MallocHeader::check_block_integrity(char* msg, size_t msglen, addres
   // we test the smallest alignment we know.
   // Should we ever start using std::max_align_t, this would be one place to
   // fix up.
-  if (!is_aligned(this, sizeof(uint64_t))) {
-    *p_corruption = (address)this;
+  if (!is_aligned(ptr, sizeof(uint64_t))) {
     jio_snprintf(msg, msglen, "block address is unaligned");
     return false;
   }
+  return true;
+}
+
+template<typename InTypeParam, typename OutTypeParam>
+inline OutTypeParam MallocHeader::resolve_checked_impl(InTypeParam memblock) {
+  char msg[256];
+  address corruption = nullptr;
+  if (!is_valid_malloced_pointer(memblock, msg, sizeof(msg))) {
+    fatal("Not a valid malloc pointer: " PTR_FORMAT ": %s", p2i(memblock), msg);
+  }
+  OutTypeParam header_pointer = (OutTypeParam)memblock - 1;
+  if (!header_pointer->check_block_integrity(msg, sizeof(msg), &corruption)) {
+    header_pointer->print_block_on_error(tty, corruption != nullptr ? corruption : (address)header_pointer);
+    fatal("NMT corruption: Block at " PTR_FORMAT ": %s", p2i(memblock), msg);
+  }
+  return header_pointer;
+}
+
+inline MallocHeader* MallocHeader::resolve_checked(void* memblock) {
+  return MallocHeader::resolve_checked_impl<void*, MallocHeader*>(memblock);
+}
+inline const MallocHeader* MallocHeader::resolve_checked(const void* memblock) {
+  return MallocHeader::resolve_checked_impl<const void*, const MallocHeader*>(memblock);
+}
+
+inline bool MallocHeader::check_block_integrity(char* msg, size_t msglen, address* p_corruption) const {
+  // Note: if you modify the error messages here, make sure you
+  // adapt the associated gtests too.
 
   // Check header canary
   if (_canary != _header_canary_life_mark) {
@@ -130,7 +147,7 @@ inline bool MallocHeader::check_block_integrity(char* msg, size_t msglen, addres
   if (get_footer() != _footer_canary_life_mark) {
     *p_corruption = footer_address();
     jio_snprintf(msg, msglen, "footer canary broken at " PTR_FORMAT " (buffer overflow?)",
-                p2i(footer_address()));
+                 p2i(footer_address()));
     return false;
   }
   return true;

src/hotspot/share/services/mallocTracker.cpp

@@ -170,10 +170,9 @@ void* MallocTracker::record_malloc(void* malloc_base, size_t size, MEMFLAGS flag
 #ifdef ASSERT
   // Read back
   {
-    MallocHeader* const header2 = malloc_header(memblock);
+    const MallocHeader* header2 = MallocHeader::resolve_checked(memblock);
     assert(header2->size() == size, "Wrong size");
     assert(header2->flags() == flags, "Wrong flags");
-    header2->assert_block_integrity();
   }
 #endif
 
@@ -184,8 +183,7 @@ void* MallocTracker::record_free_block(void* memblock) {
   assert(MemTracker::enabled(), "Sanity");
   assert(memblock != nullptr, "precondition");
 
-  MallocHeader* const header = malloc_header(memblock);
-  header->assert_block_integrity();
+  MallocHeader* header = MallocHeader::resolve_checked(memblock);
 
   deaccount(header->free_info());
 

src/hotspot/share/services/mallocTracker.hpp

@@ -330,11 +330,11 @@ class MallocTracker : AllStatic {
 
   static inline MallocHeader* malloc_header(void *memblock) {
     assert(memblock != nullptr, "null pointer");
-    return (MallocHeader*)((char*)memblock - sizeof(MallocHeader));
+    return (MallocHeader*)memblock -1;
   }
   static inline const MallocHeader* malloc_header(const void *memblock) {
     assert(memblock != nullptr, "null pointer");
-    return (const MallocHeader*)((const char*)memblock - sizeof(MallocHeader));
+    return (const MallocHeader*)memblock -1;
   }
 };
 

test/hotspot/gtest/nmt/test_nmt_cornercases.cpp

@@ -33,8 +33,7 @@
 
 // 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();
+  const MallocHeader* hdr = MallocHeader::resolve_checked(payload);
   EXPECT_EQ(hdr->size(), size);
   EXPECT_EQ(hdr->flags(), type);
 }