[GR-67460] Change language permission feature to print only a single violation per privileged method.

PullRequest: graal/21440

Author: tzezula

substratevm/src/com.oracle.svm.truffle.tck/src/META-INF/native-image/native-image.properties

@@ -21,6 +21,7 @@ JavaArgs = --add-exports org.graalvm.nativeimage.base/com.oracle.svm.util=ALL-UN
            --add-exports jdk.internal.vm.ci/jdk.vm.ci.meta=ALL-UNNAMED
 
 ProvidedHostedOptions = \
+  TruffleTCKCollectMode= \
   TruffleTCKPermissionsReportFile= \
   TruffleTCKPermissionsExcludeFiles= \
   TruffleTCKPermissionsMaxStackTraceDepth= \

substratevm/src/com.oracle.svm.truffle.tck/src/com/oracle/svm/truffle/tck/PermissionsFeature.java

@@ -129,6 +129,25 @@ public enum ActionKind {
         Throw
     }
 
+    /**
+     * Specifies how privileged method violations are reported. See
+     * {@link Options#TruffleTCKCollectMode}.
+     */
+    public enum CollectMode {
+        /**
+         * Reports only one violation in total.
+         */
+        Single,
+        /**
+         * Reports one call path per privileged method used.
+         */
+        SinglePrivilegedMethodUsage,
+        /**
+         * Reports all call paths for all violations.
+         */
+        All
+    }
+
     public static class Options {
         @Option(help = "Path to file where to store report of Truffle language privilege access.")//
         public static final HostedOptionKey<String> TruffleTCKPermissionsReportFile = new HostedOptionKey<>(null);
@@ -151,6 +170,15 @@ public static class Options {
                           "Warn": Log a warning message to stderr.
                           "Throw" (default): Throw an exception and abort the native-image build process.""", type = OptionType.Expert)//
         public static final HostedOptionKey<ActionKind> TruffleTCKUnusedAllowListEntriesAction = new HostedOptionKey<>(ActionKind.Throw);
+
+        @Option(help = """
+                        Specifies how privileged method violations are reported.
+                        Available options:
+                          - Single: Reports only one violation in total.
+                          - SinglePrivilegedMethodUsage: Reports one call path per privileged method used.
+                          - All: Reports all call paths for all violations.
+                        """, type = OptionType.Expert)//
+        public static final HostedOptionKey<CollectMode> TruffleTCKCollectMode = new HostedOptionKey<>(CollectMode.SinglePrivilegedMethodUsage);
     }
 
     /**
@@ -346,6 +374,7 @@ public void afterAnalysis(AfterAnalysisAccess access) {
         }
         FeatureImpl.AfterAnalysisAccessImpl accessImpl = (FeatureImpl.AfterAnalysisAccessImpl) access;
         DebugContext debugContext = accessImpl.getDebugContext();
+        CollectMode collectMode = Options.TruffleTCKCollectMode.getValue();
         try (DebugContext.Scope s = debugContext.scope(ClassUtil.getUnqualifiedName(getClass()))) {
             BigBang bb = accessImpl.getBigBang();
             Map<BaseMethodNode, Set<BaseMethodNode>> cg = callGraph(bb, deniedMethods, debugContext, (SVMHost) bb.getHostVM());
@@ -356,8 +385,11 @@ public void afterAnalysis(AfterAnalysisAccess access) {
                 if (cg.containsKey(deniedMethod)) {
                     collectViolations(report, deniedMethod,
                                     maxStackDepth, Options.TruffleTCKPermissionsMaxErrors.getValue(),
-                                    cg, contextFilters,
+                                    cg, contextFilters, collectMode,
                                     new LinkedList<>(), new HashSet<>(), 1, 0);
+                    if (!report.isEmpty() && collectMode == CollectMode.Single) {
+                        break;
+                    }
                 }
             }
             if (!report.isEmpty()) {
@@ -367,7 +399,14 @@ public void afterAnalysis(AfterAnalysisAccess access) {
                                 (pw) -> {
                                     StringBuilder builder = new StringBuilder();
                                     for (List<BaseMethodNode> callPath : report) {
+                                        boolean privilegedMethod = true;
                                         for (BaseMethodNode call : callPath) {
+                                            if (privilegedMethod) {
+                                                builder.append("Illegal call to privileged method ");
+                                                privilegedMethod = false;
+                                            } else {
+                                                builder.append("    at ");
+                                            }
                                             builder.append(call.asStackTraceElement()).append(System.lineSeparator());
                                         }
                                         builder.append(System.lineSeparator());
@@ -569,6 +608,7 @@ private static boolean isBackTraceOverLanguageMethod(AnalysisMethodNode method,
      * @param callGraph call graph obtained from
      *            {@link PermissionsFeature#callGraph(BigBang, Set, DebugContext, SVMHost)}
      * @param contextFiltersParam filters removing known valid calls
+     * @param collectMode violation collect mode, see {@link Options#TruffleTCKCollectMode}
      * @param currentPath current path from a privileged method in a call graph
      * @param visited set of already visited methods, these methods are already part of an existing
      *            report or do not lead to language class
@@ -581,6 +621,7 @@ private int collectViolations(
                     int maxReports,
                     Map<BaseMethodNode, Set<BaseMethodNode>> callGraph,
                     Set<CallGraphFilter> contextFiltersParam,
+                    CollectMode collectMode,
                     List<BaseMethodNode> currentPath,
                     Set<BaseMethodNode> visited,
                     int depth,
@@ -599,7 +640,8 @@ private int collectViolations(
                 Set<BaseMethodNode> callers = callGraph.get(mNode);
                 if (depth > maxDepth) {
                     if (!callers.isEmpty()) {
-                        numReports = collectViolations(report, callers.iterator().next(), maxDepth, maxReports, callGraph, contextFiltersParam, currentPath, visited, depth + 1, numReports);
+                        numReports = collectViolations(report, callers.iterator().next(), maxDepth, maxReports, callGraph, contextFiltersParam, collectMode, currentPath, visited, depth + 1,
+                                        numReports);
                     }
                 } else if (!isSystemOrSafeClass(mNode)) {
                     List<BaseMethodNode> callPath = new ArrayList<>(currentPath);
@@ -608,7 +650,10 @@ private int collectViolations(
                 } else {
                     for (BaseMethodNode caller : callers) {
                         if (contextFiltersParam.stream().noneMatch((f) -> f.test(mNode, caller, currentPath))) {
-                            numReports = collectViolations(report, caller, maxDepth, maxReports, callGraph, contextFiltersParam, currentPath, visited, depth + 1, numReports);
+                            numReports = collectViolations(report, caller, maxDepth, maxReports, callGraph, contextFiltersParam, collectMode, currentPath, visited, depth + 1, numReports);
+                            if (numReports > initialNumReports && collectMode != CollectMode.All) {
+                                break;
+                            }
                         }
                     }
                 }

vm/mx.vm/mx_vm_gate.py

@@ -599,8 +599,11 @@ def gate_python(tasks):
             python_suite.extensions.run_python_unittests(python_svm_image_path)
 
 
-def _svm_truffle_tck(native_image, language_id, language_distribution=None, fail_on_error=True, print_call_tree=False):
+def _svm_truffle_tck(native_image, language_id, language_distribution=None, fail_on_error=True, print_call_tree=False,
+                     additional_options=None):
     assert language_distribution, 'Language_distribution must be given'
+    if additional_options is None:
+        additional_options = []
     dists = [
         mx.distribution('substratevm:SVM_TRUFFLE_TCK'),
         language_distribution
@@ -631,7 +634,7 @@ def _collect_excludes(suite, suite_import, excludes):
             f'-H:TruffleTCKPermissionsReportFile={report_file}',
             f'-H:Path={svmbuild}',
             '--add-exports=org.graalvm.truffle.runtime/com.oracle.truffle.runtime=ALL-UNNAMED'
-        ] + print_call_tree_options) + [
+        ] + print_call_tree_options + additional_options) + [
             'com.oracle.svm.truffle.tck.MockMain'
         ]
         if excludes:
@@ -642,6 +645,9 @@ def _collect_excludes(suite, suite_import, excludes):
             with open(report_file, "r") as f:
                 for line in f.readlines():
                     message = message + line
+            message = message + ("\nNote: If the method is not used directly by the language, but is part of the call path "
+                                 "because it is used internally by the JDK and introduced by a polymorphic call, consider "
+                                 "adding it to `jdk_allowed_methods.json`.")
             if fail_on_error:
                 mx.abort(message)
             else:
@@ -679,7 +685,8 @@ def _copy_call_tree(source_folder):
             test_language_dist = [d for d in truffle_suite.dists if d.name == 'TRUFFLE_TCK_TESTS_LANGUAGE'][0]
             native_image_context, svm = graalvm_svm()
             with native_image_context(svm.IMAGE_ASSERTION_FLAGS) as native_image:
-                result = _svm_truffle_tck(native_image, 'TCKSmokeTestLanguage', test_language_dist, False, True)
+                result = _svm_truffle_tck(native_image, 'TCKSmokeTestLanguage', test_language_dist, False, True,
+                                          ['-H:TruffleTCKCollectMode=All'])
                 privileged_calls = result[0]
                 reports_folder = result[1]
                 if not 'Failed: Language TCKSmokeTestLanguage performs following privileged calls' in privileged_calls: