8225388: Running jcmd Compiler.CodeHeap_Analytics all 0 cause crash

Reviewed-by: thartmann, sspitsyn

src/hotspot/share/code/codeCache.cpp

@@ -1284,7 +1284,7 @@ void CodeCache::report_codemem_full(int code_blob_type, bool print) {
 
     if (heap->full_count() == 0) {
       if (PrintCodeHeapAnalytics) {
-        CompileBroker::print_heapinfo(tty, "all", "4096"); // details, may be a lot!
+        CompileBroker::print_heapinfo(tty, "all", 4096); // details, may be a lot!
       }
     }
   }
@@ -1571,7 +1571,7 @@ void CodeCache::log_state(outputStream* st) {
 
 //---<  BEGIN  >--- CodeHeap State Analytics.
 
-void CodeCache::aggregate(outputStream *out, const char* granularity) {
+void CodeCache::aggregate(outputStream *out, size_t granularity) {
   FOR_ALL_ALLOCABLE_HEAPS(heap) {
     CodeHeapState::aggregate(out, (*heap), granularity);
   }

src/hotspot/share/code/codeCache.hpp

@@ -294,7 +294,7 @@ class CodeCache : AllStatic {
 
   // CodeHeap State Analytics.
   // interface methods for CodeHeap printing, called by CompileBroker
-  static void aggregate(outputStream *out, const char* granularity);
+  static void aggregate(outputStream *out, size_t granularity);
   static void discard(outputStream *out);
   static void print_usedSpace(outputStream *out);
   static void print_freeSpace(outputStream *out);

src/hotspot/share/code/codeHeapState.cpp

@@ -530,7 +530,7 @@ void CodeHeapState::discard(outputStream* out, CodeHeap* heap) {
   }
 }
 
-void CodeHeapState::aggregate(outputStream* out, CodeHeap* heap, const char* granularity_request) {
+void CodeHeapState::aggregate(outputStream* out, CodeHeap* heap, size_t granularity) {
   unsigned int nBlocks_free    = 0;
   unsigned int nBlocks_used    = 0;
   unsigned int nBlocks_zomb    = 0;
@@ -612,7 +612,8 @@ void CodeHeapState::aggregate(outputStream* out, CodeHeap* heap, const char* gra
   //   Finally, we adjust the granularity such that each granule covers at most 64k-1 segments.
   //   This is necessary to prevent an unsigned short overflow while accumulating space information.
   //
-  size_t granularity = strtol(granularity_request, NULL, 0);
+  assert(granularity > 0, "granularity should be positive.");
+
   if (granularity > size) {
     granularity = size;
   }

src/hotspot/share/code/codeHeapState.hpp

@@ -99,7 +99,7 @@ class CodeHeapState : public CHeapObj<mtCode> {
 
  public:
   static void discard(outputStream* out, CodeHeap* heap);
-  static void aggregate(outputStream* out, CodeHeap* heap, const char* granularity);
+  static void aggregate(outputStream* out, CodeHeap* heap, size_t granularity);
   static void print_usedSpace(outputStream* out, CodeHeap* heap);
   static void print_freeSpace(outputStream* out, CodeHeap* heap);
   static void print_count(outputStream* out, CodeHeap* heap);

src/hotspot/share/compiler/compileBroker.cpp

@@ -2640,7 +2640,7 @@ void CompileBroker::print_info(outputStream *out) {
 //       That's a tradeoff which keeps together important blocks of output.
 //       At the same time, continuous tty_lock hold time is kept in check,
 //       preventing concurrently printing threads from stalling a long time.
-void CompileBroker::print_heapinfo(outputStream* out, const char* function, const char* granularity) {
+void CompileBroker::print_heapinfo(outputStream* out, const char* function, size_t granularity) {
   TimeStamp ts_total;
   TimeStamp ts_global;
   TimeStamp ts;

src/hotspot/share/compiler/compileBroker.hpp

@@ -417,7 +417,7 @@ public:
 
   // CodeHeap State Analytics.
   static void print_info(outputStream *out);
-  static void print_heapinfo(outputStream *out, const char* function, const char* granularity );
+  static void print_heapinfo(outputStream *out, const char* function, size_t granularity);
 };
 
 #endif // SHARE_COMPILER_COMPILEBROKER_HPP

src/hotspot/share/runtime/java.cpp

@@ -310,7 +310,7 @@ void print_statistics() {
   // CodeHeap State Analytics.
   // Does also call NMethodSweeper::print(tty)
   if (PrintCodeHeapAnalytics) {
-    CompileBroker::print_heapinfo(NULL, "all", "4096"); // details
+    CompileBroker::print_heapinfo(NULL, "all", 4096); // details
   } else if (PrintMethodFlushingStatistics) {
     NMethodSweeper::print(tty);
   }
@@ -378,7 +378,7 @@ void print_statistics() {
   // CodeHeap State Analytics.
   // Does also call NMethodSweeper::print(tty)
   if (PrintCodeHeapAnalytics) {
-    CompileBroker::print_heapinfo(NULL, "all", "4096"); // details
+    CompileBroker::print_heapinfo(NULL, "all", 4096); // details
   } else if (PrintMethodFlushingStatistics) {
     NMethodSweeper::print(tty);
   }

src/hotspot/share/services/diagnosticCommand.cpp

@@ -946,13 +946,20 @@ void CodeCacheDCmd::execute(DCmdSource source, TRAPS) {
 CodeHeapAnalyticsDCmd::CodeHeapAnalyticsDCmd(outputStream* output, bool heap) :
                                              DCmdWithParser(output, heap),
   _function("function", "Function to be performed (aggregate, UsedSpace, FreeSpace, MethodCount, MethodSpace, MethodAge, MethodNames, discard", "STRING", false, "all"),
-  _granularity("granularity", "Detail level - smaller value -> more detail", "STRING", false, "4096") {
+  _granularity("granularity", "Detail level - smaller value -> more detail", "INT", false, "4096") {
   _dcmdparser.add_dcmd_argument(&_function);
   _dcmdparser.add_dcmd_argument(&_granularity);
 }
 
 void CodeHeapAnalyticsDCmd::execute(DCmdSource source, TRAPS) {
-  CompileBroker::print_heapinfo(output(), _function.value(), _granularity.value());
+  jlong granularity = _granularity.value();
+  if (granularity < 1) {
+    Exceptions::fthrow(THREAD_AND_LOCATION, vmSymbols::java_lang_IllegalArgumentException(),
+                       "Invalid granularity value " JLONG_FORMAT  ". Should be positive.\n", granularity);
+    return;
+  }
+
+  CompileBroker::print_heapinfo(output(), _function.value(), granularity);
 }
 
 int CodeHeapAnalyticsDCmd::num_arguments() {

src/hotspot/share/services/diagnosticCommand.hpp

@@ -645,7 +645,7 @@ public:
 class CodeHeapAnalyticsDCmd : public DCmdWithParser {
 protected:
   DCmdArgument<char*> _function;
-  DCmdArgument<char*> _granularity;
+  DCmdArgument<jlong> _granularity;
 public:
   CodeHeapAnalyticsDCmd(outputStream* output, bool heap);
   static const char* name() {

test/hotspot/jtreg/serviceability/dcmd/compiler/CodeHeapAnalyticsParams.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2019, 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.
+ */
+
+import jdk.test.lib.dcmd.PidJcmdExecutor;
+
+/*
+ * @test CodeHeapAnalyticsParams
+ * @key jcmd
+ * @summary Test the Compiler.CodeHeap_Analytics command
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          java.management
+ * @run driver CodeHeapAnalyticsParams
+ */
+
+public class CodeHeapAnalyticsParams {
+
+    public static void main(String args[]) throws Exception {
+        PidJcmdExecutor executor = new PidJcmdExecutor();
+        executor.execute("Compiler.CodeHeap_Analytics all 1").shouldHaveExitValue(0);
+        executor.execute("Compiler.CodeHeap_Analytics all 0").shouldHaveExitValue(1);
+        executor.execute("Compiler.CodeHeap_Analytics all k").shouldHaveExitValue(1);
+    }
+}