8135198: Add -XX:VMOptionsFile support to JAVA_TOOL_OPTIONS and _JAVA_OPTIONS
authorrdurbin
Fri, 08 Jan 2016 15:38:08 -0800
changeset 35466 4ace9ef0201b
parent 35464 35f4f3d2812b
child 35467 807dbe98f47f
8135198: Add -XX:VMOptionsFile support to JAVA_TOOL_OPTIONS and _JAVA_OPTIONS Reviewed-by: dcubed, ddmitriev, ahgross, gthornbr, coleenp
hotspot/src/share/vm/runtime/arguments.cpp
hotspot/src/share/vm/runtime/arguments.hpp
hotspot/test/runtime/CommandLine/VMOptionsFile/TestVMOptionsFile.java
--- a/hotspot/src/share/vm/runtime/arguments.cpp	Fri Jan 08 12:56:16 2016 +0000
+++ b/hotspot/src/share/vm/runtime/arguments.cpp	Fri Jan 08 15:38:08 2016 -0800
@@ -2622,7 +2622,7 @@
                                        SysClassPath* scp_p,
                                        bool* scp_assembly_required_p,
                                        Flag::Flags origin) {
-  // Remaining part of option string
+  // For match_option to return remaining or value part of option string
   const char* tail;
 
   // iterate over arguments
@@ -3502,15 +3502,19 @@
 class ScopedVMInitArgs : public StackObj {
  private:
   JavaVMInitArgs _args;
+  char*          _container_name;
   bool           _is_set;
+  char*          _vm_options_file_arg;
 
  public:
-  ScopedVMInitArgs() {
+  ScopedVMInitArgs(const char *container_name) {
     _args.version = JNI_VERSION_1_2;
     _args.nOptions = 0;
     _args.options = NULL;
     _args.ignoreUnrecognized = false;
+    _container_name = (char *)container_name;
     _is_set = false;
+    _vm_options_file_arg = NULL;
   }
 
   // Populates the JavaVMInitArgs object represented by this
@@ -3542,10 +3546,23 @@
     return JNI_OK;
   }
 
-  JavaVMInitArgs* get() { return &_args; }
-  bool is_set()         { return _is_set; }
+  JavaVMInitArgs* get()             { return &_args; }
+  char* container_name()            { return _container_name; }
+  bool  is_set()                    { return _is_set; }
+  bool  found_vm_options_file_arg() { return _vm_options_file_arg != NULL; }
+  char* vm_options_file_arg()       { return _vm_options_file_arg; }
+
+  void set_vm_options_file_arg(const char *vm_options_file_arg) {
+    if (_vm_options_file_arg != NULL) {
+      os::free(_vm_options_file_arg);
+    }
+    _vm_options_file_arg = os::strdup_check_oom(vm_options_file_arg);
+  }
 
   ~ScopedVMInitArgs() {
+    if (_vm_options_file_arg != NULL) {
+      os::free(_vm_options_file_arg);
+    }
     if (_args.options == NULL) return;
     for (int i = 0; i < _args.nOptions; i++) {
       os::free(_args.options[i].optionString);
@@ -3826,12 +3843,23 @@
 
 #endif // PRODUCT
 
+bool Arguments::args_contains_vm_options_file_arg(const JavaVMInitArgs* args) {
+  for (int index = 0; index < args->nOptions; index++) {
+    const JavaVMOption* option = args->options + index;
+    const char* tail;
+    if (match_option(option, "-XX:VMOptionsFile=", &tail)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 jint Arguments::insert_vm_options_file(const JavaVMInitArgs* args,
-                                       char** vm_options_file,
+                                       const char* vm_options_file,
                                        const int vm_options_file_pos,
-                                       ScopedVMInitArgs *vm_options_file_args,
+                                       ScopedVMInitArgs* vm_options_file_args,
                                        ScopedVMInitArgs* args_out) {
-  jint code = parse_vm_options_file(*vm_options_file, vm_options_file_args);
+  jint code = parse_vm_options_file(vm_options_file, vm_options_file_args);
   if (code != JNI_OK) {
     return code;
   }
@@ -3840,17 +3868,46 @@
     return JNI_OK;
   }
 
+  if (args_contains_vm_options_file_arg(vm_options_file_args->get())) {
+    jio_fprintf(defaultStream::error_stream(),
+                "A VM options file may not refer to a VM options file. "
+                "Specification of '-XX:VMOptionsFile=<file-name>' in the "
+                "options file '%s' in options container '%s' is an error.\n",
+                vm_options_file_args->vm_options_file_arg(),
+                vm_options_file_args->container_name());
+    return JNI_EINVAL;
+  }
+
   return args_out->insert(args, vm_options_file_args->get(),
                           vm_options_file_pos);
 }
 
+// Expand -XX:VMOptionsFile found in args_in as needed.
+// mod_args and args_out parameters may return values as needed.
+jint Arguments::expand_vm_options_as_needed(const JavaVMInitArgs* args_in,
+                                            ScopedVMInitArgs* mod_args,
+                                            JavaVMInitArgs** args_out) {
+  jint code = match_special_option_and_act(args_in, mod_args);
+  if (code != JNI_OK) {
+    return code;
+  }
+
+  if (mod_args->is_set()) {
+    // args_in contains -XX:VMOptionsFile and mod_args contains the
+    // original options from args_in along with the options expanded
+    // from the VMOptionsFile. Return a short-hand to the caller.
+    *args_out = mod_args->get();
+  } else {
+    *args_out = (JavaVMInitArgs *)args_in;  // no changes so use args_in
+  }
+  return JNI_OK;
+}
+
 jint Arguments::match_special_option_and_act(const JavaVMInitArgs* args,
-                                             char ** vm_options_file,
                                              ScopedVMInitArgs* args_out) {
   // Remaining part of option string
   const char* tail;
-  int   vm_options_file_pos = -1;
-  ScopedVMInitArgs vm_options_file_args;
+  ScopedVMInitArgs vm_options_file_args(args_out->container_name());
 
   for (int index = 0; index < args->nOptions; index++) {
     const JavaVMOption* option = args->options + index;
@@ -3862,39 +3919,34 @@
       continue;
     }
     if (match_option(option, "-XX:VMOptionsFile=", &tail)) {
-      if (vm_options_file != NULL) {
-        // The caller accepts -XX:VMOptionsFile
-        if (*vm_options_file != NULL) {
-          jio_fprintf(defaultStream::error_stream(),
-                      "The VM Options file can only be specified once and "
-                      "only on the command line.\n");
-          return JNI_EINVAL;
-        }
-
-        *vm_options_file = (char *) tail;
-        vm_options_file_pos = index;  // save position of -XX:VMOptionsFile
-        // If there's a VMOptionsFile, parse that (also can set flags_file)
-        jint code = insert_vm_options_file(args, vm_options_file,
-                                           vm_options_file_pos,
-                                           &vm_options_file_args, args_out);
-        if (code != JNI_OK) {
-          return code;
-        }
-        if (args_out->is_set()) {
-          // The VMOptions file inserted some options so switch 'args'
-          // to the new set of options, and continue processing which
-          // preserves "last option wins" semantics.
-          args = args_out->get();
-          // The first option from the VMOptionsFile replaces the
-          // current option.  So we back track to process the
-          // replacement option.
-          index--;
-        }
-      } else {
+      if (vm_options_file_args.found_vm_options_file_arg()) {
         jio_fprintf(defaultStream::error_stream(),
-                    "VM options file is only supported on the command line\n");
+                    "The option '%s' is already specified in the options "
+                    "container '%s' so the specification of '%s' in the "
+                    "same options container is an error.\n",
+                    vm_options_file_args.vm_options_file_arg(),
+                    vm_options_file_args.container_name(),
+                    option->optionString);
         return JNI_EINVAL;
       }
+      vm_options_file_args.set_vm_options_file_arg(option->optionString);
+      // If there's a VMOptionsFile, parse that
+      jint code = insert_vm_options_file(args, tail, index,
+                                         &vm_options_file_args, args_out);
+      if (code != JNI_OK) {
+        return code;
+      }
+      args_out->set_vm_options_file_arg(vm_options_file_args.vm_options_file_arg());
+      if (args_out->is_set()) {
+        // The VMOptions file inserted some options so switch 'args'
+        // to the new set of options, and continue processing which
+        // preserves "last option wins" semantics.
+        args = args_out->get();
+        // The first option from the VMOptionsFile replaces the
+        // current option.  So we back track to process the
+        // replacement option.
+        index--;
+      }
       continue;
     }
     if (match_option(option, "-XX:+PrintVMOptions")) {
@@ -3963,7 +4015,7 @@
 
 // Parse entry point called from JNI_CreateJavaVM
 
-jint Arguments::parse(const JavaVMInitArgs* args) {
+jint Arguments::parse(const JavaVMInitArgs* initial_cmd_args) {
   assert(verify_special_jvm_flags(), "deprecated and obsolete flag table inconsistent");
 
   // Initialize ranges and constraints
@@ -3972,67 +4024,74 @@
 
   // If flag "-XX:Flags=flags-file" is used it will be the first option to be processed.
   const char* hotspotrc = ".hotspotrc";
-  char* vm_options_file = NULL;
   bool settings_file_specified = false;
   bool needs_hotspotrc_warning = false;
-  ScopedVMInitArgs java_tool_options_args;
-  ScopedVMInitArgs java_options_args;
-  ScopedVMInitArgs modified_cmd_line_args;
+  ScopedVMInitArgs initial_java_tool_options_args("env_var='JAVA_TOOL_OPTIONS'");
+  ScopedVMInitArgs initial_java_options_args("env_var='_JAVA_OPTIONS'");
+
+  // Pointers to current working set of containers
+  JavaVMInitArgs* cur_cmd_args;
+  JavaVMInitArgs* cur_java_options_args;
+  JavaVMInitArgs* cur_java_tool_options_args;
+
+  // Containers for modified/expanded options
+  ScopedVMInitArgs mod_cmd_args("cmd_line_args");
+  ScopedVMInitArgs mod_java_tool_options_args("env_var='JAVA_TOOL_OPTIONS'");
+  ScopedVMInitArgs mod_java_options_args("env_var='_JAVA_OPTIONS'");
+
 
   jint code =
-      parse_java_tool_options_environment_variable(&java_tool_options_args);
+      parse_java_tool_options_environment_variable(&initial_java_tool_options_args);
   if (code != JNI_OK) {
     return code;
   }
 
-  code = parse_java_options_environment_variable(&java_options_args);
-  if (code != JNI_OK) {
-    return code;
-  }
-
-  code = match_special_option_and_act(java_tool_options_args.get(),
-                                      NULL, NULL);
+  code = parse_java_options_environment_variable(&initial_java_options_args);
   if (code != JNI_OK) {
     return code;
   }
 
-  code = match_special_option_and_act(args, &vm_options_file,
-                                      &modified_cmd_line_args);
+  code = expand_vm_options_as_needed(initial_java_tool_options_args.get(),
+                                     &mod_java_tool_options_args,
+                                     &cur_java_tool_options_args);
   if (code != JNI_OK) {
     return code;
   }
 
-
-  // The command line arguments have been modified to include VMOptionsFile arguments.
-  if (modified_cmd_line_args.is_set()) {
-    args = modified_cmd_line_args.get();
-  }
-
-  code = match_special_option_and_act(java_options_args.get(),
-                                      NULL, NULL);
+  code = expand_vm_options_as_needed(initial_cmd_args,
+                                     &mod_cmd_args,
+                                     &cur_cmd_args);
   if (code != JNI_OK) {
     return code;
   }
 
-  const char * flags_file = Arguments::get_jvm_flags_file();
+  code = expand_vm_options_as_needed(initial_java_options_args.get(),
+                                     &mod_java_options_args,
+                                     &cur_java_options_args);
+  if (code != JNI_OK) {
+    return code;
+  }
+
+  const char* flags_file = Arguments::get_jvm_flags_file();
   settings_file_specified = (flags_file != NULL);
 
   if (IgnoreUnrecognizedVMOptions) {
-    // uncast const to modify the flag args->ignoreUnrecognized
-    *(jboolean*)(&args->ignoreUnrecognized) = true;
-    java_tool_options_args.get()->ignoreUnrecognized = true;
-    java_options_args.get()->ignoreUnrecognized = true;
+    cur_cmd_args->ignoreUnrecognized = true;
+    cur_java_tool_options_args->ignoreUnrecognized = true;
+    cur_java_options_args->ignoreUnrecognized = true;
   }
 
   // Parse specified settings file
   if (settings_file_specified) {
-    if (!process_settings_file(flags_file, true, args->ignoreUnrecognized)) {
+    if (!process_settings_file(flags_file, true,
+                               cur_cmd_args->ignoreUnrecognized)) {
       return JNI_EINVAL;
     }
   } else {
 #ifdef ASSERT
     // Parse default .hotspotrc settings file
-    if (!process_settings_file(".hotspotrc", false, args->ignoreUnrecognized)) {
+    if (!process_settings_file(".hotspotrc", false,
+                               cur_cmd_args->ignoreUnrecognized)) {
       return JNI_EINVAL;
     }
 #else
@@ -4044,15 +4103,15 @@
   }
 
   if (PrintVMOptions) {
-    print_options(java_tool_options_args.get());
-    print_options(args);
-    print_options(java_options_args.get());
+    print_options(cur_java_tool_options_args);
+    print_options(cur_cmd_args);
+    print_options(cur_java_options_args);
   }
 
   // Parse JavaVMInitArgs structure passed in, as well as JAVA_TOOL_OPTIONS and _JAVA_OPTIONS
-  jint result = parse_vm_init_args(java_tool_options_args.get(),
-                                   java_options_args.get(),
-                                   args);   // command line arguments
+  jint result = parse_vm_init_args(cur_java_tool_options_args,
+                                   cur_java_options_args,
+                                   cur_cmd_args);
 
   if (result != JNI_OK) {
     return result;
--- a/hotspot/src/share/vm/runtime/arguments.hpp	Fri Jan 08 12:56:16 2016 +0000
+++ b/hotspot/src/share/vm/runtime/arguments.hpp	Fri Jan 08 15:38:08 2016 -0800
@@ -379,12 +379,15 @@
   static jint parse_vm_options_file(const char* file_name, ScopedVMInitArgs* vm_args);
   static jint parse_options_buffer(const char* name, char* buffer, const size_t buf_len, ScopedVMInitArgs* vm_args);
   static jint insert_vm_options_file(const JavaVMInitArgs* args,
-                                     char** vm_options_file,
+                                     const char* vm_options_file,
                                      const int vm_options_file_pos,
                                      ScopedVMInitArgs* vm_options_file_args,
                                      ScopedVMInitArgs* args_out);
+  static bool args_contains_vm_options_file_arg(const JavaVMInitArgs* args);
+  static jint expand_vm_options_as_needed(const JavaVMInitArgs* args_in,
+                                          ScopedVMInitArgs* mod_args,
+                                          JavaVMInitArgs** args_out);
   static jint match_special_option_and_act(const JavaVMInitArgs* args,
-                                           char** vm_options_file,
                                            ScopedVMInitArgs* args_out);
 
   static jint parse_vm_init_args(const JavaVMInitArgs *java_tool_options_args,
--- a/hotspot/test/runtime/CommandLine/VMOptionsFile/TestVMOptionsFile.java	Fri Jan 08 12:56:16 2016 +0000
+++ b/hotspot/test/runtime/CommandLine/VMOptionsFile/TestVMOptionsFile.java	Fri Jan 08 15:38:08 2016 -0800
@@ -285,6 +285,8 @@
     }
 
     private static void testVMOptions() throws Exception {
+        ProcessBuilder pb;
+
         /* Check that empty VM Option file is accepted without errors */
         addVMOptionsFile(VM_OPTION_FILE_EMPTY);
 
@@ -385,6 +387,33 @@
         checkProperty("my.property", "value" + REPEAT_COUNT);
         checkVMOption("MinHeapFreeRatio", "17");
         checkVMOption("MaxHeapFreeRatio", "85");
+
+        /* Pass VM Option file in _JAVA_OPTIONS environment variable */
+        addVMParam("-showversion");
+        addVMOptionsToCheck("SurvivorRatio", "MinHeapFreeRatio");
+        pb = createProcessBuilder();
+
+        updateEnvironment(pb, JAVA_OPTIONS, "-XX:VMOptionsFile=" + getAbsolutePathFromSource(VM_OPTION_FILE_1));
+
+        runJavaCheckExitValue(pb, JVM_SUCCESS);
+        outputShouldContain("interpreted mode");
+        checkProperty("optfile_1", "option_file_1");
+        checkVMOption("SurvivorRatio", "16");
+        checkVMOption("MinHeapFreeRatio", "22");
+
+        /* Pass VM Option file in JAVA_TOOL_OPTIONS environment variable */
+        addVMOptionsToCheck("UseGCOverheadLimit", "NewRatio", "MinHeapFreeRatio", "MaxFDLimit", "AlwaysPreTouch");
+        pb = createProcessBuilder();
+
+        updateEnvironment(pb, JAVA_TOOL_OPTIONS, "-XX:VMOptionsFile=" + VM_OPTION_FILE_2);
+
+        runJavaCheckExitValue(pb, JVM_SUCCESS);
+        checkProperty("javax.net.ssl.keyStorePassword", "someVALUE123+");
+        checkVMOption("UseGCOverheadLimit", "true");
+        checkVMOption("NewRatio", "4");
+        checkVMOption("MinHeapFreeRatio", "3");
+        checkVMOption("MaxFDLimit", "true");
+        checkVMOption("AlwaysPreTouch", "false");
     }
 
     private static ProcessBuilder prepareTestCase(int testCase) throws Exception {
@@ -548,13 +577,13 @@
         addVMOptionsFile(VM_OPTION_FILE_WITH_SAME_VM_OPTION_FILE);
 
         runJavaCheckExitValue(JVM_FAIL_WITH_EXIT_CODE_1);
-        outputShouldContain("The VM Options file can only be specified once and only on the command line.");
+        outputShouldContain("A VM options file may not refer to a VM options file. Specification of '-XX:VMOptionsFile=<file-name>' in the options file");
 
         /* Pass VM option file with VM option file option in it */
         addVMOptionsFile(VM_OPTION_FILE_WITH_VM_OPTION_FILE);
 
         runJavaCheckExitValue(JVM_FAIL_WITH_EXIT_CODE_1);
-        outputShouldContain("The VM Options file can only be specified once and only on the command line.");
+        outputShouldContain("A VM options file may not refer to a VM options file. Specification of '-XX:VMOptionsFile=<file-name>' in the options file");
 
         /* Pass VM option file which is not accessible (without read permissions) */
         addVMOptionsFile(getAbsolutePathFromSource(VM_OPTION_FILE_WITHOUT_READ_PERMISSIONS));
@@ -567,7 +596,7 @@
         addVMOptionsFile(VM_OPTION_FILE_2);
 
         runJavaCheckExitValue(JVM_FAIL_WITH_EXIT_CODE_1);
-        outputShouldContain("The VM Options file can only be specified once and only on the command line.");
+        outputShouldContain("is already specified in the");
 
         /* Pass empty option file i.e. pass "-XX:VMOptionsFile=" */
         addVMOptionsFile("");
@@ -586,22 +615,6 @@
 
         runJavaCheckExitValue(JVM_FAIL_WITH_EXIT_CODE_1);
         outputShouldContain("Unmatched quote in");
-
-        /* Pass VM Option file in _JAVA_OPTIONS environment variable */
-        pb = createProcessBuilder();
-
-        updateEnvironment(pb, JAVA_OPTIONS, "-XX:VMOptionsFile=" + getAbsolutePathFromSource(VM_OPTION_FILE_1));
-
-        runJavaCheckExitValue(pb, JVM_FAIL_WITH_EXIT_CODE_1);
-        outputShouldContain("VM options file is only supported on the command line");
-
-        /* Pass VM Option file in JAVA_TOOL_OPTIONS environment variable */
-        pb = createProcessBuilder();
-
-        updateEnvironment(pb, JAVA_TOOL_OPTIONS, "-XX:VMOptionsFile=" + getAbsolutePathFromSource(VM_OPTION_FILE_1));
-
-        runJavaCheckExitValue(pb, JVM_FAIL_WITH_EXIT_CODE_1);
-        outputShouldContain("VM options file is only supported on the command line");
     }
 
     public static void main(String[] args) throws Exception {