Hello core-libs-dev,
I'm a VM engineer at Microsoft and new to this mailing list. I took a look at
JDK- 8234076 and the root cause is similar to a prior thread on a Windows
launcher bug for JDK- 8231863, after the command line arguments are processed,
the static variable firstAppArgIndex in
src/java.base/share/native/libjli/args.c remains set to NOT_FOUND (-1). The
reason for not finding a firstAppArgIndex in this case is because the command
line supplied to the launcher has every argument prefixed with a dash, for
example:
java --module-path=mods --module=com.greetings/com.greetings.Main --help
Because all arguments are prefixed with a dash, the checkArg function inside
src/java.base/share/native/libjli/args.c never finds an app arg index.
Currently, this only causes an issue on Windows because of the custom argument
expansion code in function CreateApplicationArgs of
src/java.base/windows/native/libjli/java_md.c. The code queries the
firstAppArgIndex and proceeds to perform an array access without checking if
the app index is greater or equal to 0, causing the randomly observed crash.
I have made the following patch to address the issue:
--- a/src/java.base/windows/native/libjli/java_md.c
+++ b/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"
@@ -1015,6 +1016,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);
I wanted to ask for an advice and opinion if this is the correct way to fix the
problem?
I did few tests and I can confirm that the fix above doesn't change the current
behaviour on Windows, because in case the negative index array access doesn't
cause a crash, the later sanity check below (in java_md.c CreateApplicationArgs
) will always catch the invalid case and simply pass down the arguments as is.
// sanity check, ensure same number of arguments for application
if (j != argc) {
JLI_TraceLauncher("Warning: app args count doesn't match, %d %d\n", j,
argc);
JLI_TraceLauncher("passing arguments as-is.\n");
JLI_MemFree(appArgIdx);
return NewPlatformStringArray(env, strv, argc);
}
However, if the command line is supplied with --module= instead of --module
followed by a blank space, Windows will not perform argument expansion on the
application arguments. For example:
java --module-path=mods --module=com.greetings/com.greetings.Main ./* --help
(the path will not be expanded)
java --module-path=mods --module com.greetings/com.greetings.Main ./* --help
(the path will be correctly expanded)
I tested on Linux x86-64 and the argument expansion is correctly performed in
both cases where --module= is used and where --module is followed by a space.
Essentially, Windows behaves differently in this case, but it has always been
this way. The patch only prevents the random crash, fixing the command line
incompatibility will require a more substantial change.
Please let me know if this approach is acceptable for the current bug report
and I'll make a webrev and include appropriate launcher tests. I was thinking
the tests should do extra validation on the output from _JAVA_LAUNCHER_DEBUG on
Windows.
Thank you!
-Nikola
