8058251: assert(_count > 0) failed: Negative counter when running runtime/NMT/MallocTrackingVerify.java

Fixed an issue when overflowing the MallocSite hash table bucket

Reviewed-by: coleenp, gtriantafill

hotspot/src/share/vm/prims/whitebox.cpp

@@ -282,7 +282,7 @@ WB_END
 // NMT picks it up correctly
 WB_ENTRY(jlong, WB_NMTMalloc(JNIEnv* env, jobject o, jlong size))
   jlong addr = 0;
-    addr = (jlong)(uintptr_t)os::malloc(size, mtTest);
+  addr = (jlong)(uintptr_t)os::malloc(size, mtTest);
   return addr;
 WB_END
 
@@ -291,7 +291,7 @@ WB_END
 WB_ENTRY(jlong, WB_NMTMallocWithPseudoStack(JNIEnv* env, jobject o, jlong size, jint pseudo_stack))
   address pc = (address)(size_t)pseudo_stack;
   NativeCallStack stack(&pc, 1);
-  return (jlong)os::malloc(size, mtTest, stack);
+  return (jlong)(uintptr_t)os::malloc(size, mtTest, stack);
 WB_END
 
 // Free the memory allocated by NMTAllocTest

hotspot/src/share/vm/runtime/os.cpp

@@ -571,17 +571,6 @@ 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
-  // NMT can not track malloc allocation size > MAX_MALLOC_SIZE, which is
-  // (1GB - 1) on 32-bit system. It is not an issue on 64-bit system, where
-  // MAX_MALLOC_SIZE = ((1 << 62) - 1).
-  // VM code does not have such large malloc allocation. However, it can come
-  // Unsafe call.
-  if (MemTracker::tracking_level() >= NMT_summary && size > MAX_MALLOC_SIZE) {
-    return NULL;
-  }
-#endif
-
 #ifdef ASSERT
   // checking for the WatcherThread and crash_protection first
   // since os::malloc can be called when the libjvm.{dll,so} is
