JDK-8231863 bug fix candidate

[Henry Jen]

The fix is on the spot, good catch.

As the test, test/jdk/tools/launcher/ArgsFileTest.java is testing the file expansions, we can add a test case there to have an argument file not have newline at the end, and check that we get correct arguments.

Cheers,
Henry


> On Nov 1, 2019, at 7:06 AM, Mat Carter <Matthew.Carter at microsoft.com> wrote:
> 
> Hello core-libs-dev
> 
> I'm a VM engineer at Microsoft, now that we've signed the OCA (news of which was shared by Bruno Borges in the discuss mailing list) I wanted to pick up an issue in order to gain familiarity with the change process
> 
> After reproducing the error reported in "JDK-8231863: Crash if classpath is read from @argument file and the main gets option argument" (https://bugs.openjdk.java.net/browse/JDK-8231863), I've identified the root cause and a candidate fix.
> 
> As such I'd like to pick this issue up, assuming its current state of unassigned still holds what's the process of having it assigned to me or having it somehow reserved as I don't want to cause unnecessary duplicated work.
> 
> As well as validating the fix in the debugger on WIndows and Linux, I've run the AdoptOpenJDK sanity tests on Mac, Windows and Linux for both tip and 11u.
> 
> I'm currently figuring out where/how to write regression tests for this case; input from this mailing list would be welcomed (examples of tests for similar bugs etc)
> 
> This error occurs on all platforms (tip + 11u) but only results in a crash on Windows in java_md.c due to that platforms dependency on JLI_GetAppArgIndex().  Note that while the error is 100% reproducible, the crash only occurs <10%.  The following change fixes that problem by passing in the final token fragment from the argument file into the state machine via checkArg()
> 
> --- a/src/java.base/share/native/libjli/args.c
> +++ b/src/java.base/share/native/libjli/args.c
> @@ -337,7 +337,9 @@ static JLI_List readArgFile(FILE *file) {
>      // remaining partial token
>      if (ctx.state == IN_TOKEN || ctx.state == IN_QUOTE) {
>          if (ctx.parts->size != 0) {
> -            JLI_List_add(rv, JLI_List_combine(ctx.parts));
> +            token = JLI_List_combine(ctx.parts);
> +            checkArg(token);
> +            JLI_List_add(rv, token);
>          }
>      }
>      JLI_List_free(ctx.parts);
> 
> The fix tackles the memory corruption indirectly.  Further preventative changes could be made in java_md.c/CreateApplicationArgs to avoid future memory corruptions (caused by JLI_GetAppArgIndex() returning -1 in this case) ; but I felt that that handling those error cases ran secondary to addressing the bug in the argument file parsing code.
> 
> Cheers
> Mat