Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 19:37:23 GMT, Jorn Vernee wrote: > It looks like it's possible to get even more custom by using a custom > `HelpFormatter` as well, if we wanted. I think what you have is okay for the first integration, just looks odd to have the help/usage say "String" and "Path". Using a help formatter can be something for a follow-up issue, assuming it feasible. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2197088172
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 17:44:54 GMT, Alan Bateman wrote: >> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java >> line 113: >> >>> 111: // Class-Path attribute specifies that jars that >>> 112: // are not found are simply ignored. Do the same >>> here >>> 113: classPathJars.offer(otherJar); >> >> The expansion of the class path looks right but the Class-Path attribute is >> awkward to deal with as its relative URI rather than a file path. More on >> this this later. > > Not important for now but the expansion will probably need to"visited" set to > detect cycles (yes, it is possible). > > In addition to cycles, it is possible to have several JAR files with the same > Class-Path value, e.g. lib/logging.jar, so a visited set would be useful for > that too. FWIW, javac has `com.sun.tools.javac.file.FSInfo.getJarClassPath`, which is not transitive, and is implemented a bit differently, but I wonder if there's a chance for some code sharing. Maybe not now, but eventually. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649296334
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 19:37:23 GMT, Jorn Vernee wrote: > I've massaged the parsing code to where the help message now looks like this: It is better but it might mean looking at HelpFormatter as you mentioned,. Right now the usage message `--add-modules ` whereas the java --help will print `--add-modules [,...]` and javac --help will print `--add-modules (,)*`. Not super important for now, I think the main thing with this PR is getting agreement to add a tool with a short shelf life. It's clear there is a longer term story here to have one tool to do static analysis and replace (or front) `jdeps`, `jdeprscan` and this the new tool here. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2182708610
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee 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: > > update man page header to be consisten with the others I've massaged the parsing code to where the help message now looks like this: ... Option Description -- --- -?, -h, --help help --add-modules List of root modules to scan --class-path The class path as used at runtime --module-path The module path as used at runtime --print-native-access print a comma separated list of modules that may perform native access operations. ALL-UNNAMED is used to indicate unnamed modules. --release The runtime version that will run the application --version Print version information and exit I'm not sure if joptsimple has a way to display the option arguments like `[,]` to indicate multiple options (at least I couldn't find it immediately). - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181397654
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 16:13:04 GMT, Alan Bateman wrote: > Another thing is that using joptsimple gives up a bit of control, e.g. the > help output shows the parameter for --class-path as `` where the java > launcher and other tools will show "path" or "class path". Same thing with > `--release` where it shows `` where jar, javac, and other tools will > say "release". Not important for now, just comments from trying out the tool. It seems that it is possible in joptsimple to attach a description to the argument as well, and get something like: --module-pathThe class path as used at runtime It looks like we could also customize the argument type and get something like: --module-path (we'd just need to add a `ModulePath` class with a `valueOf(String)` method) - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181315973
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 17:40:25 GMT, Alan Bateman wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update man page header to be consisten with the others > > src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java > line 113: > >> 111: // Class-Path attribute specifies that jars that >> 112: // are not found are simply ignored. Do the same >> here >> 113: classPathJars.offer(otherJar); > > The expansion of the class path looks right but the Class-Path attribute is > awkward to deal with as its relative URI rather than a file path. More on > this this later. Not important for now but the expansion will probably need to a "visited" set to detect cycles (yes, it is possible). - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647934012
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee 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: > > update man page header to be consisten with the others src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java line 113: > 111: // Class-Path attribute specifies that jars that > 112: // are not found are simply ignored. Do the same here > 113: classPathJars.offer(otherJar); The expansion of the class path looks right but the Class-Path attribute is awkward to deal with as its relative URI rather than a file path. More on this this later. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647929185
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Wed, 19 Jun 2024 21:13:33 GMT, Maurizio Cimadamore wrote: >> What do you suggest? Just a note in the error message that exploded >> modules/class paths are not supported? > > Something like that yes An altermative is to use ResolvedModule::reference to get a ModuleReference, then use its open method to open the contents of the module to get a ModuleReader. That will give you a stream over the names of all resources in the module. It will work for all exploded and packaged modules. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647911987
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Tue, 18 Jun 2024 16:35:10 GMT, Jorn Vernee wrote:
>> Jorn Vernee has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update man page header to be consisten with the others
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java
> line 93:
>
>> 91: }
>> 92:
>> 93: public PlatformDescription getPlatformTrusted(String platformName) {
>
> I noticed that `getPlatform` was not throwing an exception if the release was
> not valid, which then later results in an NPE. I've added an explicit check
> here instead. The caller can then catch the exception.
I see the "options" arg is not used so maybe another approach here is a one-arg
getPlatform that throws PlatformNotSupported and then migrate the other usages
at another time.
(This is just a passing comment, not important for the core of this feature).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1647877795
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee 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:
>
> update man page header to be consisten with the others
`jnativescan --version` prints the value of Runtime.version where jdeprscan and
some of the other tools print System.getProperty("java.version"). Just
something to check as they might look inconsistent.
-
PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181075350
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee 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: > > update man page header to be consisten with the others One thing to check is usages like `jnativescan foo`, should that fail as the tool doesn't take args. Another thing is that using joptsimple gives up a bit of control, e.g. the help output shows the parameter for --class-path as `` where the java launcher and other tools will show "path" or "class path". Same thing with `--release` where it shows `` where jar, javac, and other tools will say "release". Not important for now, just comments from trying out the tool. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2181070909
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:51:22 GMT, Evemose wrote: > wouldn't it be better to create one uniform tool See my reply here: https://github.com/openjdk/jdk/pull/19774#issuecomment-2179078565 - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2180653743
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 12:11:25 GMT, Jorn Vernee 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: > > update man page header to be consisten with the others Just as a guest here. Just theoretically, wouldn't it be better to create one uniform tool to detect all "undesired" APIs. This would be more flexible, and potentially could be adapted to use some kind of "meticulous" mode, that will also indicate usage of public APIs, that, while never will be removed, are recommended to remove in favor of new alternatives (like old Date API or old collections". Just a thought btw, not a proposal for now - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2180594098
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
> 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: update man page header to be consisten with the others - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/4c6abddf..06f53e31 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19774=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19774=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
