RFR: 8304425: ClassHierarchyResolver from Reflection [v9]

[ExE Boss]

On Wed, 7 Jun 2023 14:15:10 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Add API to explore Class Hierarchy with a `ClassLoader` or a `Lookup` with proper privileges, with tests.
>> 
>> This addition is useful in case classes at runtime are not loaded from the system class loader, such as Proxy. This is also useful to APIs that generate bytecode with a `Lookup` object, such as a custom single-abstract-method class implementations from a method handle.
>> 
>> See https://mail.openjdk.org/pipermail/classfile-api-dev/2023-March/000249.html as well.
>> 
>> Current questions, which I wish to discuss with @asotona:
>> 1. Should the resolver fail fast on `IllegalAccessException` from the lookup? This usually indicates the hierarchy resolver is set up improperly, and proceeding may simply yield verification errors in class loading that are hard to track. For bytecode-generating APIs, throwing access errors for the Lookup eagerly is also more preferable than later silent generation failure.
>> 2. Whether the default resolver should be reading from jrt alone, reflection alone, or jrt then reflection. I personally believe reflection alone is more reliable, for classes may redefined with instrumentation or jfr, which may not be reflected in the system resources.
>> 3. In addition, I don't think chaining system class loader reflection after system resource retrieval is really meaningful: is there any case where reflection always works while the system resource retrieval always fails?
>
> Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
> 
>  - Review changes, fixed tests
>  - Merge branch 'master' into hierarchy-resolve
>  - 1. Moved the default resolver to a static method, in anticipation of future changes
>    2. Removed SecurityManager related content
>    3. Changed ClassHierarchyInfo into an interface
>    4. Moved caching method from static to instance method
>  - Merge branch 'master' into hierarchy-resolve
>  - rename to ofClassLoading/ofResourceParsing
>    convert the default class provider to bypass security manager restrictions
>  - Merge branch 'master' into hierarchy-resolve
>  - Merge branch 'master' into hierarchy-resolve
>  - Test both cached and uncached resolvers
>  - Update the class hierarchy resolver api as per mailing list last week
>  - Merge branch 'master' into hierarchy-resolve
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/a1ab377d...90645b6f

src/java.base/share/classes/jdk/internal/classfile/impl/Util.java line 103:

> 101:             }
> 102:             return descOrInternalName.substring(1, descOrInternalName.length() - 1).replace('/', '.');
> 103:         } else {

Note that this won’t work right if this method is called with the internal name of a class whose FQCN starts with `L`, the correct implementation would be:
Suggestion:

        if (descOrInternalName.startsWith("L") && descOrInternalName.endsWith(";")) {
            // descriptors of classes or interfaces
            if (descOrInternalName.length() <= 2) {
                throw new IllegalArgumentException(descOrInternalName);
            }
            return descOrInternalName.substring(1, descOrInternalName.length() - 1).replace('/', '.');
        } else {

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13082#discussion_r1226031008