8234076: JVM crashes on Windows 10 using --module=NAME

Reviewed-by: ksrini, henryjen

src/java.base/share/native/libjli/args.c

@@ -130,6 +130,8 @@ static void checkArg(const char *arg) {
             }
         } else if (JLI_StrCmp(arg, "--disable-@files") == 0) {
             stopExpansion = JNI_TRUE;
+        } else if (JLI_StrCCmp(arg, "--module=") == 0) {
+            idx = argsCount;
         }
     } else {
         if (!expectingNoDashArg) {
@@ -449,6 +451,7 @@ int isTerminalOpt(char *arg) {
     return JLI_StrCmp(arg, "-jar") == 0 ||
            JLI_StrCmp(arg, "-m") == 0 ||
            JLI_StrCmp(arg, "--module") == 0 ||
+           JLI_StrCCmp(arg, "--module=") == 0 ||
            JLI_StrCmp(arg, "--dry-run") == 0 ||
            JLI_StrCmp(arg, "-h") == 0 ||
            JLI_StrCmp(arg, "-?") == 0 ||

src/java.base/windows/native/libjli/java_md.c

@@ -34,6 +34,7 @@
 #include <sys/stat.h>
 #include <wtypes.h>
 #include <commctrl.h>
+#include <assert.h>
 
 #include <jni.h>
 #include "java.h"
@@ -1008,6 +1009,17 @@ CreateApplicationArgs(JNIEnv *env, char **strv, int argc)
 
     // sanity check, match the args we have, to the holy grail
     idx = JLI_GetAppArgIndex();
+
+    // First arg index is NOT_FOUND
+    if (idx < 0) {
+        // The only allowed value should be NOT_FOUND (-1) unless another change introduces
+        // a different negative index
+        assert (idx == -1);
+        JLI_TraceLauncher("Warning: first app arg index not found, %d\n", idx);
+        JLI_TraceLauncher("passing arguments as-is.\n");
+        return NewPlatformStringArray(env, strv, argc);
+    }
+
     isTool = (idx == 0);
     if (isTool) { idx++; } // skip tool name
     JLI_TraceLauncher("AppArgIndex: %d points to %s\n", idx, stdargs[idx].arg);

test/jdk/tools/launcher/ArgsEnvVar.java

@@ -37,6 +37,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.regex.Pattern;
+import java.nio.file.Paths;
+import java.nio.file.Path;
 
 public class ArgsEnvVar extends TestHelper {
     private static File testJar = null;
@@ -153,6 +155,7 @@ public void noTerminalOpt() throws IOException {
                 List.of("-jar", "test.jar"),
                 List.of("-m", "test/Foo"),
                 List.of("--module", "test/Foo"),
+                List.of("--module=test/Foo"),
                 List.of("--dry-run"),
                 List.of("-h"),
                 List.of("-?"),
@@ -241,6 +244,69 @@ public void testTrailingSpaces() {
         verifyOptions(List.of("--add-exports", "java.base/jdk.internal.misc=ALL-UNNAMED", "-jar", "test.jar"), tr);
     }
 
+
+    @Test
+    // That that we can correctly handle the module longform argument option
+    // when supplied in an argument file
+    public void modulesInArgsFile() throws IOException {
+        File cwd = new File(".");
+        File testModuleDir = new File(cwd, "modules_test");
+
+        createEchoArgumentsModule(testModuleDir);
+
+        Path SRC_DIR = Paths.get(testModuleDir.getAbsolutePath(), "src");
+        Path MODS_DIR = Paths.get(testModuleDir.getAbsolutePath(), "mods");
+
+        // test module / main class
+        String MODULE_OPTION = "--module=test/launcher.Main";
+        String TEST_MODULE = "test";
+
+        // javac -d mods/test src/test/**
+        TestResult tr = doExec(
+            javacCmd,
+            "-d", MODS_DIR.toString(),
+            "--module-source-path", SRC_DIR.toString(),
+            "--module", TEST_MODULE);
+
+        if (!tr.isOK()) {
+            System.out.println("test did not compile");
+            throw new RuntimeException("Error: modules test did not compile");
+        }
+
+        // verify the terminating ability of --module= through environment variables
+        File argFile = createArgFile("cmdargs", List.of("--module-path", MODS_DIR.toString(), MODULE_OPTION, "--hello"));
+        env.put(JDK_JAVA_OPTIONS, "@cmdargs");
+        tr = doExec(env, javaCmd);
+        tr.checkNegative();
+        tr.contains("Error: Option " + MODULE_OPTION + " in @cmdargs is not allowed in environment variable JDK_JAVA_OPTIONS");
+        if (!tr.testStatus) {
+            System.out.println(tr);
+            throw new RuntimeException("test fails");
+        }
+
+        // check that specifying --module and --module-path with file works
+        tr = doExec(javaCmd, "-Dfile.encoding=UTF-8", "@cmdargs");
+        tr.contains("[--hello]");
+        if (!tr.testStatus) {
+            System.out.println(tr);
+            throw new RuntimeException("test fails");
+        }
+
+        // check with reversed --module-path and --module in the arguments file, this will fail, --module= is terminating
+        File argFile1 = createArgFile("cmdargs1", List.of(MODULE_OPTION, "--module-path", MODS_DIR.toString(), "--hello"));
+        tr = doExec(javaCmd, "-Dfile.encoding=UTF-8", "@cmdargs1");
+        tr.checkNegative();
+        if (!tr.testStatus) {
+            System.out.println(tr);
+            throw new RuntimeException("test fails");
+        }
+
+        // clean-up
+        argFile.delete();
+        argFile1.delete();
+        recursiveDelete(testModuleDir);
+    }
+
     public static void main(String... args) throws Exception {
         init();
         ArgsEnvVar a = new ArgsEnvVar();

test/jdk/tools/launcher/TestHelper.java

@@ -500,6 +500,40 @@ static boolean isEnglishLocale() {
         return Locale.getDefault().getLanguage().equals("en");
     }
 
+    /**
+     * Helper method to initialize a simple module that just prints the passed in arguments
+     */
+    static void createEchoArgumentsModule(File modulesDir) throws IOException {
+        if (modulesDir.exists()) {
+            recursiveDelete(modulesDir);
+        }
+
+        modulesDir.mkdirs();
+
+        File srcDir = new File(modulesDir, "src");
+        File modsDir = new File(modulesDir, "mods");
+        File testDir = new File(srcDir, "test");
+        File launcherTestDir = new File(testDir, "launcher");
+        srcDir.mkdir();
+        modsDir.mkdir();
+        testDir.mkdir();
+        launcherTestDir.mkdir();
+
+        String[] moduleInfoCode = { "module test {}" };
+        createFile(new File(testDir, "module-info.java"), Arrays.asList(moduleInfoCode));
+
+        String[] moduleCode = {
+            "package launcher;",
+            "import java.util.Arrays;",
+            "public class Main {",
+            "public static void main(String[] args) {",
+            "System.out.println(Arrays.toString(args));",
+            "}",
+            "}"
+        };
+        createFile(new File(launcherTestDir, "Main.java"), Arrays.asList(moduleCode));
+    }
+
     /*
      * A class to encapsulate the test results and stuff, with some ease
      * of use methods to check the test results.

test/jdk/tools/launcher/TestSpecialArgs.java

@@ -246,6 +246,10 @@ void testNMArgumentProcessing() throws FileNotFoundException {
         if (!tr.contains("Error: Could not find or load main class AbsentClass")) {
             throw new RuntimeException("Test Fails");
         }
+
+        // Make sure we handle correctly the module long form (--module=)
+        tr = doExec(javaCmd, "-XX:NativeMemoryTracking=summary", "--module=jdk.compiler/com.sun.tools.javac.Main", "--help");
+        ensureNoWarnings(tr);
     }
 
     void ensureNoWarnings(TestResult tr) {

test/jdk/tools/launcher/modules/basic/BasicTest.java

@@ -29,6 +29,7 @@
  *          jdk.jlink
  * @build BasicTest jdk.test.lib.compiler.CompilerUtils
  * @run testng BasicTest
+ * @bug 8234076
  * @summary Basic test of starting an application as a module
  */
 
@@ -40,6 +41,8 @@
 
 import jdk.test.lib.compiler.CompilerUtils;
 import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.Utils;
 
 import org.testng.annotations.BeforeTest;
 import org.testng.annotations.Test;
@@ -70,6 +73,8 @@
     // the module main class
     private static final String MAIN_CLASS = "jdk.test.Main";
 
+    // for Windows specific launcher tests
+    static final boolean IS_WINDOWS = System.getProperty("os.name", "unknown").startsWith("Windows");
 
     @BeforeTest
     public void compileTestModule() throws Exception {
@@ -259,4 +264,87 @@ public void testTryRunWithMainClassInWrongModule() throws Exception {
         assertTrue(exitValue != 0);
     }
 
+
+    /**
+     * Helper method that creates a ProcessBuilder with command line arguments
+     * while setting the _JAVA_LAUNCHER_DEBUG environment variable.
+     */
+    private ProcessBuilder createProcessWithLauncherDebugging(String... cmds) {
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(Utils.addTestJavaOpts(cmds));
+        pb.environment().put("_JAVA_LAUNCHER_DEBUG", "true");
+
+        return pb;
+    }
+
+     /**
+     * Test the ability for the Windows launcher to do proper application argument
+     * detection and expansion, when using the long form module option and all passed in
+     * command line arguments are prefixed with a dash.
+     *
+     * These tests are not expected to work on *nixes, and are ignored.
+     */
+    public void testWindowsWithLongFormModuleOption() throws Exception {
+        if (!IS_WINDOWS) {
+            return;
+        }
+
+        String dir = MODS_DIR.toString();
+        String mid = TEST_MODULE + "/" + MAIN_CLASS;
+
+        // java --module-path=mods --module=$TESTMODULE/$MAINCLASS --help
+        // We should be able to find the argument --help as an application argument
+        ProcessTools.executeProcess(
+            createProcessWithLauncherDebugging(
+                "--module-path=" + dir,
+                "--module=" + mid,
+                "--help"))
+            .outputTo(System.out)
+            .errorTo(System.out)
+            .shouldContain("F--help");
+
+        // java --module-path=mods --module=$TESTMODULE/$MAINCLASS <...src/test>/*.java --help
+        // We should be able to see argument expansion happen
+        ProcessTools.executeProcess(
+            createProcessWithLauncherDebugging(
+                "--module-path=" + dir,
+                "--module=" + mid,
+                SRC_DIR.resolve(TEST_MODULE).toString() + "\\*.java",
+                "--help"))
+            .outputTo(System.out)
+            .errorTo(System.out)
+            .shouldContain("F--help")
+            .shouldContain("module-info.java");
+    }
+
+
+    /**
+     * Test that --module= is terminating for VM argument processing just like --module
+     */
+    public void testLongFormModuleOptionTermination() throws Exception {
+        String dir = MODS_DIR.toString();
+        String mid = TEST_MODULE + "/" + MAIN_CLASS;
+
+        // java --module-path=mods --module=$TESTMODULE/$MAINCLASS --module-path=mods --module=$TESTMODULE/$MAINCLASS
+        // The first --module= will terminate the VM arguments processing. The second pair of module-path and module will be
+        // deemed as application arguments
+        OutputAnalyzer output = ProcessTools.executeProcess(
+            createProcessWithLauncherDebugging(
+                "--module-path=" + dir,
+                "--module=" + mid,
+                "--module-path=" + dir,
+                "--module=" + mid))
+            .outputTo(System.out)
+            .errorTo(System.out)
+            .shouldContain("argv[ 0] = '--module-path=" + dir)
+            .shouldContain("argv[ 1] = '--module=" + mid);
+
+        if (IS_WINDOWS) {
+            output.shouldContain("F--module-path=" + dir).shouldContain("F--module=" + mid);
+        }
+
+        // java --module=$TESTMODULE/$MAINCLASS --module-path=mods
+        // This command line will not work as --module= is terminating and the module will be not found
+        int exitValue = exec("--module=" + mid, "--module-path" + dir);
+        assertTrue(exitValue != 0);
+    }
 }