On Wed, 19 Jun 2024 17:16:54 GMT, Maurizio Cimadamore <mcimadam...@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
>
> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/Main.java line 126:
> 
>> 124:             out.println("""
>> 125:                 The jnativescan tool can be used to find methods that 
>> may access native functionality when
>> 126:                 run. This includes methods that call restricted 
>> methods, and 'native' method declarations.
> 
> I think it would be more readable if we said "This includes restricted method 
> calls and `native` method declarations`. The way it is now it seems (for some 
> weird subjective reason) that the sentence is saying "methods ... that call 
> ... native method declarations" :-)

Oh, the oxford comma has failed me :D

> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
>  line 43:
> 
>> 41: import java.util.*;
>> 42: 
>> 43: class RestrictedMethodFinder {
> 
> The name of this is a bit confusing as this class looks for both restricted 
> methods and JNI decls

True. How about `NativeAccessFinder`?

> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/RestrictedMethodFinder.java
>  line 48:
> 
>> 46:     // see 
>> make/langtools/src/classes/build/tools/symbolgenerator/CreateSymbols.java
>> 47:     private static final String RESTRICTED_NAME = 
>> "Ljdk/internal/javac/Restricted+Annotation;";
>> 48:     private static final List<String> RESTRICTED_MODULES = 
>> List.of("java.base");
> 
> unused?

Yes

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646535247
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646538303
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1646535411

Reply via email to