[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR: 8299329: Assertion failure with fastdebug build when trying to use CDS without classpath
From:       Ioi Lam <iklam () openjdk ! org>
Date:       2022-12-24 4:18:48
Message-ID: f5WGOwkMlB5-XsPKA7qiDyFMv2eiZa9RB5J7Nq8ggYk=.94d2dcee-0d9d-450c-be51-ffc7f3fbaae3 () github ! com
[Download RAW message or body]

On Fri, 23 Dec 2022 18:12:38 GMT, Severin Gehwolf <sgehwolf@openjdk.org> wrote:

> > It appears longest_common_app_classpath_prefix_len() is not returning correct \
> > value when there is no file separator in the path being searched backwards. \
> > Instead of returning 0 it return 1. 
> > In case of empty classpath, it can result in assertion failure in check_paths():
> > 
> > `assert(strlen(rp_array->at(i)) > (size_t)runtime_prefix_len, "sanity");`
> > 
> > It can also result in incorrectly validating the app classpaths if they only \
> > differ by first character. Eg: 
> > Dump time:
> > -cp hello.jar
> > Run time:
> > -cp mello.jar
> > 
> > This would not result in classpath mismatch!
> > 
> > This fix updates longest_common_app_classpath_prefix_len() to return 0 if no file \
> > separator character is found. 
> > Signed-off-by: Ashutosh Mehra <asmehra@redhat.com>
> 
> Could we add a test for this a la \
> `test/hotspot/jtreg/runtime/cds/appcds/WrongClasspath.java`?

> @jerboaa I see WrongClasspath.java already has a test for "no classpath" condition, \
> but it is not failing with debug build. The command line used for running "no \
> classpaht" test reveals it is actually running with a classpath: 
> ```
> /work/jdk/build/base-fd/images/jdk/bin/java -cp \
> /work/jdk/JTwork/classes/runtime/cds/appcds/WrongClasspath.d:/work/jdk/test/hotspot/ \
> jtreg/runtime/cds/appcds:/work/jdk/JTwork/classes/test/lib:/work/jdk/test/lib:/work/ \
> jtreg/build/images/jtreg/lib/javatest.jar:/work/jtreg/build/images/jtreg/lib/jtreg.jar \
> -Xshare:on -showversion \
> -XX:SharedArchiveFile=/work/jdk/JTwork/scratch/appcds-15h18m10s791.jsa \
> -Dtest.timeout.factor=1.0 -XX:+UnlockDiagnosticVMOptions -XX:VerifyArchivedFields=1 \
> -Xlog:cds Hello ```
> 
> The classpath is appended by \
> jdk.test.lib.process.ProcessTools#createJavaProcessBuilder(): 
> ```
> args.add("-cp");
> args.add(System.getProperty("java.class.path"));
> ```
> 
> That means the "no classpath" test is not really testing what it is expected to.

Maybe we can do this?


    // Then try to execute the archive without -classpath -- it should fail
    TestCommon.run(
-       /* "-cp", appJar, */ // <- uncomment this and the execution should succeed
+       // Force classpath to be the empty string. Note: -cp "" doesn't work on \
Windows. +       "-Djava.class.path=",
        "-Xlog:cds",
        "Hello")
        .assertAbnormalExit(unableToUseMsg, mismatchMsg, hintMsg);


That should override the `-cp` parameter given by `createJavaProcessBuilder`, which \
appears earlier in the command-line.

Also, I think you should add the `-cp hello.jar` vs `-cp mello.jar` test case into \
WrongClasspath.java.

Thanks!

-------------

PR: https://git.openjdk.org/jdk/pull/11781


[prev in list] [next in list] [prev in thread] [next in thread]