Skip to content

Commit 95ff8d4

Browse files
dougxcdougxc
committed
removed Java version checks from code that may run in libgraal class loader at libgraal build time
1 parent d132578 commit 95ff8d4

File tree

17 files changed

+136
-174
lines changed

17 files changed

+136
-174
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/CheckGraalInvariants.java‎

Lines changed: 75 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,15 @@
3232
import java.lang.reflect.Field;
3333
import java.lang.reflect.Method;
3434
import java.lang.reflect.Modifier;
35+
import java.net.MalformedURLException;
36+
import java.net.URL;
37+
import java.net.URLClassLoader;
3538
import java.nio.file.Files;
3639
import java.nio.file.Path;
3740
import java.util.ArrayList;
3841
import java.util.Arrays;
3942
import java.util.Collections;
43+
import java.util.Comparator;
4044
import java.util.Enumeration;
4145
import java.util.HashMap;
4246
import java.util.HashSet;
@@ -48,6 +52,8 @@
4852
import java.util.concurrent.ThreadPoolExecutor;
4953
import java.util.concurrent.TimeUnit;
5054
import java.util.function.Supplier;
55+
import java.util.stream.Collectors;
56+
import java.util.stream.Stream;
5157
import java.util.zip.ZipEntry;
5258
import java.util.zip.ZipFile;
5359

