8277822: Remove debug-only heap overrun checks in os::malloc and friends

Reviewed-by: coleenp, zgu

Author: tstuefe

src/hotspot/share/runtime/globals.hpp

@@ -543,7 +543,7 @@ const intx ObjectAlignmentInBytes = 8;
           "compression. Otherwise the level must be between 1 and 9.")      \
           range(0, 9)                                                       \
                                                                             \
-  product(ccstr, NativeMemoryTracking, "off",                               \
+  product(ccstr, NativeMemoryTracking, DEBUG_ONLY("summary") NOT_DEBUG("off"), \
           "Native memory tracking options")                                 \
                                                                             \
   product(bool, PrintNMTStatistics, false, DIAGNOSTIC,                      \

src/hotspot/share/runtime/os.cpp

@@ -38,7 +38,6 @@
 #include "logging/log.hpp"
 #include "logging/logStream.hpp"
 #include "memory/allocation.inline.hpp"
-#include "memory/guardedMemory.hpp"
 #include "memory/resourceArea.hpp"
 #include "memory/universe.hpp"
 #include "oops/compressedOops.inline.hpp"
@@ -83,13 +82,6 @@ int               os::_processor_count    = 0;
 int               os::_initial_active_processor_count = 0;
 os::PageSizes     os::_page_sizes;
 
-#ifndef PRODUCT
-julong os::num_mallocs = 0;         // # of calls to malloc/realloc
-julong os::alloc_bytes = 0;         // # of bytes allocated
-julong os::num_frees = 0;           // # of calls to free
-julong os::free_bytes = 0;          // # of bytes freed
-#endif
-
 static size_t cur_malloc_words = 0;  // current size for MallocMaxTestWords
 
 DEBUG_ONLY(bool os::_mutex_init_done = false;)
@@ -603,30 +595,11 @@ char* os::strdup_check_oom(const char* str, MEMFLAGS flags) {
   return p;
 }
 
-
-#define paranoid                 0  /* only set to 1 if you suspect checking code has bug */
-
-#ifdef ASSERT
-
-static void verify_memory(void* ptr) {
-  GuardedMemory guarded(ptr);
-  if (!guarded.verify_guards()) {
-    LogTarget(Warning, malloc, free) lt;
-    ResourceMark rm;
-    LogStream ls(lt);
-    ls.print_cr("## nof_mallocs = " UINT64_FORMAT ", nof_frees = " UINT64_FORMAT, os::num_mallocs, os::num_frees);
-    ls.print_cr("## memory stomp:");
-    guarded.print_on(&ls);
-    fatal("memory stomping error");
-  }
-}
-
-#endif
-
 //
 // This function supports testing of the malloc out of memory
 // condition without really running the system out of memory.
 //
+
 static bool has_reached_max_malloc_test_peak(size_t alloc_size) {
   if (MallocMaxTestWords > 0) {
     size_t words = (alloc_size / BytesPerWord);
@@ -639,13 +612,24 @@ static bool has_reached_max_malloc_test_peak(size_t alloc_size) {
   return false;
 }
 
+#ifdef ASSERT
+static void check_crash_protection() {
+  assert(!os::ThreadCrashProtection::is_crash_protected(Thread::current_or_null()),
+         "not allowed when crash protection is set");
+}
+static void break_if_ptr_caught(void* ptr) {
+  if (p2i(ptr) == (intptr_t)MallocCatchPtr) {
+    log_warning(malloc, free)("ptr caught: " PTR_FORMAT, p2i(ptr));
+    breakpoint();
+  }
+}
+#endif // ASSERT
+
 void* os::malloc(size_t size, MEMFLAGS flags) {
   return os::malloc(size, flags, CALLER_PC);
 }
 
 void* os::malloc(size_t size, MEMFLAGS memflags, const NativeCallStack& stack) {
-  NOT_PRODUCT(inc_stat_counter(&num_mallocs, 1));
-  NOT_PRODUCT(inc_stat_counter(&alloc_bytes, size));
 
 #if INCLUDE_NMT
   {
@@ -656,58 +640,35 @@ void* os::malloc(size_t size, MEMFLAGS memflags, const NativeCallStack& stack) {
   }
 #endif
 
-  // Since os::malloc can be called when the libjvm.{dll,so} is
-  // first loaded and we don't have a thread yet we must accept NULL also here.
-  assert(!os::ThreadCrashProtection::is_crash_protected(Thread::current_or_null()),
-         "malloc() not allowed when crash protection is set");
+  DEBUG_ONLY(check_crash_protection());
 
-  if (size == 0) {
-    // return a valid pointer if size is zero
-    // if NULL is returned the calling functions assume out of memory.
-    size = 1;
-  }
-
-  // NMT support
-  NMT_TrackingLevel level = MemTracker::tracking_level();
-  const size_t nmt_overhead =
-      MemTracker::malloc_header_size(level) + MemTracker::malloc_footer_size(level);
-
-#ifndef ASSERT
-  const size_t alloc_size = size + nmt_overhead;
-#else
-  const size_t alloc_size = GuardedMemory::get_total_size(size + nmt_overhead);
-  if (size + nmt_overhead > alloc_size) { // Check for rollover.
-    return NULL;
-  }
-#endif
+  // On malloc(0), implementators of malloc(3) have the choice to return either
+  // NULL or a unique non-NULL pointer. To unify libc behavior across our platforms
+  // we chose the latter.
+  size = MAX2((size_t)1, size);
 
   // For the test flag -XX:MallocMaxTestWords
   if (has_reached_max_malloc_test_peak(size)) {
     return NULL;
   }
 
-  u_char* ptr;
-  ptr = (u_char*)::malloc(alloc_size);
+  const NMT_TrackingLevel level = MemTracker::tracking_level();
+  const size_t nmt_overhead =
+      MemTracker::malloc_header_size(level) + MemTracker::malloc_footer_size(level);
+
+  const size_t outer_size = size + nmt_overhead;
 
-#ifdef ASSERT
-  if (ptr == NULL) {
+  void* const outer_ptr = (u_char*)::malloc(outer_size);
+  if (outer_ptr == NULL) {
     return NULL;
   }
-  // Wrap memory with guard
-  GuardedMemory guarded(ptr, size + nmt_overhead);
-  ptr = guarded.get_user_ptr();
 
-  if ((intptr_t)ptr == (intptr_t)MallocCatchPtr) {
-    log_warning(malloc, free)("os::malloc caught, " SIZE_FORMAT " bytes --> " PTR_FORMAT, size, p2i(ptr));
-    breakpoint();
-  }
-  if (paranoid) {
-    verify_memory(ptr);
-  }
-#endif
+  void* inner_ptr = MemTracker::record_malloc((address)outer_ptr, size, memflags, stack, level);
 
-  // we do not track guard memory
-  return MemTracker::record_malloc((address)ptr, size, memflags, stack, level);
+  DEBUG_ONLY(::memset(inner_ptr, uninitBlockPad, size);)
+  DEBUG_ONLY(break_if_ptr_caught(inner_ptr);)
+
+  return inner_ptr;
 }
 
 void* os::realloc(void *memblock, size_t size, MEMFLAGS flags) {
@@ -725,59 +686,41 @@ void* os::realloc(void *memblock, size_t size, MEMFLAGS memflags, const NativeCa
   }
 #endif
 
+  if (memblock == NULL) {
+    return os::malloc(size, memflags, stack);
+  }
+
+  DEBUG_ONLY(check_crash_protection());
+
+  // On realloc(p, 0), implementators of realloc(3) have the choice to return either
+  // NULL or a unique non-NULL pointer. To unify libc behavior across our platforms
+  // we chose the latter.
+  size = MAX2((size_t)1, size);
+
   // For the test flag -XX:MallocMaxTestWords
   if (has_reached_max_malloc_test_peak(size)) {
     return NULL;
   }
 
-  if (size == 0) {
-    // return a valid pointer if size is zero
-    // if NULL is returned the calling functions assume out of memory.
-    size = 1;
-  }
-
-#ifndef ASSERT
-  NOT_PRODUCT(inc_stat_counter(&num_mallocs, 1));
-  NOT_PRODUCT(inc_stat_counter(&alloc_bytes, size));
-   // NMT support
-  NMT_TrackingLevel level = MemTracker::tracking_level();
-  void* membase = MemTracker::record_free(memblock, level);
+  const NMT_TrackingLevel level = MemTracker::tracking_level();
   const size_t nmt_overhead =
       MemTracker::malloc_header_size(level) + MemTracker::malloc_footer_size(level);
-  void* ptr = ::realloc(membase, size + nmt_overhead);
-  return MemTracker::record_malloc(ptr, size, memflags, stack, level);
-#else
-  if (memblock == NULL) {
-    return os::malloc(size, memflags, stack);
-  }
-  if ((intptr_t)memblock == (intptr_t)MallocCatchPtr) {
-    log_warning(malloc, free)("os::realloc caught " PTR_FORMAT, p2i(memblock));
-    breakpoint();
-  }
-  // NMT support
-  void* membase = MemTracker::malloc_base(memblock);
-  verify_memory(membase);
-  // always move the block
-  void* ptr = os::malloc(size, memflags, stack);
-  // Copy to new memory if malloc didn't fail
-  if (ptr != NULL ) {
-    GuardedMemory guarded(MemTracker::malloc_base(memblock));
-    // Guard's user data contains NMT header
-    NMT_TrackingLevel level = MemTracker::tracking_level();
-    const size_t nmt_overhead =
-        MemTracker::malloc_header_size(level) + MemTracker::malloc_footer_size(level);
-    size_t memblock_size = guarded.get_user_size() - nmt_overhead;
-    memcpy(ptr, memblock, MIN2(size, memblock_size));
-    if (paranoid) {
-      verify_memory(MemTracker::malloc_base(ptr));
-    }
-    os::free(memblock);
-  }
-  return ptr;
-#endif
+
+  const size_t new_outer_size = size + nmt_overhead;
+
+  // If NMT is enabled, this checks for heap overwrites, then de-accounts the old block.
+  void* const old_outer_ptr = MemTracker::record_free(memblock, level);
+
+  void* const new_outer_ptr = ::realloc(old_outer_ptr, new_outer_size);
+
+  // If NMT is enabled, this checks for heap overwrites, then de-accounts the old block.
+  void* const new_inner_ptr = MemTracker::record_malloc(new_outer_ptr, size, memflags, stack, level);
+
+  DEBUG_ONLY(break_if_ptr_caught(new_inner_ptr);)
+
+  return new_inner_ptr;
 }
 
-// handles NULL pointers
 void  os::free(void *memblock) {
 
 #if INCLUDE_NMT
@@ -786,25 +729,17 @@ void  os::free(void *memblock) {
   }
 #endif
 
-  NOT_PRODUCT(inc_stat_counter(&num_frees, 1));
-#ifdef ASSERT
-  if (memblock == NULL) return;
-  if ((intptr_t)memblock == (intptr_t)MallocCatchPtr) {
-    log_warning(malloc, free)("os::free caught " PTR_FORMAT, p2i(memblock));
-    breakpoint();
+  if (memblock == NULL) {
+    return;
   }
-  void* membase = MemTracker::record_free(memblock, MemTracker::tracking_level());
-  verify_memory(membase);
 
-  GuardedMemory guarded(membase);
-  size_t size = guarded.get_user_size();
-  inc_stat_counter(&free_bytes, size);
-  membase = guarded.release_for_freeing();
-  ::free(membase);
-#else
-  void* membase = MemTracker::record_free(memblock, MemTracker::tracking_level());
-  ::free(membase);
-#endif
+  DEBUG_ONLY(break_if_ptr_caught(memblock);)
+
+  const NMT_TrackingLevel level = MemTracker::tracking_level();
+
+  // If NMT is enabled, this checks for heap overwrites, then de-accounts the old block.
+  void* const old_outer_ptr = MemTracker::record_free(memblock, level);
+  ::free(old_outer_ptr);
 }
 
 void os::init_random(unsigned int initval) {

src/hotspot/share/runtime/os.hpp

@@ -784,13 +784,6 @@ class os: AllStatic {
   // Like strdup, but exit VM when strdup() returns NULL
   static char* strdup_check_oom(const char*, MEMFLAGS flags = mtInternal);
 
-#ifndef PRODUCT
-  static julong num_mallocs;         // # of calls to malloc/realloc
-  static julong alloc_bytes;         // # of bytes allocated
-  static julong num_frees;           // # of calls to free
-  static julong free_bytes;          // # of bytes freed
-#endif
-
   // SocketInterface (ex HPI SocketInterface )
   static int socket(int domain, int type, int protocol);
   static int socket_close(int fd);

test/hotspot/jtreg/gtest/NMTGtests.java

@@ -24,26 +24,29 @@
  */
 
 /*
- * This tests NMT by running gtests with NMT enabled.
- *
- * To save time, we just run them for debug builds (where we would catch assertions) and only a selection of tests
- * (namely, NMT tests themselves, and - for the detail statistics - os tests, since those reserve a lot and stress NMT)
+ * This tests NMT by running gtests with NMT enabled (only those which are relevant for NMT)
+ */
+
+/* @test id=nmt-off
+ * @summary Run NMT-related gtests with NMT switched off
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.xml
+ * @run main/native GTestWrapper --gtest_filter=NMT*:os* -XX:NativeMemoryTracking=off
  */
 
 /* @test id=nmt-summary
  * @summary Run NMT-related gtests with summary statistics
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *          java.xml
- * @requires vm.debug
- * @run main/native GTestWrapper --gtest_filter=NMT* -XX:NativeMemoryTracking=summary
+ * @run main/native GTestWrapper --gtest_filter=NMT*:os* -XX:NativeMemoryTracking=summary
  */
 
 /* @test id=nmt-detail
  * @summary Run NMT-related gtests with detailed statistics
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *          java.xml
- * @requires vm.debug
  * @run main/native GTestWrapper --gtest_filter=NMT*:os* -XX:NativeMemoryTracking=detail
  */

test/hotspot/jtreg/runtime/NMT/JcmdWithNMTDisabled.java

@@ -30,6 +30,7 @@
  * @run driver JcmdWithNMTDisabled 1
  */
 
+import jdk.test.lib.Platform;
 import jdk.test.lib.process.ProcessTools;
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.JDKToolFinder;
@@ -47,10 +48,12 @@ public static void main(String args[]) throws Exception {
       OutputAnalyzer output;
       String testjdkPath = System.getProperty("test.jdk");
 
-      // First run without enabling NMT
-      pb = ProcessTools.createJavaProcessBuilder("-Dtest.jdk=" + testjdkPath, "JcmdWithNMTDisabled");
-      output = new OutputAnalyzer(pb.start());
-      output.shouldHaveExitValue(0);
+      // First run without enabling NMT (not in debug, where NMT is by default on)
+      if (!Platform.isDebugBuild()) {
+        pb = ProcessTools.createJavaProcessBuilder("-Dtest.jdk=" + testjdkPath, "JcmdWithNMTDisabled");
+        output = new OutputAnalyzer(pb.start());
+        output.shouldHaveExitValue(0);
+      }
 
       // Then run with explicitly disabling NMT, should not be any difference
       pb = ProcessTools.createJavaProcessBuilder("-Dtest.jdk=" + testjdkPath, "-XX:NativeMemoryTracking=off", "JcmdWithNMTDisabled");

test/hotspot/jtreg/runtime/NMT/PrintNMTStatisticsWithNMTDisabled.java

@@ -38,7 +38,7 @@ public class PrintNMTStatisticsWithNMTDisabled {
   public static void main(String args[]) throws Exception {
     ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
       "-XX:+UnlockDiagnosticVMOptions",
-      "-XX:+PrintNMTStatistics",
+      "-XX:+PrintNMTStatistics", "-XX:NativeMemoryTracking=off",
       "-version");
     OutputAnalyzer output = new OutputAnalyzer(pb.start());
     output.shouldContain("warning: PrintNMTStatistics is disabled, because native memory tracking is not enabled");