Address review comments

Improve warning for JNI methods, similar to what's described in JEP 472 Beef up tests
mcimadamore · May 14, 2024 · 0d21bf9 · 0d21bf9
1 parent 6805e29
commit 0d21bf9
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 15 deletions.
diff --git a/src/java.base/share/classes/java/lang/Module.java b/src/java.base/share/classes/java/lang/Module.java
@@ -312,20 +312,20 @@ void ensureNativeAccess(Class<?> owner, String methodName, Class<?> currentClass
         ModuleBootstrap.IllegalNativeAccess illegalNativeAccess = ModuleBootstrap.illegalNativeAccess();
         if (illegalNativeAccess != ModuleBootstrap.IllegalNativeAccess.ALLOW &&
                 !EnableNativeAccess.isNativeAccessEnabled(target)) {
+            String mod = isNamed() ? "module " + getName() : "an unnamed module";
+            if (currentClass != null) {
+                // try to extract location of the current class (e.g. jar or folder)
+                URL url = System.codeSource(currentClass);
+                if (url != null) {
+                    mod += " (" + url + ")";
+                }
+            }
             if (illegalNativeAccess == ModuleBootstrap.IllegalNativeAccess.DENY) {
-                throw new IllegalCallerException("Illegal native access from: " + this);
+                throw new IllegalCallerException("Illegal native access from " + mod);
             } else if (EnableNativeAccess.trySetEnableNativeAccess(target)) {
                 // warn and set flag, so that only one warning is reported per module
                 String cls = owner.getName();
                 String mtd = cls + "::" + methodName;
-                String mod = isNamed() ? "module " + getName() : "an unnamed module";
-                if (currentClass != null) {
-                    // try to extract location of the current class (e.g. jar or folder)
-                    URL url = System.codeSource(currentClass);
-                    if (url != null) {
-                        mod += " (" + url + ")";
-                    }
-                }
                 String modflag = isNamed() ? getName() : "ALL-UNNAMED";
                 String caller = currentClass != null ? currentClass.getName() : "code";
                 if (jni) {

diff --git a/test/jdk/java/foreign/enablenativeaccess/TestEnableNativeAccess.java b/test/jdk/java/foreign/enablenativeaccess/TestEnableNativeAccess.java
@@ -146,9 +146,32 @@ public void testBadValue() throws Exception {
         run("panama_deny_bad_all_module_path_module", PANAMA_MAIN,
                 failWithWarning("WARNING: Unknown module: ALL-MODULE-PATH specified to --enable-native-access"),
                 "--illegal-native-access=deny", "--enable-native-access=ALL-MODULE-PATH" );
-        run("panama_deny_no_module", PANAMA_MAIN,
+        run("panama_deny_no_module_main", PANAMA_MAIN,
                 failWithError("module panama_module"),
                 "--illegal-native-access=deny");
+        run("panama_deny_no_module_invoke", PANAMA_INVOKE,
+                failWithError("module panama_module"),
+                "--illegal-native-access=deny");
+        run("panama_deny_no_module_reflection", PANAMA_REFLECTION,
+                failWithError("module panama_module"),
+                "--illegal-native-access=deny");
+        run("panama_deny_no_module_jni", PANAMA_JNI,
+                failWithError("module panama_jni_load_module"),
+                "--illegal-native-access=deny");
+    }
+
+    public void testDetailedWarningMessage() throws Exception {
+        run("panama_enable_native_access_warn_jni", PANAMA_JNI,
+                success()
+                        // call to System::loadLibrary from panama_jni_load_module
+                        .expect("WARNING: A restricted method in java.lang.System has been called")
+                        .expect("WARNING: java.lang.System::loadLibrary has been called by org.openjdk.jni.PanamaMainJNI in module panama_jni_load_module")
+                        // JNI native method binding in panama_jni_def_module
+                        .expect("WARNING: A native method in org.openjdk.jni.def.PanamaJNIDef has been bound")
+                        .expect("WARNING: org.openjdk.jni.def.PanamaJNIDef::nativeLinker0 has been called by org.openjdk.jni.def.PanamaJNIDef in module panama_jni_def_module")
+                        // upcall to Linker::downcallHandle from JNI code
+                        .expect("WARNING: A restricted method in java.lang.foreign.Linker has been called")
+                        .expect("WARNING: java.lang.foreign.Linker::downcallHandle has been called by code in an unnamed module"));
     }
 
     private int count(Iterable<String> lines, CharSequence cs) {

diff --git a/test/jdk/java/foreign/enablenativeaccess/TestEnableNativeAccessDynamic.java b/test/jdk/java/foreign/enablenativeaccess/TestEnableNativeAccessDynamic.java
@@ -56,7 +56,7 @@ public Object[][] succeedCases() {
 
     @DataProvider(name = "failureCases")
     public Object[][] failureCases() {
-        String errMsg = "Illegal native access from: module panama_module";
+        String errMsg = "Illegal native access from module panama_module";
         return new Object[][] {
                 { "panama_enable_native_access_fail", PANAMA_MAIN, failWithError(errMsg) },
                 { "panama_enable_native_access_fail_reflection", PANAMA_REFLECTION, failWithError(errMsg) },

diff --git a/...java/foreign/enablenativeaccess/panama_jni_load_module/org/openjdk/jni/PanamaMainJNI.java b/...java/foreign/enablenativeaccess/panama_jni_load_module/org/openjdk/jni/PanamaMainJNI.java
@@ -27,11 +27,8 @@
 
 public class PanamaMainJNI {
 
-    static {
-        System.loadLibrary("LinkerInvokerModule");
-    }
-
     public static void main(String[] args) {
+        System.loadLibrary("LinkerInvokerModule");
         PanamaJNIUse.run();
     }
 }