On Wed, 19 Jun 2024 21:16:46 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 > > Jorn Vernee has updated the pull request incrementally with two additional > commits since the last revision: > > - review comments > - add man page Nice changes! src/jdk.jdeps/share/man/jnativescan.1 line 43: > 41: .PP > 42: jnativescan - static analysis tool that scans one or more jar files for > 43: uses of native functionality, such as restricted methods or I think using `functionalities` would be better. Also "restricted method **calls** works better too (so it's restricted method **calls** and native method **declarations. src/jdk.jdeps/share/man/jnativescan.1 line 55: > 53: The \f[V]jnative\f[R] tool is a static analysis tool provided by the JDK > 54: that scans a JAR file for uses of native functionality, such as > 55: restricted methods or \f[V]native\f[R] method declarations. Same comments as above src/jdk.jdeps/share/man/jnativescan.1 line 60: > 58: configuration, as well as a set of root modules, and a target release. > 59: It scans the jars on the class and module paths, and reports uses of > 60: native functionality either in a tree like structure, which also functionalities src/jdk.jdeps/share/man/jnativescan.1 line 68: > 66: .TP > 67: \f[V]--class-path\f[R] \f[I]path\f[R] > 68: Used to specify a path list of class path jar files to be scanned. I can't parse "path list of class path jar files". What is the argument here? I think it has to be a list of jars? If so, one way could be `a list of paths pointing to the jar files to be scanned` src/jdk.jdeps/share/man/jnativescan.1 line 76: > 74: .TP > 75: \f[V]--module-path\f[R] \f[I]path\f[R] > 76: Used to specify a path list of jar files or directories containing This is also hard to read. Perhaps `a list of paths pointing to the jar files to be scanned, or the directories containing said jar files.` src/jdk.jdeps/share/man/jnativescan.1 line 112: > 110: Used to specifiy a comma-separated list of module names that indicate > 111: the root modules to scan. > 112: All the modules in the root set will be scanned, as well as any modules Suggestion: All the root modules will be scanned, as well as any modules src/jdk.jdeps/share/man/jnativescan.1 line 121: > 119: .TP > 120: \f[V]--release\f[R] \f[I]version\f[R] > 121: Used to specify the Java SE release that specifies the set of restricted In principle, the release could also affect which methods are native or not? src/jdk.jdeps/share/man/jnativescan.1 line 127: > 125: This option should be set to the version of the runtime under which the > 126: application is eventually intended to be run. > 127: If this flag is omitted, the version of \f[V]jnativescan\f[R] is used as Maybe we should point to `Runtime.version()` instead? src/jdk.jdeps/share/man/jnativescan.1 line 146: > 144: For the class path, the tool will scan all jar files, including those > 145: found recursively through the \f[V]Class-Path\f[R] manifest attribute. > 146: For the module path, the tool scans all modules in the root module set Note: sometimes you say root modules, sometimes root module set. I'd suggest to pick one and remain consistent. src/jdk.jdeps/share/man/jnativescan.1 line 166: > 164: .fi > 165: .PP > 166: \f[V]app.jar (ALL-UNNAMED)\f[R] is the path to the jar file, with the This is a good explanation, I'm not sure whether it's necessary, as the output seems quite self-explanatory. But I'm ok either way. ------------- Marked as reviewed by mcimadamore (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2129052320 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646696108 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646696214 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646696446 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646696855 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646697366 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646702494 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646703008 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646703573 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646704113 PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646704937
