Skip to content

Commit cb3dc44

Browse files
dougxcdougxc
committed
[GR-63115] Disable ObjdumpDisassemblerProvider in native image.
PullRequest: graal/20368
2 parents a5850e9 + 450eb38 commit cb3dc44

File tree

8 files changed

+145
-75
lines changed

8 files changed

+145
-75
lines changed

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ public static boolean testSnippet1() {
9999

100100
@Test
101101
public void test1() {
102-
// OptionValues options = new OptionValues(getInitialOptions(), HighTier.Options.Inline,
103-
// false);
104-
OptionValues options = getInitialOptions();
105-
test(options, "testSnippet1");
102+
test("testSnippet1");
106103
}
107104

108105
@Override

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

Lines changed: 73 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@
3636
import java.util.regex.Pattern;
3737

3838
import jdk.graal.compiler.code.CompilationResult.CodeAnnotation;
39+
import jdk.graal.compiler.core.common.NativeImageSupport;
40+
import jdk.graal.compiler.debug.GraalError;
41+
import jdk.graal.compiler.debug.TTY;
3942
import jdk.graal.compiler.options.Option;
4043
import jdk.graal.compiler.options.OptionKey;
4144
import jdk.graal.compiler.options.OptionType;
@@ -72,13 +75,14 @@ static class Options {
7275
}
7376

7477
// cached validity of candidate objdump executables.
75-
private Map<String, Boolean> objdumpCache = new HashMap<>();
78+
private static final Map<String, Boolean> objdumpCache = new HashMap<>();
7679

7780
private static Process createProcess(String[] cmd) {
7881
ProcessBuilder pb = new ProcessBuilder(cmd);
7982
try {
8083
return pb.start();
8184
} catch (IOException e) {
85+
TTY.printf("WARNING: Error executing '%s' (%s)%n", String.join(" ", cmd), e);
8286
}
8387
return null;
8488
}
@@ -90,6 +94,9 @@ public boolean isAvailable(OptionValues options) {
9094

9195
@Override
9296
public String disassembleCompiledCode(OptionValues options, CodeCacheProvider codeCache, CompilationResult compResult) {
97+
if (NativeImageSupport.inRuntimeCode() && !ENABLE_OBJDUMP) {
98+
throw new GraalError("Objdump not available");
99+
}
93100
String objdump = getObjdump(options);
94101
if (objdump == null) {
95102
return null;
@@ -129,8 +136,7 @@ public String disassembleCompiledCode(OptionValues options, CodeCacheProvider co
129136
putAnnotation(annotations, a.getPosition(), a.toString());
130137
}
131138
for (Infopoint infopoint : compResult.getInfopoints()) {
132-
if (infopoint instanceof Call) {
133-
Call call = (Call) infopoint;
139+
if (infopoint instanceof Call call) {
134140
if (call.debugInfo != null) {
135141
putAnnotation(annotations, call.pcOffset + call.size, CodeUtil.append(new StringBuilder(100), call.debugInfo, slotFormatter).toString());
136142
}
@@ -170,15 +176,14 @@ public String disassembleCompiledCode(OptionValues options, CodeCacheProvider co
170176
String errLine = ebr.readLine();
171177
if (errLine != null) {
172178
System.err.println("Error output from executing: " + CollectionsUtil.mapAndJoin(cmdline, e -> quoteShellArg(String.valueOf(e)), " "));
173-
System.err.println(errLine);
174-
while ((errLine = ebr.readLine()) != null) {
179+
do {
175180
System.err.println(errLine);
176-
}
181+
} while ((errLine = ebr.readLine()) != null);
177182
}
178183
}
179184
return sb.toString();
180185
} catch (IOException e) {
181-
e.printStackTrace();
186+
e.printStackTrace(TTY.out);
182187
return null;
183188
} finally {
184189
if (tmp != null) {
@@ -188,9 +193,9 @@ public String disassembleCompiledCode(OptionValues options, CodeCacheProvider co
188193
}
189194

190195
/**
191-
* Pattern for a single shell command argument that does not need to quoted.
196+
* Pattern for a single shell command argument that does not need to be quoted.
192197
*/
193-
private static final Pattern SAFE_SHELL_ARG = Pattern.compile("[A-Za-z0-9@%_\\-\\+=:,\\./]+");
198+
private static final Pattern SAFE_SHELL_ARG = Pattern.compile("[A-Za-z0-9@%_\\-+=:,./]+");
194199

195200
/**
196201
* Reliably quote a string as a single shell command argument.
@@ -207,52 +212,78 @@ public static String quoteShellArg(String arg) {
207212
return "'" + arg.replace("'", "'\"'\"'") + "'";
208213
}
209214

215+
private static final String ENABLE_OBJDUMP_PROP = "debug.jdk.graal.enableObjdump";
216+
217+
/**
218+
* Support for objdump is excluded by default from native images (including libgraal) to reduce
219+
* the image size. It also reduces security concerns related to running subprocesses.
220+
*
221+
* To objdump during development, set the {@value #ENABLE_OBJDUMP_PROP} system property to true
222+
* when building native images.
223+
*/
224+
private static final boolean ENABLE_OBJDUMP = Boolean.parseBoolean(GraalServices.getSavedProperty(ENABLE_OBJDUMP_PROP));
225+
226+
private static boolean objdumpUnsupportedWarned;
227+
210228
/**
211229
* Searches for a valid GNU objdump executable.
212230
*/
213-
private String getObjdump(OptionValues options) {
231+
private static String getObjdump(OptionValues options) {
214232
// for security, user must provide the possible objdump locations.
215233
String candidates = Options.ObjdumpExecutables.getValue(options);
216234
if (candidates != null && !candidates.isEmpty()) {
235+
if (NativeImageSupport.inRuntimeCode() && !ENABLE_OBJDUMP) {
236+
if (!objdumpUnsupportedWarned) {
237+
// Ignore races or multiple isolates - an extra warning is ok
238+
objdumpUnsupportedWarned = true;
239+
TTY.printf("WARNING: Objdump not supported as the %s system property was false when building.%n",
240+
ENABLE_OBJDUMP_PROP);
241+
}
242+
return null;
243+
}
244+
217245
for (String candidate : candidates.split(",")) {
218-
// first checking to see if a cached verdict for this candidate exists.
219-
Boolean cachedQuery = objdumpCache.get(candidate);
220-
if (cachedQuery != null) {
221-
if (cachedQuery.booleanValue()) {
222-
return candidate;
223-
} else {
224-
// this candidate was previously determined to not be acceptable.
225-
continue;
246+
synchronized (objdumpCache) {
247+
// first checking to see if a cached verdict for this candidate exists.
248+
Boolean cachedQuery = objdumpCache.get(candidate);
249+
if (cachedQuery != null) {
250+
if (cachedQuery) {
251+
return candidate;
252+
} else {
253+
// this candidate was previously determined to not be acceptable.
254+
continue;
255+
}
226256
}
227-
}
228-
try {
229257
String[] cmd = {candidate, "--version"};
230-
Process proc = createProcess(cmd);
231-
if (proc == null) {
232-
// bad candidate.
233-
objdumpCache.put(candidate, Boolean.FALSE);
234-
return null;
235-
}
236-
InputStream is = proc.getInputStream();
237-
int exitValue = proc.waitFor();
238-
if (exitValue == 0) {
239-
byte[] buf = new byte[is.available()];
240-
int pos = 0;
241-
while (pos < buf.length) {
242-
int read = is.read(buf, pos, buf.length - pos);
243-
pos += read;
258+
try {
259+
Process proc = createProcess(cmd);
260+
if (proc == null) {
261+
// bad candidate.
262+
objdumpCache.put(candidate, Boolean.FALSE);
263+
return null;
244264
}
245-
String output = new String(buf);
246-
if (output.contains("GNU objdump")) {
247-
// this candidate meets the criteria.
248-
objdumpCache.put(candidate, Boolean.TRUE);
249-
return candidate;
265+
InputStream is = proc.getInputStream();
266+
int exitValue = proc.waitFor();
267+
if (exitValue == 0) {
268+
byte[] buf = new byte[is.available()];
269+
int pos = 0;
270+
while (pos < buf.length) {
271+
int read = is.read(buf, pos, buf.length - pos);
272+
pos += read;
273+
}
274+
String output = new String(buf);
275+
if (output.contains("GNU objdump")) {
276+
// this candidate meets the criteria.
277+
objdumpCache.put(candidate, Boolean.TRUE);
278+
return candidate;
279+
}
250280
}
281+
} catch (IOException | InterruptedException e) {
282+
TTY.printf("WARNING: Error reading input from '%s' (%s)%n", String.join(" ", cmd), e);
251283
}
252-
} catch (IOException | InterruptedException e) {
284+
// bad candidate.
285+
objdumpCache.put(candidate, Boolean.FALSE);
253286
}
254-
// bad candidate.
255-
objdumpCache.put(candidate, Boolean.FALSE);
256287
}
257288
}
258289
return null;

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ public void close() {
212212
/**
213213
* Signal the execution of the phase identified by {@code name} starts.
214214
*/
215-
public void enterPhase(String name) {
215+
public void enterPhase(CharSequence name) {
216216
if (!isEnabled()) {
217217
return;
218218
}
@@ -226,7 +226,7 @@ public void enterPhase(String name) {
226226
/**
227227
* Signal the execution of the phase identified by {@code name} is over.
228228
*/
229-
public void exitPhase(String name) {
229+
public void exitPhase(CharSequence name) {
230230
if (!isEnabled()) {
231231
return;
232232
}
@@ -237,7 +237,7 @@ public void exitPhase(String name) {
237237
currentNode = currentNode.parent;
238238
}
239239

240-
private void setCurrentNodeDuration(String name) {
240+
private void setCurrentNodeDuration(CharSequence name) {
241241
assert currentNode.startTimeNS >= 0 : Assertions.errorMessage("Must have a positive start time", name, elapsedPhaseTreeAsString());
242242
currentNode.durationNS = System.nanoTime() - currentNode.startTimeNS;
243243
}
@@ -278,7 +278,7 @@ private class PhaseTreeNode {
278278
/**
279279
* The name of this node, normally the {@link BasePhase#contractorName()}.
280280
*/
281-
private final String name;
281+
private final CharSequence name;
282282

283283
/**
284284
* The time stamp in ns when this phase started running.
@@ -295,7 +295,7 @@ private class PhaseTreeNode {
295295
*/
296296
public boolean closed;
297297

298-
PhaseTreeNode(String name) {
298+
PhaseTreeNode(CharSequence name) {
299299
this.name = name;
300300
}
301301

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,17 +456,17 @@ public interface CompilerPhaseScope extends AutoCloseable {
456456
* ends
457457
*/
458458
public CompilerPhaseScope enterCompilerPhase(CharSequence phaseName) {
459-
CompilationAlarm.current().enterPhase(phaseName.toString());
459+
CompilationAlarm.current().enterPhase(phaseName);
460460
if (compilationListener != null) {
461461
return new DecoratingCompilerPhaseScope(() -> {
462-
CompilationAlarm.current().exitPhase(phaseName.toString());
462+
CompilationAlarm.current().exitPhase(phaseName);
463463
}, enterCompilerPhase(() -> phaseName));
464464
}
465465
return new CompilerPhaseScope() {
466466

467467
@Override
468468
public void close() {
469-
CompilationAlarm.current().exitPhase(phaseName.toString());
469+
CompilationAlarm.current().exitPhase(phaseName);
470470
}
471471
};
472472
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ final class IgvDumpChannel implements WritableByteChannel {
5050
private static final String ENABLE_NETWORK_DUMPING_PROP = "debug.jdk.graal.enableNetworkDumping";
5151

5252
/**
53-
* Support for IGV dumping to a network port is excluded by default from libgraal to reduce the
54-
* libgraal image size. It also reduces security concerns related to opening random network
55-
* connections.
53+
* Support for IGV dumping to a network port is excluded by default from native images
54+
* (including libgraal) to reduce the image size. It also reduces security concerns related to
55+
* opening random network connections.
5656
*
57-
* To enable IGV dumping to the network during libgraal based development, set the
58-
* {@value #ENABLE_NETWORK_DUMPING_PROP} system property to true when building libgraal.
57+
* To enable IGV dumping to the network during development, set the
58+
* {@value #ENABLE_NETWORK_DUMPING_PROP} system property to true when building native images.
5959
*/
6060
private static final boolean ENABLE_NETWORK_DUMPING = Boolean.parseBoolean(GraalServices.getSavedProperty(ENABLE_NETWORK_DUMPING_PROP));
6161

@@ -107,7 +107,7 @@ WritableByteChannel channel() throws IOException {
107107
if (!networkDumpingUnsupportedWarned) {
108108
// Ignore races or multiple isolates - an extra warning is ok
109109
networkDumpingUnsupportedWarned = true;
110-
TTY.printf("WARNING: Graph dumping to network not supported as the %s system property was false when building libgraal - dumping to file instead.%n",
110+
TTY.printf("WARNING: Graph dumping to network not supported as the %s system property was false when building - dumping to file instead.%n",
111111
ENABLE_NETWORK_DUMPING_PROP);
112112
}
113113
sharedChannel = createFileChannel(pathProvider, null);

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,18 +94,38 @@ private static void addProviders(String arch, Class<?> service) {
9494
// Skip provider for another architecture
9595
continue;
9696
}
97+
if (provider.getClass().getAnnotation(LibGraalSupport.HostedOnly.class) != null) {
98+
// Skip hosted-only providers
99+
continue;
100+
}
97101
providers.add(provider);
98102
}
99103
}
100104

105+
/**
106+
* Determines if {@code c} is annotated by {@link LibGraalService}.
107+
*/
108+
static boolean isLibGraalService(Class<?> c) {
109+
if (c != null && c.getAnnotation(LibGraalService.class) != null) {
110+
if (c.getAnnotation(LibGraalSupport.HostedOnly.class) != null) {
111+
throw new GraalError("Class %s cannot be annotated by both %s and %s as they are mutually exclusive)",
112+
c.getName(),
113+
LibGraalService.class.getName(),
114+
LibGraalSupport.HostedOnly.class.getName());
115+
}
116+
return true;
117+
}
118+
return false;
119+
}
120+
101121
static {
102122
LibGraalSupport libgraal = LibGraalSupport.INSTANCE;
103123
if (libgraal != null) {
104124
libgraalServices = new HashMap<>();
105125
String arch = getJVMCIArch();
106126
libgraal.getClassModuleMap().keySet().stream()//
107127
.map(GraalServices::loadClassOrNull)//
108-
.filter(c -> c != null && c.getAnnotation(LibGraalService.class) != null)//
128+
.filter(GraalServices::isLibGraalService)//
109129
.forEach(service -> addProviders(arch, service));
110130
} else {
111131
libgraalServices = null;

0 commit comments

Comments
 (0)