# HG changeset patch
# User ddmitriev
# Date 1440772351 -10800
# Node ID a37aac88925c8d95d98dd2f38d77bc853bf179f8
# Parent  bf890f7af01425e342ad6f6c3244b2ec930b8eec
8132725: Memory leak in Arguments::add_property function
Summary: Logic in add_property was rewritten to avoid memory leak
Reviewed-by: iklam, coleenp

diff -r bf890f7af014 -r a37aac88925c src/share/vm/prims/jvmtiEnv.cpp
--- a/src/share/vm/prims/jvmtiEnv.cpp	Fri Aug 28 11:10:57 2015 +0200
+++ b/src/share/vm/prims/jvmtiEnv.cpp	Fri Aug 28 17:32:31 2015 +0300
@@ -3482,7 +3482,7 @@
 
   for (SystemProperty* p = Arguments::system_properties(); p != NULL; p = p->next()) {
     if (strcmp(property, p->key()) == 0) {
-      if (p->set_value((char *)value_ptr)) {
+      if (p->set_value(value_ptr)) {
         err =  JVMTI_ERROR_NONE;
       }
     }
diff -r bf890f7af014 -r a37aac88925c src/share/vm/runtime/arguments.cpp
--- a/src/share/vm/runtime/arguments.cpp	Fri Aug 28 11:10:57 2015 +0200
+++ b/src/share/vm/runtime/arguments.cpp	Fri Aug 28 17:32:31 2015 +0300
@@ -983,53 +983,61 @@
 
 bool Arguments::add_property(const char* prop) {
   const char* eq = strchr(prop, '=');
-  char* key;
-  // ns must be static--its address may be stored in a SystemProperty object.
-  const static char ns[1] = {0};
-  char* value = (char *)ns;
-
-  size_t key_len = (eq == NULL) ? strlen(prop) : (eq - prop);
-  key = AllocateHeap(key_len + 1, mtInternal);
-  strncpy(key, prop, key_len);
-  key[key_len] = '\0';
-
-  if (eq != NULL) {
-    size_t value_len = strlen(prop) - key_len - 1;
-    value = AllocateHeap(value_len + 1, mtInternal);
-    strncpy(value, &prop[key_len + 1], value_len + 1);
+  const char* key;
+  const char* value = "";
+
+  if (eq == NULL) {
+    // property doesn't have a value, thus use passed string
+    key = prop;
+  } else {
+    // property have a value, thus extract it and save to the
+    // allocated string
+    size_t key_len = eq - prop;
+    char* tmp_key = AllocateHeap(key_len + 1, mtInternal);
+
+    strncpy(tmp_key, prop, key_len);
+    tmp_key[key_len] = '\0';
+    key = tmp_key;
+
+    value = &prop[key_len + 1];
   }
 
   if (strcmp(key, "java.compiler") == 0) {
     process_java_compiler_argument(value);
-    FreeHeap(key);
-    if (eq != NULL) {
-      FreeHeap(value);
-    }
-    return true;
-  } else if (strcmp(key, "sun.java.command") == 0) {
-    _java_command = value;
-
     // Record value in Arguments, but let it get passed to Java.
   } else if (strcmp(key, "sun.java.launcher.is_altjvm") == 0 ||
              strcmp(key, "sun.java.launcher.pid") == 0) {
     // sun.java.launcher.is_altjvm and sun.java.launcher.pid property are
     // private and are processed in process_sun_java_launcher_properties();
     // the sun.java.launcher property is passed on to the java application
-    FreeHeap(key);
-    if (eq != NULL) {
-      FreeHeap(value);
-    }
-    return true;
-  } else if (strcmp(key, "java.vendor.url.bug") == 0) {
-    // save it in _java_vendor_url_bug, so JVM fatal error handler can access
-    // its value without going through the property list or making a Java call.
-    _java_vendor_url_bug = value;
   } else if (strcmp(key, "sun.boot.library.path") == 0) {
     PropertyList_unique_add(&_system_properties, key, value, true);
-    return true;
+  } else {
+    if (strcmp(key, "sun.java.command") == 0) {
+      if (_java_command != NULL) {
+        os::free(_java_command);
+      }
+      _java_command = os::strdup_check_oom(value, mtInternal);
+    } else if (strcmp(key, "java.vendor.url.bug") == 0) {
+      if (_java_vendor_url_bug != DEFAULT_VENDOR_URL_BUG) {
+        assert(_java_vendor_url_bug != NULL, "_java_vendor_url_bug is NULL");
+        os::free((void *)_java_vendor_url_bug);
+      }
+      // save it in _java_vendor_url_bug, so JVM fatal error handler can access
+      // its value without going through the property list or making a Java call.
+      _java_vendor_url_bug = os::strdup_check_oom(value, mtInternal);
+    }
+
+    // Create new property and add at the end of the list
+    PropertyList_unique_add(&_system_properties, key, value);
   }
-  // Create new property and add at the end of the list
-  PropertyList_unique_add(&_system_properties, key, value);
+
+  if (key != prop) {
+    // SystemProperty copy passed value, thus free previously allocated
+    // memory
+    FreeHeap((void *)key);
+  }
+
   return true;
 }
 
@@ -1046,7 +1054,7 @@
   // Ensure Agent_OnLoad has the correct initial values.
   // This may not be the final mode; mode may change later in onload phase.
   PropertyList_unique_add(&_system_properties, "java.vm.info",
-                          (char*)VM_Version::vm_info_string(), false);
+                          VM_Version::vm_info_string(), false);
 
   UseInterpreter             = true;
   UseCompiler                = true;
@@ -1858,7 +1866,7 @@
 }
 
 // Aggressive optimization flags  -XX:+AggressiveOpts
-void Arguments::set_aggressive_opts_flags() {
+jint Arguments::set_aggressive_opts_flags() {
 #ifdef COMPILER2
   if (AggressiveUnboxing) {
     if (FLAG_IS_DEFAULT(EliminateAutoBox)) {
@@ -1885,7 +1893,9 @@
     // Feed the cache size setting into the JDK
     char buffer[1024];
     sprintf(buffer, "java.lang.Integer.IntegerCache.high=" INTX_FORMAT, AutoBoxCacheMax);
-    add_property(buffer);
+    if (!add_property(buffer)) {
+      return JNI_ENOMEM;
+    }
   }
   if (AggressiveOpts && FLAG_IS_DEFAULT(BiasedLockingStartupDelay)) {
     FLAG_SET_DEFAULT(BiasedLockingStartupDelay, 500);
@@ -1898,12 +1908,14 @@
 //      FLAG_SET_DEFAULT(EliminateZeroing, true);
 //    }
   }
+
+  return JNI_OK;
 }
 
 //===========================================================================================================
 // Parsing of java.compiler property
 
-void Arguments::process_java_compiler_argument(char* arg) {
+void Arguments::process_java_compiler_argument(const char* arg) {
   // For backwards compatibility, Djava.compiler=NONE or ""
   // causes us to switch to -Xint mode UNLESS -Xdebug
   // is also specified.
@@ -3870,7 +3882,10 @@
   set_bytecode_flags();
 
   // Set flags if Aggressive optimization flags (-XX:+AggressiveOpts) enabled
-  set_aggressive_opts_flags();
+  jint code = set_aggressive_opts_flags();
+  if (code != JNI_OK) {
+    return code;
+  }
 
   // Turn off biased locking for locking debug mode flags,
   // which are subtly different from each other but neither works with
@@ -4036,7 +4051,7 @@
   }
 }
 
-void Arguments::PropertyList_add(SystemProperty** plist, const char* k, char* v) {
+void Arguments::PropertyList_add(SystemProperty** plist, const char* k, const char* v) {
   if (plist == NULL)
     return;
 
@@ -4049,7 +4064,7 @@
 }
 
 // This add maintains unique property key in the list.
-void Arguments::PropertyList_unique_add(SystemProperty** plist, const char* k, char* v, jboolean append) {
+void Arguments::PropertyList_unique_add(SystemProperty** plist, const char* k, const char* v, jboolean append) {
   if (plist == NULL)
     return;
 
diff -r bf890f7af014 -r a37aac88925c src/share/vm/runtime/arguments.hpp
--- a/src/share/vm/runtime/arguments.hpp	Fri Aug 28 11:10:57 2015 +0200
+++ b/src/share/vm/runtime/arguments.hpp	Fri Aug 28 17:32:31 2015 +0300
@@ -60,7 +60,7 @@
   char* value() const                       { return _value; }
   SystemProperty* next() const              { return _next; }
   void set_next(SystemProperty* next)       { _next = next; }
-  bool set_value(char *value) {
+  bool set_value(const char *value) {
     if (writeable()) {
       if (_value != NULL) {
         FreeHeap(_value);
@@ -364,14 +364,14 @@
   static bool add_property(const char* prop);
 
   // Aggressive optimization flags.
-  static void set_aggressive_opts_flags();
+  static jint set_aggressive_opts_flags();
 
   // Argument parsing
   static void do_pd_flag_adjustments();
   static bool parse_argument(const char* arg, Flag::Flags origin);
   static bool process_argument(const char* arg, jboolean ignore_unrecognized, Flag::Flags origin);
   static void process_java_launcher_argument(const char*, void*);
-  static void process_java_compiler_argument(char* arg);
+  static void process_java_compiler_argument(const char* arg);
   static jint parse_options_environment_variable(const char* name, ScopedVMInitArgs* vm_args);
   static jint parse_java_tool_options_environment_variable(ScopedVMInitArgs* vm_args);
   static jint parse_java_options_environment_variable(ScopedVMInitArgs* vm_args);
@@ -561,22 +561,22 @@
   // Property List manipulation
   static void PropertyList_add(SystemProperty *element);
   static void PropertyList_add(SystemProperty** plist, SystemProperty *element);
-  static void PropertyList_add(SystemProperty** plist, const char* k, char* v);
-  static void PropertyList_unique_add(SystemProperty** plist, const char* k, char* v) {
+  static void PropertyList_add(SystemProperty** plist, const char* k, const char* v);
+  static void PropertyList_unique_add(SystemProperty** plist, const char* k, const char* v) {
     PropertyList_unique_add(plist, k, v, false);
   }
-  static void PropertyList_unique_add(SystemProperty** plist, const char* k, char* v, jboolean append);
+  static void PropertyList_unique_add(SystemProperty** plist, const char* k, const char* v, jboolean append);
   static const char* PropertyList_get_value(SystemProperty* plist, const char* key);
   static int  PropertyList_count(SystemProperty* pl);
   static const char* PropertyList_get_key_at(SystemProperty* pl,int index);
   static char* PropertyList_get_value_at(SystemProperty* pl,int index);
 
   // Miscellaneous System property value getter and setters.
-  static void set_dll_dir(char *value) { _sun_boot_library_path->set_value(value); }
-  static void set_java_home(char *value) { _java_home->set_value(value); }
-  static void set_library_path(char *value) { _java_library_path->set_value(value); }
+  static void set_dll_dir(const char *value) { _sun_boot_library_path->set_value(value); }
+  static void set_java_home(const char *value) { _java_home->set_value(value); }
+  static void set_library_path(const char *value) { _java_library_path->set_value(value); }
   static void set_ext_dirs(char *value)     { _ext_dirs = os::strdup_check_oom(value); }
-  static void set_sysclasspath(char *value) { _sun_boot_class_path->set_value(value); }
+  static void set_sysclasspath(const char *value) { _sun_boot_class_path->set_value(value); }
   static void append_sysclasspath(const char *value) { _sun_boot_class_path->append_value(value); }
 
   static char* get_java_home() { return _java_home->value(); }