@@ -109,7 +115,6 @@
109115
import jdk.vm.ci.code.BailoutException;
110116
import jdk.vm.ci.code.Register;
111117
import jdk.vm.ci.code.Register.RegisterCategory;
112-
import jdk.vm.ci.hotspot.HotSpotConstantPool;
113118
import jdk.vm.ci.meta.ConstantPool;
114119
import jdk.vm.ci.meta.JavaField;
115120
import jdk.vm.ci.meta.JavaMethod;
@@ -151,7 +156,7 @@ public static void main(String[] args) {
151156

152157
public static String relativeFileName(String absolutePath) {
153158
int lastFileSeparatorIndex = absolutePath.lastIndexOf(File.separator);
154-
return absolutePath.substring(lastFileSeparatorIndex >= 0 ? lastFileSeparatorIndex : 0);
159+
return absolutePath.substring(Math.max(lastFileSeparatorIndex, 0));
155160
}
156161

157162
public static class InvariantsTool {
@@ -162,27 +167,41 @@ protected boolean shouldProcess(String classpathEntry) {
162167
}
163168
if (classpathEntry.endsWith(".jar")) {
164169
String name = new File(classpathEntry).getName();
165-
return name.contains("graal") || name.contains("jdk.graal.compiler");
170+
return name.contains("graal");
166171
}
167172
return false;
168173
}
169174

170-
protected String getClassPath() {
171-
String classpath;
172-
classpath = JRT_CLASS_PATH_ENTRY;
173-
String upgradeModulePath = System.getProperty("jdk.module.upgrade.path");
174-
if (upgradeModulePath != null) {
175-
classpath += File.pathSeparator + upgradeModulePath;
176-
}
177-
178-
// Also process classes that go into the libgraal native image.
175+
Path getLibgraalJar() {
176+
assert shouldVerifyLibGraalInvariants();
179177
String javaClassPath = System.getProperty("java.class.path");
180178
if (javaClassPath != null) {
181-
for (String path : javaClassPath.split(File.pathSeparator)) {
182-
if (path.contains("libgraal") && !path.contains("processor") && !path.contains("management")) {
183-
classpath += File.pathSeparator + path;
179+
String[] jcp = javaClassPath.split(File.pathSeparator);
180+
for (String s : jcp) {
181+
Path path = Path.of(s);
182+
if (s.endsWith(".jar")) {
183+
Path libgraal = path.getParent().resolve("libgraal.jar");
184+
if (Files.exists(libgraal)) {
185+
return libgraal;
186+
}
184187
}
185188
}
189+
throw new AssertionError(String.format("Could not find libgraal.jar as sibling of a jar on java.class.path:%n %s",
190+
Stream.of(jcp).sorted().collect(Collectors.joining("\n "))));
191+
}
192+
throw new AssertionError("The java.class.path system property is missing");
193+
}
194+
195+
protected List<String> getClassPath() {
196+
List<String> classpath = new ArrayList<>();
197+
classpath.add(JRT_CLASS_PATH_ENTRY);
198+
String upgradeModulePath = System.getProperty("jdk.module.upgrade.path");
199+
if (upgradeModulePath != null) {
200+
classpath.addAll(List.of(upgradeModulePath.split(File.pathSeparator)));
201+
}
202+
203+
if (shouldVerifyLibGraalInvariants()) {
204+
classpath.add(getLibgraalJar().toString());
186205
}
187206
return classpath;
188207
}
@@ -194,6 +213,10 @@ protected boolean shouldLoadClass(String className) {
194213
return true;
195214
}
196215

216+
public boolean shouldVerifyLibGraalInvariants() {
217+
return true;
218+
}
219+
197220
public boolean shouldVerifyFoldableMethods() {
198221
return true;
199222
}
@@ -253,11 +276,11 @@ public static void runTest(InvariantsTool tool) {
253276

254277
Assume.assumeTrue(VerifyPhase.class.desiredAssertionStatus());
255278

256-
String bootclasspath = tool.getClassPath();
257-
Assert.assertNotNull("Cannot find boot class path", bootclasspath);
279+
List<String> classPath = tool.getClassPath();
280+
Assert.assertNotNull("Cannot find class path", classPath);
258281

259282
final List<String> classNames = new ArrayList<>();
260-
for (String path : bootclasspath.split(File.pathSeparator)) {
283+
for (String path : classPath) {
261284
if (tool.shouldProcess(path)) {
262285
try {
263286
if (path.equals(JRT_CLASS_PATH_ENTRY)) {
@@ -284,16 +307,17 @@ public static void runTest(InvariantsTool tool) {
284307
}
285308
});
286309
} else {
287-
final ZipFile zipFile = new ZipFile(file);
288-
for (final Enumeration<? extends ZipEntry> entry = zipFile.entries(); entry.hasMoreElements();) {
289-
final ZipEntry zipEntry = entry.nextElement();
290-
String name = zipEntry.getName();
291-
if (name.endsWith(".class") && !name.startsWith("META-INF/versions/")) {
292-
String className = name.substring(0, name.length() - ".class".length()).replace('/', '.');
293-
if (isInNativeImage(className) || isGSON(className) || isONNX(className)) {
294-
continue;
310+
try (ZipFile zipFile = new ZipFile(file)) {
311+
for (final Enumeration<? extends ZipEntry> entry = zipFile.entries(); entry.hasMoreElements();) {
312+
final ZipEntry zipEntry = entry.nextElement();
313+
String name = zipEntry.getName();
314+
if (name.endsWith(".class") && !name.startsWith("META-INF/versions/")) {
315+
String className = name.substring(0, name.length() - ".class".length()).replace('/', '.');
316+
if (isInNativeImage(className) || isGSON(className) || isONNX(className)) {
317+
continue;
318+
}
319+
classNames.add(className);
295320
}
296-
classNames.add(className);
297321
}
298322
}
299323
}
@@ -303,7 +327,7 @@ public static void runTest(InvariantsTool tool) {
303327
}
304328
}
305329
}
306-
Assert.assertFalse("Could not find graal jars on boot class path: " + bootclasspath, classNames.isEmpty());
330+
Assert.assertFalse("Could not find graal jars on class path: " + classPath, classNames.isEmpty());
307331

308332
// Allows a subset of methods to be checked through use of a system property
309333
String property = System.getProperty(CheckGraalInvariants.class.getName() + ".filters");
@@ -336,7 +360,6 @@ public static void runTest(InvariantsTool tool) {
336360
verifiers.add(new VerifyDebugUsage());
337361
verifiers.add(new VerifyVirtualizableUsage());
338362
verifiers.add(new VerifyUpdateUsages());
339-
verifiers.add(new VerifyLibGraalContextChecks());
340363
verifiers.add(new VerifyWordFactoryUsage());
341364
verifiers.add(new VerifyBailoutUsage());
342365
verifiers.add(new VerifySystemPropertyUsage());
@@ -357,7 +380,6 @@ public static void runTest(InvariantsTool tool) {
357380
verifiers.add(new VerifyStringCaseUsage());
358381
verifiers.add(new VerifyMathAbs());
359382
verifiers.add(new VerifyLoopInfo());
360-
verifiers.add(new VerifyRuntimeVersionFeature());
361383
verifiers.add(new VerifyGuardsStageUsages());
362384
verifiers.add(new VerifyAArch64RegisterUsages());
363385
VerifyAssertionUsage assertionUsages = null;
@@ -368,6 +390,10 @@ public static void runTest(InvariantsTool tool) {
368390
verifiers.add(assertionUsages);
369391
}
370392

393+
if (tool.shouldVerifyLibGraalInvariants()) {
394+
verifiers.add(new VerifyLibGraalContextChecks());
395+
}
396+
371397
loadVerifiers(verifiers);
372398

373399
VerifyFoldableMethods foldableMethodsVerifier = new VerifyFoldableMethods();
@@ -401,9 +427,14 @@ public static void runTest(InvariantsTool tool) {
401427
ResolvedJavaType optionDescriptorsType = metaAccess.lookupJavaType(OptionDescriptors.class);
402428

403429
if (errors.isEmpty()) {
430+
ClassLoader cl = CheckGraalInvariants.class.getClassLoader();
431+
if (tool.shouldVerifyLibGraalInvariants()) {
432+
URL[] urls = {toURL(tool.getLibgraalJar())};
433+
cl = new URLClassLoader(urls, cl);
434+
}
404435
// Order outer classes before the inner classes
405-
classNames.sort((String a, String b) -> a.compareTo(b));
406-
List<Class<?>> classes = loadClasses(tool, metaAccess, classNames);
436+
classNames.sort(Comparator.naturalOrder());
437+
List<Class<?>> classes = loadClasses(tool, metaAccess, classNames, cl);
407438
for (Class<?> c : classes) {
408439
String className = c.getName();
409440
executor.execute(() -> {
@@ -491,7 +522,7 @@ public static void runTest(InvariantsTool tool) {
491522
StringBuilder msg = new StringBuilder();
492523
String nl = String.format("%n");
493524
for (String e : errors) {
494-
if (msg.length() != 0) {
525+
if (!msg.isEmpty()) {
495526
msg.append(nl);
496527
}
497528
msg.append(e);
@@ -500,6 +531,14 @@ public static void runTest(InvariantsTool tool) {
500531
}
501532
}
502533

534+
private static URL toURL(Path path) {
535+
try {
536+
return path.toUri().toURL();
537+
} catch (MalformedURLException e) {
538+
throw new GraalError(e);
539+
}
540+
}
541+
503542
@SuppressWarnings("unchecked")
504543
private static void loadVerifiers(List<VerifyPhase<CoreProviders>> verifiers) {
505544
for (VerifyPhase<CoreProviders> verifier : ServiceLoader.load(VerifyPhase.class)) {
@@ -578,14 +617,14 @@ private static boolean isONNX(String className) {
578617
return className.contains("ai.onnxruntime");
579618
}
580619

581-
private static List<Class<?>> loadClasses(InvariantsTool tool, MetaAccessProvider metaAccess, List<String> classNames) {
620+
private static List<Class<?>> loadClasses(InvariantsTool tool, MetaAccessProvider metaAccess, List<String> classNames, ClassLoader cl) {
582621
List<Class<?>> classes = new ArrayList<>(classNames.size());
583622
for (String className : classNames) {
584623
if (!tool.shouldLoadClass(className)) {
585624
continue;
586625
}
587626
try {
588-
Class<?> c = Class.forName(className, false, CheckGraalInvariants.class.getClassLoader());
627+
Class<?> c = Class.forName(className, false, cl);
589628

590629
/*
591630
* Ensure all types are linked eagerly, so that we can access the bytecode of all
@@ -806,7 +845,7 @@ public boolean supportsLazyInitialization(ConstantPool cp) {
806845

807846
@Override
808847
public void loadReferencedType(GraphBuilderContext builder, ConstantPool cp, int cpi, int bytecode) {
809-
((HotSpotConstantPool) cp).loadReferencedType(cpi, bytecode, false);
848+
cp.loadReferencedType(cpi, bytecode, false);
810849
}
811850

812851
@Override

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/VerifyLibGraalContextChecks.java‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,23 @@
2828

2929
import jdk.graal.compiler.nodes.StructuredGraph;
3030
import jdk.graal.compiler.nodes.java.LoadFieldNode;
31+
import jdk.graal.compiler.nodes.java.MethodCallTargetNode;
3132
import jdk.graal.compiler.nodes.spi.CoreProviders;
3233
import jdk.graal.compiler.phases.VerifyPhase;
3334
import jdk.graal.compiler.serviceprovider.GraalServices;
35+
import jdk.vm.ci.meta.MetaAccessProvider;
3436
import jdk.vm.ci.meta.ResolvedJavaField;
37+
import jdk.vm.ci.meta.ResolvedJavaMethod;
3538
import jdk.vm.ci.meta.ResolvedJavaType;
3639
import jdk.vm.ci.services.Services;
3740

3841
/**
39-
* Ensures that no code accesses {@link jdk.vm.ci.services.Services#IS_IN_NATIVE_IMAGE}.
42+
* Checks invariants related to libgraal. This includes:
43+
* <ul>
44+
* <li>no code accesses {@link jdk.vm.ci.services.Services#IS_IN_NATIVE_IMAGE}</li>
45+
* <li>no calls to Runtime.version() as libgraal might be deployed in different JDK version than the
46+
* one in which it was built</li>
47+
* </ul>
4048
*/
4149
public class VerifyLibGraalContextChecks extends VerifyPhase<CoreProviders> {
4250

@@ -47,8 +55,9 @@ public boolean checkContract() {
4755

4856
@Override
4957
protected void verify(StructuredGraph graph, CoreProviders context) {
58+
MetaAccessProvider metaAccess = context.getMetaAccess();
5059

51-
final ResolvedJavaType servicesType = context.getMetaAccess().lookupJavaType(Services.class);
60+
final ResolvedJavaType servicesType = metaAccess.lookupJavaType(Services.class);
5261
servicesType.getStaticFields();
5362

5463
List<LoadFieldNode> loads = graph.getNodes().filter(LoadFieldNode.class).snapshot();
@@ -64,5 +73,21 @@ protected void verify(StructuredGraph graph, CoreProviders context) {
6473
}
6574
}
6675
}
76+
77+
ResolvedJavaType runtimeType = metaAccess.lookupJavaType(Runtime.class);
78+
ResolvedJavaType runtimeVersionType = metaAccess.lookupJavaType(Runtime.Version.class);
79+
for (MethodCallTargetNode t : graph.getNodes(MethodCallTargetNode.TYPE)) {
80+
ResolvedJavaMethod callee = t.targetMethod();
81+
String calleeName = callee.getName();
82+
ResolvedJavaType calleeDeclaringClass = callee.getDeclaringClass();
83+
if (calleeDeclaringClass.equals(runtimeType)) {
84+
if (calleeName.equals("version")) {
85+
throw new VerificationError(t, "call to Runtime.version() not allowed");
86+
}
87+
}
88+
if (runtimeVersionType.equals(callee.getDeclaringClass()) && "feature".equals(callee.getName())) {
89+
throw new VerificationError("Call to %s at callsite %s is prohibited, use JavaVersionUtil.JAVA_SPEC", callee.format("%H.%n(%p)"), graph.method().format("%H.%n(%p)"));
90+
}
91+
}
6792
}
6893
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/VerifyRuntimeVersionFeature.java‎

Lines changed: 0 additions & 67 deletions
This file was deleted.

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,13 @@
3232
import jdk.graal.compiler.hotspot.HotSpotGraalRuntimeProvider;
3333
import jdk.graal.compiler.runtime.RuntimeProvider;
3434
import org.junit.Assert;
35-
import org.junit.Assume;
3635
import org.junit.Test;
3736

3837
import jdk.vm.ci.common.JVMCIError;
3938

4039
public class GraalHotSpotVMConfigAccessTest {
4140
@Test
4241
public void test() {
43-
Assume.assumeTrue("Only expect error in JDK with explicit JVMCI version (e.g. labsjdk)", GraalHotSpotVMConfig.JVMCI);
4442
HotSpotGraalRuntimeProvider rt = (HotSpotGraalRuntimeProvider) Graal.getRequiredCapability(RuntimeProvider.class);
4543
GraalHotSpotVMConfig config = rt.getVMConfig();
4644

0 commit comments

Comments
 (0)