RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v7]

[Jan Lahoda]

On Thu, 20 Jun 2024 19:45:29 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find code that accesses native functionality. Currently this includes `native` method declarations, and methods marked with `@Restricted`.
>> 
>> The tool accepts a list of class path and module path entries through `--class-path` and `--module-path`, and a set of root modules through `--add-modules`, as well as an optional target release with `--release`.
>> 
>> The default mode is for the tool to report all uses of `@Restricted` methods, and `native` method declaration in a tree-like structure:
>> 
>> 
>> app.jar (ALL-UNNAMED):
>>   main.Main:
>>     main.Main::main(String[])void references restricted methods:
>>       java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>>     main.Main::m()void is a native method declaration
>> 
>> 
>> The `--print-native-access` option can be used print out all the module names of modules doing native access in a comma separated list. For class path code, this will print out `ALL-UNNAMED`.
>> 
>> Testing: 
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API, and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add extra test for missing root modules

Overall looks great to me. Some minor comments inline.

src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java line 87:

> 85:     @Override
> 86:     public PlatformDescription getPlatform(String platformName, String options) throws PlatformNotSupported {
> 87:         if (Source.lookup(platformName) == null) {

`Source.lookup` is probably not the right way to check whether the platform is supported - the `Source` enum may (and does) contain versions for which we don't have the historical API record. I think `SUPPORTED_JAVA_PLATFORM_VERSIONS.contains(platformName)` would work better, since the content is read directly from `ct.sym`.

src/jdk.jdeps/share/classes/com/sun/tools/jdeprscan/Main.java line 417:

> 415:                 return false;
> 416:             }
> 417:             JavaFileManager fm = pp.getPlatformTrusted(release).getFileManager();

Not sure if this change is necessary. I believe `release` is verified to be a valid platform name at this point, so even with the new check, it should still work. (And `getPlatformTrusted` could possibly be eliminated.) But maybe I am missing something?

src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/ClassResolver.java line 125:

> 123:         private static Map<String, String> packageToSystemModule(JavaFileManager platformFileManager) {
> 124:             try {
> 125:                 Set<JavaFileManager.Location> locations = platformFileManager.listLocationsForModules(

FWIW: +1 on using the modules from the `ct.sym` rather than runtime modules here!

test/langtools/tools/jnativescan/TestJNativeScan.java line 174:

> 172:                                   "-add-modules", "org.singlejar,org.myapp",
> 173:                                   "--print-native-access"))
> 174:                 .stdoutShouldContain("org.singlejar")

It is a small thing, bu was there a consideration for a stronger assert on the output, checking that the output is precisely something like `--enable-native-access org.lib,org.service,org.singlejar`? Would require that the output is stable, which may be tricky, but also not a bad property. Just an idea for consideration.

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

PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2133198642
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649303668
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649307310
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649297021
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649294137