8303070: Memory leak in DCmdArgument<char*>::parse_value

Reviewed-by: fparain, jcking, jsjolen, eosterlund, coleenp

Author: David Holmes

src/hotspot/share/services/diagnosticArgument.cpp

@@ -178,33 +178,30 @@ template <> void DCmdArgument<bool>::init_value(TRAPS) {
 
 template <> void DCmdArgument<bool>::destroy_value() { }
 
+template <> void DCmdArgument<char*>::destroy_value() {
+  FREE_C_HEAP_ARRAY(char, _value);
+  set_value(nullptr);
+}
+
 template <> void DCmdArgument<char*>::parse_value(const char* str,
                                                   size_t len, TRAPS) {
   if (str == nullptr) {
-    _value = nullptr;
+    destroy_value();
   } else {
-    _value = NEW_C_HEAP_ARRAY(char, len + 1, mtInternal);
+    // Use realloc as we may have a default set.
+    _value = REALLOC_C_HEAP_ARRAY(char, _value, len + 1, mtInternal);
     int n = os::snprintf(_value, len + 1, "%.*s", (int)len, str);
     assert((size_t)n <= len, "Unexpected number of characters in string");
   }
 }
 
 template <> void DCmdArgument<char*>::init_value(TRAPS) {
-  if (has_default() && _default_string != nullptr) {
+  set_value(nullptr); // Must be initialized before calling parse_value
+  if (has_default()) {
     this->parse_value(_default_string, strlen(_default_string), THREAD);
-    if (HAS_PENDING_EXCEPTION) {
-     fatal("Default string must be parsable");
-    }
-  } else {
-    set_value(nullptr);
   }
 }
 
-template <> void DCmdArgument<char*>::destroy_value() {
-  FREE_C_HEAP_ARRAY(char, _value);
-  set_value(nullptr);
-}
-
 template <> void DCmdArgument<NanoTimeArgument>::parse_value(const char* str,
                                                  size_t len, TRAPS) {
   if (str == nullptr) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 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
@@ -74,5 +74,12 @@ public static void main(String args[]) throws Exception {
     output = new OutputAnalyzer(pb.start());
     output.shouldContain("Incorrect scale value: apa");
 
+    pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), pid, "VM.native_memory", "summary", "scale="});
+    output = new OutputAnalyzer(pb.start());
+    output.shouldContain("Incorrect scale value:");
+
+    pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), pid, "VM.native_memory", "summary", "scale"});
+    output = new OutputAnalyzer(pb.start());
+    output.shouldContain("Incorrect scale value: (null)");
   }
 }