On Wed, 19 Jun 2024 17:53:12 GMT, Maurizio Cimadamore <[email protected]>
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
>
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/ClassResolver.java line
> 126:
>
>> 124:
>> 125: private static Map<String, String> packageToSystemModule() {
>> 126: List<ModuleDescriptor> descriptors = ModuleFinder.ofSystem()
>
> Is it a problem that we compute the package -> module map using the runtime
> info (so latest version) but then all the other info is taken from a
> release-specific symbol file? E.g. say that package "foo" was moved from
> module "A" to module "B" in version N, and that user passes N - 1 as release
> to the scan tool - would that work?
That's a good point, I don't think that scenario will work. We should really
use the release specific info if we can. I think that's relatively easy to do,
will take a look.
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java
> line 81:
>
>> 79:
>> 80: Map<ScannedModule, Map<ClassDesc, List<RestrictedUse>>>
>> allRestrictedMethods;
>> 81: try(ClassResolver classesToScan =
>> ClassResolver.forScannedModules(modulesToScan, version);
>
> Could it make sense here to "chain" the two resolvers into one, and just pass
> the chained one to the restricted finder?
We need to scan all the classes in the `classesToScan` resolver, but only look
for restricted methods in the system resolver. So, they need to stay separate.
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java
> line 165:
>
>> 163: restrictedUses.forEach(use -> {
>> 164: switch (use) {
>> 165: case
>> RestrictedUse.NativeMethodDecl(MethodRef nmd) ->
>
> should user be able to filter output using an option? E.g. say if they are
> only interested in restricted methods but not JNI (seems remote, but adding
> it here for completeness)
That's a good suggestion, but to pull on that string some more, there's also
the possibility of filtering based on class/package/method names. Maybe for a
followup?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646564392
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646563349
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646566299
