RFR: 8353840: JNativeScan should not throw error for missing system classes

[Jorn Vernee]

On Tue, 8 Apr 2025 08:18:36 GMT, Danish Nawab <duke at openjdk.org> wrote:

> ## Description
> 
> https://bugs.openjdk.org/browse/JDK-8353840 
> 
> ### Existing behavior
> Log the error message and terminate in case of missing system class
> 
> 
> $ jnativescan --class-path reactor-core-3.7.4.jar; echo "Exit code: $?"
> 
> ERROR: Error while processing method: reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory$TracingException::get()String
> CAUSED BY: System class can not be found: sun.misc.JavaLangAccess
> Exit code: 1
> 
> 
> ### New behavior
> Still log the error message about the missing system class, but continue the analysis
> 
> 
> $ build/macosx-aarch64-server-release/jdk/bin/jnativescan --class-path reactor-core-3.7.4.jar; echo "Exit code: $?" 
> 
>   <no restricted methods>
> Error while processing method: reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory$TracingException::get()String: System class can not be found: sun.misc.JavaLangAccess
> Error while processing method: reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory$TracingException::<clinit>()void: System class can not be found: sun.misc.SharedSecrets
> Error while processing method: reactor.core.publisher.CallSiteSupplierFactory$SharedSecretsCallSiteSupplierFactory::<clinit>()void: System class can not be found: sun.misc.SharedSecrets
> Exit code: 0
> 
> 
> ## Design choices
> 
> Propagate `err` all the way to `NativeMethodFinder` which can log the error to it, but continue with the analysis
> 
> ### Alternatives considered
> 
> - Instead of propagating `err` downstream, adapt the outer try-catch in `com.sun.tools.jnativescan.Main#run`.
>     - Con: This would require complicated error recovery synchronization between Main and `JNativeScanTask` to resume the scanning after the exception 
> - Instead of adapting the catch block in `com.sun.tools.jnativescan.NativeMethodFinder#findAll`, don't even surface the exception from `com.sun.tools.jnativescan.ClassResolver.SystemModuleClassResolver#lookup` rather log it right in `ClassResolver`
>     - Con: We don't have access to `MethodRef`/`MethodModel` in ClassResolver#lookup which will make the error message less detailed/useful
> 
> ### stdout vs stderr 
> 
> One could argue that since this is a non-terminal error, perhaps it should be logged to stdout. However, my thinking was that while it may be non-terminal, it is still an "error", and thus should be logged to stderr. I am happy to revisit this decision, if needed.
> 
> ## Testing
> 
> The existing test `TestMissingSystemClass#testSingleJarClassPath` has ...

I've updated the issue title to start with a capital letter.

Also, note that there was some other cleanup I wanted to do in this area first: https://github.com/openjdk/jdk/pull/24493 which will likely result in merge conflicts.

src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/NativeMethodFinder.java line 91:

> 89:                              err.println("Error while processing method: " +
> 90:                                      MethodRef.ofModel(methodModel) + ": " + e.getMessage());
> 91:                          }

This catch block is per-method. It means that if the rest of the method contained references to restricted methods, we would not see them. The catch block should be moved to be just around the call to `isRestricted`, which can fail. Then we can record the error and keep iterating over the other instructions.

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

Changes requested by jvernee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24499#pullrequestreview-2749671492
PR Comment: https://git.openjdk.org/jdk/pull/24499#issuecomment-2786176663
PR Review Comment: https://git.openjdk.org/jdk/pull/24499#discussion_r2033007295