On Tue, 18 Jun 2024 16:30:37 GMT, Jorn Vernee <jver...@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

Looks very good! I left a bunch of comments and question.

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?

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)

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

PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2128837697
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646558424
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646560383

Reply via email to