8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale

Reviewed-by: eastigeevich, kbarrett

Author: afshin-zafari

src/hotspot/share/services/memReporter.cpp

@@ -474,9 +474,9 @@ void MemSummaryDiffReporter::print_malloc_diff(size_t current_amount, size_t cur
     out->print(" type=%s", NMTUtil::flag_to_name(flags));
   }
 
-  long amount_diff = diff_in_current_scale(current_amount, early_amount);
+  int64_t amount_diff = diff_in_current_scale(current_amount, early_amount);
   if (amount_diff != 0) {
-    out->print(" %+ld%s", amount_diff, scale);
+    out->print(" " INT64_PLUS_FORMAT "%s", amount_diff, scale);
   }
   if (current_count > 0) {
     out->print(" #" SIZE_FORMAT "", current_count);
@@ -493,7 +493,7 @@ void MemSummaryDiffReporter::print_arena_diff(size_t current_amount, size_t curr
   outputStream* out = output();
   out->print("arena=" SIZE_FORMAT "%s", amount_in_current_scale(current_amount), scale);
   if (diff_in_current_scale(current_amount, early_amount) != 0) {
-    out->print(" %+ld", diff_in_current_scale(current_amount, early_amount));
+    out->print(" " INT64_PLUS_FORMAT "d", diff_in_current_scale(current_amount, early_amount));
   }
 
   out->print(" #" SIZE_FORMAT "", current_count);
@@ -508,15 +508,15 @@ void MemSummaryDiffReporter::print_virtual_memory_diff(size_t current_reserved,
   const char* scale = current_scale();
   outputStream* out = output();
   out->print("reserved=" SIZE_FORMAT "%s", amount_in_current_scale(current_reserved), scale);
-  long reserved_diff = diff_in_current_scale(current_reserved, early_reserved);
+  int64_t reserved_diff = diff_in_current_scale(current_reserved, early_reserved);
   if (reserved_diff != 0) {
-    out->print(" %+ld%s", reserved_diff, scale);
+    out->print(" " INT64_PLUS_FORMAT "%s", reserved_diff, scale);
   }
 
   out->print(", committed=" SIZE_FORMAT "%s", amount_in_current_scale(current_committed), scale);
-  long committed_diff = diff_in_current_scale(current_committed, early_committed);
+  int64_t committed_diff = diff_in_current_scale(current_committed, early_committed);
   if (committed_diff != 0) {
-    out->print(" %+ld%s", committed_diff, scale);
+    out->print(" " INT64_PLUS_FORMAT "%s", committed_diff, scale);
   }
 }
 
@@ -660,10 +660,10 @@ void MemSummaryDiffReporter::diff_summary_of_type(MEMFLAGS flag,
       out->print("%27s (tracking overhead=" SIZE_FORMAT "%s", " ",
         amount_in_current_scale(_current_baseline.malloc_tracking_overhead()), scale);
 
-      long overhead_diff = diff_in_current_scale(_current_baseline.malloc_tracking_overhead(),
-           _early_baseline.malloc_tracking_overhead());
+      int64_t overhead_diff = diff_in_current_scale(_current_baseline.malloc_tracking_overhead(),
+                                                    _early_baseline.malloc_tracking_overhead());
       if (overhead_diff != 0) {
-        out->print(" %+ld%s", overhead_diff, scale);
+        out->print(" " INT64_PLUS_FORMAT "%s", overhead_diff, scale);
       }
       out->print_cr(")");
     } else if (flag == mtClass) {
@@ -695,18 +695,18 @@ void MemSummaryDiffReporter::print_metaspace_diff(const char* header,
                             early_stats.committed());
   out->print_cr(")");
 
-  long diff_used = diff_in_current_scale(current_stats.used(),
-                                         early_stats.used());
+  int64_t diff_used = diff_in_current_scale(current_stats.used(),
+                                            early_stats.used());
 
   size_t current_waste = current_stats.committed() - current_stats.used();
   size_t early_waste = early_stats.committed() - early_stats.used();
-  long diff_waste = diff_in_current_scale(current_waste, early_waste);
+  int64_t diff_waste = diff_in_current_scale(current_waste, early_waste);
 
   // Diff used
   out->print("%27s (    used=" SIZE_FORMAT "%s", " ",
     amount_in_current_scale(current_stats.used()), scale);
   if (diff_used != 0) {
-    out->print(" %+ld%s", diff_used, scale);
+    out->print(" " INT64_PLUS_FORMAT "%s", diff_used, scale);
   }
   out->print_cr(")");
 
@@ -716,7 +716,7 @@ void MemSummaryDiffReporter::print_metaspace_diff(const char* header,
   out->print("%27s (    waste=" SIZE_FORMAT "%s =%2.2f%%", " ",
     amount_in_current_scale(current_waste), scale, waste_percentage);
   if (diff_waste != 0) {
-    out->print(" %+ld%s", diff_waste, scale);
+    out->print(" " INT64_PLUS_FORMAT "%s", diff_waste, scale);
   }
   out->print_cr(")");
 }

src/hotspot/share/services/memReporter.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2023, 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
@@ -71,11 +71,37 @@ class MemReporterBase : public StackObj {
   }
 
   // Convert diff amount in bytes to current reporting scale
-  inline long diff_in_current_scale(size_t s1, size_t s2) const {
-    long amount = (long)(s1 - s2);
-    long scale = (long)_scale;
-    amount = (amount > 0) ? (amount + scale / 2) : (amount - scale / 2);
-    return amount / scale;
+  // We use int64_t instead of ssize_t because on 32-bit it allows us to express deltas larger than 2 gb.
+  // On 64-bit we never expect memory sizes larger than INT64_MAX.
+  int64_t diff_in_current_scale(size_t s1, size_t s2) const {
+    assert(_scale != 0, "wrong scale");
+
+    assert(s1 < INT64_MAX, "exceeded possible memory limits");
+    assert(s2 < INT64_MAX, "exceeded possible memory limits");
+
+    bool is_negative = false;
+    if (s1 < s2) {
+      is_negative = true;
+      swap(s1, s2);
+    }
+
+    size_t amount = s1 - s2;
+    // We can split amount into p + q, where
+    //     q = amount % _scale
+    // and p = amount - q   (which is also (amount / _scale) * _scale).
+    // Then use
+    //   size_t scaled = (p + q + _scale/2) / _scale;
+    // =>
+    //   size_t scaled = (p / _scale) + ((q + _scale/2) / _scale);
+    // The lefthand side of the addition is exact.
+    // The righthand side is 0 if q <= (_scale - 1)/2, else 1. (The -1 is to account for odd _scale values.)
+    size_t scaled = (amount / _scale);
+    if ((amount % _scale) > (_scale - 1)/2) {
+      scaled += 1;
+    }
+
+    int64_t result = static_cast<int64_t>(scaled);
+    return is_negative ? -result : result;
   }
 
   // Print summary total, malloc and virtual memory

src/hotspot/share/utilities/globalDefinitions.hpp

@@ -121,6 +121,7 @@ class oopDesc;
 
 // Format 64-bit quantities.
 #define INT64_FORMAT             "%"          PRId64
+#define INT64_PLUS_FORMAT        "%+"         PRId64
 #define INT64_FORMAT_X           "0x%"        PRIx64
 #define INT64_FORMAT_X_0         "0x%016"     PRIx64
 #define INT64_FORMAT_W(width)    "%"   #width PRId64