RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive [v7]

[Ioi Lam]

On Wed, 30 Sep 2020 04:59:20 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> This patch is reorganized after 8252725, which is separated from this patch to refactor jlink glugin code. The previous
>> webrev with hg can be found at: http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-05. With 8252725 integrated, the
>> regeneration of holder classes is simply to call the new added GenerateJLIClassesHelper.cdsGenerateHolderClasses
>> function.  Tests: tier1-4
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove trailing word of line which is not used in holder class regeneration. There is a trailing LF (Line Feed) so trim
>   white spaces from both front and end of the line or it will fail method type validation.

Changes requested by iklam (Reviewer).

test/hotspot/jtreg/runtime/cds/appcds/DumpClassListWithLF.java line 70:

> 68:                 "Hello",
> 69:                 "@lambda-form-invoker [LF_RESOLVE] java.lang.invoke.DirectMethodHandle$Holder invokeNothing  L7_L
> (anyword)", 70:                 "@lambda-form-invoker [LF_RESOLVE] java.lang.invoke.DirectMethodHandle$Holder
> invokeNothing  LL_I anyword"),

We shouldn't allow the classlist to contain arbitrary data. These two cases should generate an error.

src/hotspot/share/classfile/lambdaFormInvokers.cpp line 52:

> 50:
> 51: // trim white spaces from front and end of string.
> 52: char* trim(char* s) {

I think this creates unnecessary dependency between the C code and the Java code. The C code assumes that the Java code
has appended something like "(salvaged)" into the output, and tries to get rid of that in a non-obvious way. It's
better to modify the Java code from

static void traceSpeciesType(String cn, Class<?> salvage) {
    if (TRACE_RESOLVE || CDS.isDumpLoadedClassList()) {
        String traceSP = SPECIES_RESOLVE + " " + cn + (salvage != null ? " (salvaged)" : " (generated)");
        if (TRACE_RESOLVE) {
            System.out.println(traceSP);
        }
        CDS.logTraceResolve(traceSP);
    }
}

to

    if (TRACE_RESOLVE || CDS.isDumpLoadedClassList()) {
        String traceSP = SPECIES_RESOLVE + " " + cn;
        if (TRACE_RESOLVE) {
            System.out.println(traceSP + (salvage != null ? " (salvaged)" : " (generated)"));
        }
        CDS.logTraceResolve(traceSP);
    }

test/hotspot/jtreg/runtime/cds/appcds/DumpClassListWithLF.java line 85:

> 83:             appJar, classlist(
> 84:                 "Hello",
> 85:                 "@lambda-form-invoker [LF_XYRESOLVE] java.lang.invoke.DirectMethodHandle$Holder invokeStatic  L7_L
> (any)",

We should not allow incorrect input. This should generate an error.

test/hotspot/jtreg/runtime/cds/appcds/DumpClassListWithLF.java line 78:

> 76:                 "Hello",
> 77:                 "@lambda-form-invoker [LF_RESOLVE] my.nonexist.package.MyNonExistClassName$holder invokeStatic
> L7_L", 78:                 "@lambda-form-invoker [LF_RESOLVE] my.nonexist.package.MyNonExistClassName$holder
> invokeStatic  LL_I"),

I think it's dangerous to allow arbitrary class names here. InvokerBytecodeGenerator doesn't check the classname. This
will make it possible to overwrite the contents of arbitrary classes. We should have a check here and allow only the
specific holder classes that are supported.

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

PR: https://git.openjdk.java.net/jdk/pull/193