@@ -652,12 +641,6 @@ void* os::realloc(void *memblock, size_t size, MEMFLAGS flags) {
 }
 
 void* os::realloc(void *memblock, size_t size, MEMFLAGS memflags, const NativeCallStack& stack) {
-#if INCLUDE_NMT
-  // See comments in os::malloc() above
-  if (MemTracker::tracking_level() >= NMT_summary && size > MAX_MALLOC_SIZE) {
-    return NULL;
-  }
-#endif
 
 #ifndef ASSERT
   NOT_PRODUCT(inc_stat_counter(&num_mallocs, 1));

hotspot/src/share/vm/services/mallocTracker.cpp

@@ -72,7 +72,7 @@ void MallocHeader::release() const {
 
   MallocMemorySummary::record_free(size(), flags());
   MallocMemorySummary::record_free_malloc_header(sizeof(MallocHeader));
-  if (tracking_level() == NMT_detail) {
+  if (MemTracker::tracking_level() == NMT_detail) {
     MallocSiteTable::deallocation_at(size(), _bucket_idx, _pos_idx);
   }
 }
@@ -128,36 +128,18 @@ void* MallocTracker::record_malloc(void* malloc_base, size_t size, MEMFLAGS flag
   }
 
   // Uses placement global new operator to initialize malloc header
-  switch(level) {
-    case NMT_off:
-      return malloc_base;
-    case NMT_minimal: {
-      MallocHeader* hdr = ::new (malloc_base) MallocHeader();
-      break;
-    }
-    case NMT_summary: {
-      assert(size <= MAX_MALLOC_SIZE, "malloc size overrun for NMT");
-      header = ::new (malloc_base) MallocHeader(size, flags);
-      break;
-    }
-    case NMT_detail: {
-      assert(size <= MAX_MALLOC_SIZE, "malloc size overrun for NMT");
-      header = ::new (malloc_base) MallocHeader(size, flags, stack);
-      break;
-    }
-    default:
-      ShouldNotReachHere();
+
+  if (level == NMT_off) {
+    return malloc_base;
   }
+
+  header = ::new (malloc_base)MallocHeader(size, flags, stack, level);
   memblock = (void*)((char*)malloc_base + sizeof(MallocHeader));
 
   // The alignment check: 8 bytes alignment for 32 bit systems.
   //                      16 bytes alignment for 64-bit systems.
   assert(((size_t)memblock & (sizeof(size_t) * 2 - 1)) == 0, "Alignment check");
 
-  // Sanity check
-  assert(get_memory_tracking_level(memblock) == level,
-    "Wrong tracking level");
-
 #ifdef ASSERT
   if (level > NMT_minimal) {
     // Read back

hotspot/src/share/vm/services/mallocTracker.hpp

@@ -239,68 +239,46 @@ class MallocMemorySummary : AllStatic {
 
 class MallocHeader VALUE_OBJ_CLASS_SPEC {
 #ifdef _LP64
-  size_t           _size      : 62;
-  size_t           _level     : 2;
+  size_t           _size      : 64;
   size_t           _flags     : 8;
   size_t           _pos_idx   : 16;
   size_t           _bucket_idx: 40;
 #define MAX_MALLOCSITE_TABLE_SIZE ((size_t)1 << 40)
 #define MAX_BUCKET_LENGTH         ((size_t)(1 << 16))
-#define MAX_MALLOC_SIZE           (((size_t)1 << 62) - 1)
 #else
-  size_t           _size      : 30;
-  size_t           _level     : 2;
+  size_t           _size      : 32;
   size_t           _flags     : 8;
   size_t           _pos_idx   : 8;
   size_t           _bucket_idx: 16;
 #define MAX_MALLOCSITE_TABLE_SIZE  ((size_t)(1 << 16))
 #define MAX_BUCKET_LENGTH          ((size_t)(1 << 8))
-// Max malloc size = 1GB - 1 on 32 bit system, such has total 4GB memory
-#define MAX_MALLOC_SIZE            ((size_t)(1 << 30) - 1)
 #endif  // _LP64
 
  public:
-  // Summary tracking header
-  MallocHeader(size_t size, MEMFLAGS flags) {
+  MallocHeader(size_t size, MEMFLAGS flags, const NativeCallStack& stack, NMT_TrackingLevel level) {
     assert(sizeof(MallocHeader) == sizeof(void*) * 2,
       "Wrong header size");
 
-    _level = NMT_summary;
-    _flags = flags;
-    set_size(size);
-    MallocMemorySummary::record_malloc(size, flags);
-    MallocMemorySummary::record_new_malloc_header(sizeof(MallocHeader));
-  }
-  // Detail tracking header
-  MallocHeader(size_t size, MEMFLAGS flags, const NativeCallStack& stack) {
-    assert(sizeof(MallocHeader) == sizeof(void*) * 2,
-      "Wrong header size");
-
-    _level = NMT_detail;
-    _flags = flags;
-    set_size(size);
-    size_t bucket_idx;
-    size_t pos_idx;
-    if (record_malloc_site(stack, size, &bucket_idx, &pos_idx)) {
-      assert(bucket_idx <= MAX_MALLOCSITE_TABLE_SIZE, "Overflow bucket index");
-      assert(pos_idx <= MAX_BUCKET_LENGTH, "Overflow bucket position index");
-      _bucket_idx = bucket_idx;
-      _pos_idx = pos_idx;
+    if (level == NMT_minimal) {
+      return;
     }
+
+    _flags = flags;
+    set_size(size);
+    if (level == NMT_detail) {
+      size_t bucket_idx;
+      size_t pos_idx;
+      if (record_malloc_site(stack, size, &bucket_idx, &pos_idx)) {
+        assert(bucket_idx <= MAX_MALLOCSITE_TABLE_SIZE, "Overflow bucket index");
+        assert(pos_idx <= MAX_BUCKET_LENGTH, "Overflow bucket position index");
+        _bucket_idx = bucket_idx;
+        _pos_idx = pos_idx;
+      }
+    }
+
     MallocMemorySummary::record_malloc(size, flags);
     MallocMemorySummary::record_new_malloc_header(sizeof(MallocHeader));
   }
-  // Minimal tracking header
-  MallocHeader() {
-    assert(sizeof(MallocHeader) == sizeof(void*) * 2,
-      "Wrong header size");
-
-    _level = (unsigned short)NMT_minimal;
-  }
-
-  inline NMT_TrackingLevel tracking_level() const {
-    return (NMT_TrackingLevel)_level;
-  }
 
   inline size_t   size()  const { return _size; }
   inline MEMFLAGS flags() const { return (MEMFLAGS)_flags; }
@@ -311,7 +289,6 @@ class MallocHeader VALUE_OBJ_CLASS_SPEC {
 
  private:
   inline void set_size(size_t size) {
-    assert(size <= MAX_MALLOC_SIZE, "Malloc size too large, should use virtual memory?");
     _size = size;
   }
   bool record_malloc_site(const NativeCallStack& stack, size_t size,
@@ -347,10 +324,6 @@ class MallocTracker : AllStatic {
   // Record free on specified memory block
   static void* record_free(void* memblock);
 
-  // Get tracking level of specified memory block
-  static inline NMT_TrackingLevel get_memory_tracking_level(void* memblock);
-
-
   // Offset memory address to header address
   static inline void* get_base(void* memblock);
   static inline void* get_base(void* memblock, NMT_TrackingLevel level) {
@@ -361,16 +334,12 @@ class MallocTracker : AllStatic {
   // Get memory size
   static inline size_t get_size(void* memblock) {
     MallocHeader* header = malloc_header(memblock);
-    assert(header->tracking_level() >= NMT_summary,
-      "Wrong tracking level");
     return header->size();
   }
 
   // Get memory type
   static inline MEMFLAGS get_flags(void* memblock) {
     MallocHeader* header = malloc_header(memblock);
-    assert(header->tracking_level() >= NMT_summary,
-      "Wrong tracking level");
     return header->flags();
   }
 
@@ -394,7 +363,6 @@ class MallocTracker : AllStatic {
   static inline MallocHeader* malloc_header(void *memblock) {
     assert(memblock != NULL, "NULL pointer");
     MallocHeader* header = (MallocHeader*)((char*)memblock - sizeof(MallocHeader));
-    assert(header->tracking_level() >= NMT_minimal, "Bad header");
     return header;
   }
 };

hotspot/src/share/vm/services/mallocTracker.inline.hpp

@@ -28,13 +28,6 @@
 #include "services/mallocTracker.hpp"
 #include "services/memTracker.hpp"
 
-inline NMT_TrackingLevel MallocTracker::get_memory_tracking_level(void* memblock) {
-  assert(memblock != NULL, "Sanity check");
-  if (MemTracker::tracking_level() == NMT_off) return NMT_off;
-  MallocHeader* header = malloc_header(memblock);
-  return header->tracking_level();
-}
-
 inline void* MallocTracker::get_base(void* memblock){
   return get_base(memblock, MemTracker::tracking_level());
 }

hotspot/test/TEST.groups

@@ -87,7 +87,6 @@ needs_jdk = \
   runtime/NMT/SummarySanityCheck.java \
   runtime/NMT/ThreadedMallocTestType.java \
   runtime/NMT/ThreadedVirtualAllocTestType.java \
-  runtime/NMT/UnsafeMallocLimit.java \
   runtime/NMT/VirtualAllocCommitUncommitRecommit.java \
   runtime/NMT/VirtualAllocTestType.java \
   runtime/RedefineObject/TestRedefineObject.java \

hotspot/test/runtime/NMT/UnsafeMallocLimit.java

@@ -1,50 +0,0 @@
-/*
- * Copyright (c) 2014, 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.
- */
-
-/*
- * @test
- * @bug 8055289
- * @library /testlibrary
- * @build UnsafeMallocLimit
- * @run main/othervm -Xmx32m -XX:NativeMemoryTracking=summary UnsafeMallocLimit
- */
-
-import com.oracle.java.testlibrary.*;
-import sun.misc.Unsafe;
-
-public class UnsafeMallocLimit {
-
-    public static void main(String args[]) throws Exception {
-        if (Platform.is32bit()) {
-            Unsafe unsafe = Utils.getUnsafe();
-            try {
-                unsafe.allocateMemory(1 << 30);
-                throw new RuntimeException("Did not get expected OOME");
-            } catch (OutOfMemoryError e) {
-                // Expected exception
-            }
-        } else {
-            System.out.println("Test only valid on 32-bit platforms");
-        }
-    }
-}

hotspot/test/runtime/NMT/UnsafeMallocLimit2.java

@@ -1,54 +0,0 @@
-/*
- * Copyright (c) 2014, 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.
- */
-
-/*
- * @test
- * @bug 8058818
- * @library /testlibrary
- * @build UnsafeMallocLimit2
- * @run main/othervm -Xmx32m -XX:NativeMemoryTracking=off UnsafeMallocLimit2
- */
-
-import com.oracle.java.testlibrary.*;
-import sun.misc.Unsafe;
-
-public class UnsafeMallocLimit2 {
-
-    public static void main(String args[]) throws Exception {
-        if (Platform.is32bit()) {
-            Unsafe unsafe = Utils.getUnsafe();
-            try {
-                // Allocate greater than MALLOC_MAX and likely won't fail to allocate,
-                // so it hits the NMT code that asserted.
-                // Test that this doesn't cause an assertion with NMT off.
-                // The option above overrides if all the tests are run with NMT on.
-                unsafe.allocateMemory(0x40000000);
-                System.out.println("Allocation succeeded");
-            } catch (OutOfMemoryError e) {
-                System.out.println("Allocation failed");
-            }
-        } else {
-            System.out.println("Test only valid on 32-bit platforms");
-        }
-    }
-}