[GR-63115] Disable ObjdumpDisassemblerProvider in native image.

PullRequest: graal/20368

Author: dougxc

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/StackSlotLimitTest.java

@@ -99,10 +99,7 @@ public static boolean testSnippet1() {
 
     @Test
     public void test1() {
-        // OptionValues options = new OptionValues(getInitialOptions(), HighTier.Options.Inline,
-        // false);
-        OptionValues options = getInitialOptions();
-        test(options, "testSnippet1");
+        test("testSnippet1");
     }
 
     @Override

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/code/ObjdumpDisassemblerProvider.java

@@ -36,6 +36,9 @@
 import java.util.regex.Pattern;
 
 import jdk.graal.compiler.code.CompilationResult.CodeAnnotation;
+import jdk.graal.compiler.core.common.NativeImageSupport;
+import jdk.graal.compiler.debug.GraalError;
+import jdk.graal.compiler.debug.TTY;
 import jdk.graal.compiler.options.Option;
 import jdk.graal.compiler.options.OptionKey;
 import jdk.graal.compiler.options.OptionType;
@@ -72,13 +75,14 @@ static class Options {
     }
 
     // cached validity of candidate objdump executables.
-    private Map<String, Boolean> objdumpCache = new HashMap<>();
+    private static final Map<String, Boolean> objdumpCache = new HashMap<>();
 
     private static Process createProcess(String[] cmd) {
         ProcessBuilder pb = new ProcessBuilder(cmd);
         try {
             return pb.start();
         } catch (IOException e) {
+            TTY.printf("WARNING: Error executing '%s' (%s)%n", String.join(" ", cmd), e);
         }
         return null;
     }
@@ -90,6 +94,9 @@ public boolean isAvailable(OptionValues options) {
 
     @Override
     public String disassembleCompiledCode(OptionValues options, CodeCacheProvider codeCache, CompilationResult compResult) {
+        if (NativeImageSupport.inRuntimeCode() && !ENABLE_OBJDUMP) {
+            throw new GraalError("Objdump not available");
+        }
         String objdump = getObjdump(options);
         if (objdump == null) {
             return null;
@@ -129,8 +136,7 @@ public String disassembleCompiledCode(OptionValues options, CodeCacheProvider co
                 putAnnotation(annotations, a.getPosition(), a.toString());
             }
             for (Infopoint infopoint : compResult.getInfopoints()) {
-                if (infopoint instanceof Call) {
-                    Call call = (Call) infopoint;
+                if (infopoint instanceof Call call) {
                     if (call.debugInfo != null) {
                         putAnnotation(annotations, call.pcOffset + call.size, CodeUtil.append(new StringBuilder(100), call.debugInfo, slotFormatter).toString());
                     }
@@ -170,15 +176,14 @@ public String disassembleCompiledCode(OptionValues options, CodeCacheProvider co
                 String errLine = ebr.readLine();
                 if (errLine != null) {
                     System.err.println("Error output from executing: " + CollectionsUtil.mapAndJoin(cmdline, e -> quoteShellArg(String.valueOf(e)), " "));
-                    System.err.println(errLine);
-                    while ((errLine = ebr.readLine()) != null) {
+                    do {
                         System.err.println(errLine);
-                    }
+                    } while ((errLine = ebr.readLine()) != null);
                 }
             }
             return sb.toString();
         } catch (IOException e) {
-            e.printStackTrace();
+            e.printStackTrace(TTY.out);
             return null;
         } finally {
             if (tmp != null) {
@@ -188,9 +193,9 @@ public String disassembleCompiledCode(OptionValues options, CodeCacheProvider co
     }
 
     /**
-     * Pattern for a single shell command argument that does not need to quoted.
+     * Pattern for a single shell command argument that does not need to be quoted.
      */
-    private static final Pattern SAFE_SHELL_ARG = Pattern.compile("[A-Za-z0-9@%_\\-\\+=:,\\./]+");
+    private static final Pattern SAFE_SHELL_ARG = Pattern.compile("[A-Za-z0-9@%_\\-+=:,./]+");
 
     /**
      * Reliably quote a string as a single shell command argument.
@@ -207,52 +212,78 @@ public static String quoteShellArg(String arg) {
         return "'" + arg.replace("'", "'\"'\"'") + "'";
     }
 
+    private static final String ENABLE_OBJDUMP_PROP = "debug.jdk.graal.enableObjdump";
+
+    /**
+     * Support for objdump is excluded by default from native images (including libgraal) to reduce
+     * the image size. It also reduces security concerns related to running subprocesses.
+     *
+     * To objdump during development, set the {@value #ENABLE_OBJDUMP_PROP} system property to true
+     * when building native images.
+     */
+    private static final boolean ENABLE_OBJDUMP = Boolean.parseBoolean(GraalServices.getSavedProperty(ENABLE_OBJDUMP_PROP));
+
+    private static boolean objdumpUnsupportedWarned;
+
     /**
      * Searches for a valid GNU objdump executable.
      */
-    private String getObjdump(OptionValues options) {
+    private static String getObjdump(OptionValues options) {
         // for security, user must provide the possible objdump locations.
         String candidates = Options.ObjdumpExecutables.getValue(options);
         if (candidates != null && !candidates.isEmpty()) {
+            if (NativeImageSupport.inRuntimeCode() && !ENABLE_OBJDUMP) {
+                if (!objdumpUnsupportedWarned) {
+                    // Ignore races or multiple isolates - an extra warning is ok
+                    objdumpUnsupportedWarned = true;
+                    TTY.printf("WARNING: Objdump not supported as the %s system property was false when building.%n",
+                                    ENABLE_OBJDUMP_PROP);
+                }
+                return null;
+            }
+
             for (String candidate : candidates.split(",")) {
-                // first checking to see if a cached verdict for this candidate exists.
-                Boolean cachedQuery = objdumpCache.get(candidate);
-                if (cachedQuery != null) {
-                    if (cachedQuery.booleanValue()) {
-                        return candidate;
-                    } else {
-                        // this candidate was previously determined to not be acceptable.
-                        continue;
+                synchronized (objdumpCache) {
+                    // first checking to see if a cached verdict for this candidate exists.
+                    Boolean cachedQuery = objdumpCache.get(candidate);
+                    if (cachedQuery != null) {
+                        if (cachedQuery) {
+                            return candidate;
+                        } else {
+                            // this candidate was previously determined to not be acceptable.
+                            continue;
+                        }
                     }
-                }
-                try {
                     String[] cmd = {candidate, "--version"};
-                    Process proc = createProcess(cmd);
-                    if (proc == null) {
-                        // bad candidate.
-                        objdumpCache.put(candidate, Boolean.FALSE);
-                        return null;
-                    }
-                    InputStream is = proc.getInputStream();
-                    int exitValue = proc.waitFor();
-                    if (exitValue == 0) {
-                        byte[] buf = new byte[is.available()];
-                        int pos = 0;
-                        while (pos < buf.length) {
-                            int read = is.read(buf, pos, buf.length - pos);
-                            pos += read;
+                    try {
+                        Process proc = createProcess(cmd);
+                        if (proc == null) {
+                            // bad candidate.
+                            objdumpCache.put(candidate, Boolean.FALSE);
+                            return null;
                         }
-                        String output = new String(buf);
-                        if (output.contains("GNU objdump")) {
-                            // this candidate meets the criteria.
-                            objdumpCache.put(candidate, Boolean.TRUE);
-                            return candidate;
+                        InputStream is = proc.getInputStream();
+                        int exitValue = proc.waitFor();
+                        if (exitValue == 0) {
+                            byte[] buf = new byte[is.available()];
+                            int pos = 0;
+                            while (pos < buf.length) {
+                                int read = is.read(buf, pos, buf.length - pos);
+                                pos += read;
+                            }
+                            String output = new String(buf);
+                            if (output.contains("GNU objdump")) {
+                                // this candidate meets the criteria.
+                                objdumpCache.put(candidate, Boolean.TRUE);
+                                return candidate;
+                            }
                         }
+                    } catch (IOException | InterruptedException e) {
+                        TTY.printf("WARNING: Error reading input from '%s' (%s)%n", String.join(" ", cmd), e);
                     }
-                } catch (IOException | InterruptedException e) {
+                    // bad candidate.
+                    objdumpCache.put(candidate, Boolean.FALSE);
                 }
-                // bad candidate.
-                objdumpCache.put(candidate, Boolean.FALSE);
             }
         }
         return null;

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/common/util/CompilationAlarm.java

@@ -212,7 +212,7 @@ public void close() {
     /**
      * Signal the execution of the phase identified by {@code name} starts.
      */
-    public void enterPhase(String name) {
+    public void enterPhase(CharSequence name) {
         if (!isEnabled()) {
             return;
         }
@@ -226,7 +226,7 @@ public void enterPhase(String name) {
     /**
      * Signal the execution of the phase identified by {@code name} is over.
      */
-    public void exitPhase(String name) {
+    public void exitPhase(CharSequence name) {
         if (!isEnabled()) {
             return;
         }
@@ -237,7 +237,7 @@ public void exitPhase(String name) {
         currentNode = currentNode.parent;
     }
 
-    private void setCurrentNodeDuration(String name) {
+    private void setCurrentNodeDuration(CharSequence name) {
         assert currentNode.startTimeNS >= 0 : Assertions.errorMessage("Must have a positive start time", name, elapsedPhaseTreeAsString());
         currentNode.durationNS = System.nanoTime() - currentNode.startTimeNS;
     }
@@ -278,7 +278,7 @@ private class PhaseTreeNode {
         /**
          * The name of this node, normally the {@link BasePhase#contractorName()}.
          */
-        private final String name;
+        private final CharSequence name;
 
         /**
          * The time stamp in ns when this phase started running.
@@ -295,7 +295,7 @@ private class PhaseTreeNode {
          */
         public boolean closed;
 
-        PhaseTreeNode(String name) {
+        PhaseTreeNode(CharSequence name) {
             this.name = name;
         }
 

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/DebugContext.java

@@ -456,17 +456,17 @@ public interface CompilerPhaseScope extends AutoCloseable {
      *         ends
      */
     public CompilerPhaseScope enterCompilerPhase(CharSequence phaseName) {
-        CompilationAlarm.current().enterPhase(phaseName.toString());
+        CompilationAlarm.current().enterPhase(phaseName);
         if (compilationListener != null) {
             return new DecoratingCompilerPhaseScope(() -> {
-                CompilationAlarm.current().exitPhase(phaseName.toString());
+                CompilationAlarm.current().exitPhase(phaseName);
             }, enterCompilerPhase(() -> phaseName));
         }
         return new CompilerPhaseScope() {
 
             @Override
             public void close() {
-                CompilationAlarm.current().exitPhase(phaseName.toString());
+                CompilationAlarm.current().exitPhase(phaseName);
             }
         };
     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/IgvDumpChannel.java

@@ -50,12 +50,12 @@ final class IgvDumpChannel implements WritableByteChannel {
     private static final String ENABLE_NETWORK_DUMPING_PROP = "debug.jdk.graal.enableNetworkDumping";
 
     /**
-     * Support for IGV dumping to a network port is excluded by default from libgraal to reduce the
-     * libgraal image size. It also reduces security concerns related to opening random network
-     * connections.
+     * Support for IGV dumping to a network port is excluded by default from native images
+     * (including libgraal) to reduce the image size. It also reduces security concerns related to
+     * opening random network connections.
      *
-     * To enable IGV dumping to the network during libgraal based development, set the
-     * {@value #ENABLE_NETWORK_DUMPING_PROP} system property to true when building libgraal.
+     * To enable IGV dumping to the network during development, set the
+     * {@value #ENABLE_NETWORK_DUMPING_PROP} system property to true when building native images.
      */
     private static final boolean ENABLE_NETWORK_DUMPING = Boolean.parseBoolean(GraalServices.getSavedProperty(ENABLE_NETWORK_DUMPING_PROP));
 
@@ -107,7 +107,7 @@ WritableByteChannel channel() throws IOException {
                     if (!networkDumpingUnsupportedWarned) {
                         // Ignore races or multiple isolates - an extra warning is ok
                         networkDumpingUnsupportedWarned = true;
-                        TTY.printf("WARNING: Graph dumping to network not supported as the %s system property was false when building libgraal - dumping to file instead.%n",
+                        TTY.printf("WARNING: Graph dumping to network not supported as the %s system property was false when building - dumping to file instead.%n",
                                         ENABLE_NETWORK_DUMPING_PROP);
                     }
                     sharedChannel = createFileChannel(pathProvider, null);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/serviceprovider/GraalServices.java

@@ -94,18 +94,38 @@ private static void addProviders(String arch, Class<?> service) {
                 // Skip provider for another architecture
                 continue;
             }
+            if (provider.getClass().getAnnotation(LibGraalSupport.HostedOnly.class) != null) {
+                // Skip hosted-only providers
+                continue;
+            }
             providers.add(provider);
         }
     }
 
+    /**
+     * Determines if {@code c} is annotated by {@link LibGraalService}.
+     */
+    static boolean isLibGraalService(Class<?> c) {
+        if (c != null && c.getAnnotation(LibGraalService.class) != null) {
+            if (c.getAnnotation(LibGraalSupport.HostedOnly.class) != null) {
+                throw new GraalError("Class %s cannot be annotated by both %s and %s as they are mutually exclusive)",
+                                c.getName(),
+                                LibGraalService.class.getName(),
+                                LibGraalSupport.HostedOnly.class.getName());
+            }
+            return true;
+        }
+        return false;
+    }
+
     static {
         LibGraalSupport libgraal = LibGraalSupport.INSTANCE;
         if (libgraal != null) {
             libgraalServices = new HashMap<>();
             String arch = getJVMCIArch();
             libgraal.getClassModuleMap().keySet().stream()//
                             .map(GraalServices::loadClassOrNull)//
-                            .filter(c -> c != null && c.getAnnotation(LibGraalService.class) != null)//
+                            .filter(GraalServices::isLibGraalService)//
                             .forEach(service -> addProviders(arch, service));
         } else {
             libgraalServices = null;