From 7b47cffb43e633dd5c2d33d3cca259bbe88f8bfa Mon Sep 17 00:00:00 2001 From: Yudi Zheng Date: Thu, 17 Jul 2025 15:55:05 +0200 Subject: [PATCH 1/3] [Graal] Adapt JDK-8361842: Move input validation checks to Java for java.lang.StringCoding intrinsics --- .../hotspot/GraalHotSpotVMConfig.java | 2 ++ .../HotSpotPlatformConfigurationProvider.java | 7 ++++ .../spi/PlatformConfigurationProvider.java | 9 +++++ .../StandardGraphBuilderPlugins.java | 34 ++++++++++++++----- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java index 852e7047b872..b3f90a8c1f04 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java @@ -692,6 +692,8 @@ public boolean supportJVMTIVThreadNotification() { public final boolean deoptimizationSupportLargeAccessByteArrayVirtualization = // getConstant("Deoptimization::_support_large_access_byte_array_virtualization", Boolean.class); + public final boolean verifyIntrinsicChecks = getFlag("VerifyIntrinsicChecks", Boolean.class); + public final int l1LineSize = getFieldValue("CompilerToVM::Data::L1_line_size", Integer.class, "int", 0, "amd64".equals(osArch)); public final boolean supportsAvx512SimdSort = getFieldValue("CompilerToVM::Data::supports_avx512_simd_sort", Boolean.class, "bool", false, "amd64".equals(osArch)); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java index 8c33ff12857d..ab86399b7425 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java @@ -32,10 +32,12 @@ public class HotSpotPlatformConfigurationProvider implements PlatformConfigurati private final BarrierSet barrierSet; private final boolean canVirtualizeLargeByteArrayAccess; + private final boolean shouldVerifyIntrinsicChecks; public HotSpotPlatformConfigurationProvider(GraalHotSpotVMConfig config, BarrierSet barrierSet) { this.barrierSet = barrierSet; this.canVirtualizeLargeByteArrayAccess = config.deoptimizationSupportLargeAccessByteArrayVirtualization; + this.shouldVerifyIntrinsicChecks = config.verifyIntrinsicChecks; } @Override @@ -62,4 +64,9 @@ public boolean areLocksSideEffectFree() { public BarrierSet getBarrierSet() { return barrierSet; } + + @Override + public boolean shouldVerifyIntrinsicChecks() { + return shouldVerifyIntrinsicChecks; + } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java index c47e57dbbe97..8f4c48383d24 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java @@ -82,4 +82,13 @@ default boolean requiresStrictLockOrder() { default boolean areLocksSideEffectFree() { return true; } + + /** + * Callers of the intrinsics (e.g., StringCoding.countPositives) are responsible for intrinsic + * input validation. Enabling this option enforces additional checks within the intrinsic + * implementation. + */ + default boolean shouldVerifyIntrinsicChecks() { + return false; + } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java index deb41020dcaa..4e23c8996b00 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java @@ -2598,13 +2598,27 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec }); } + private static void generateStringRangeCheck(GraphBuilderContext b, InvocationPluginHelper helper, ValueNode array, ValueNode off, ValueNode len) { + helper.intrinsicRangeCheck(off, Condition.LT, ConstantNode.forInt(0)); + helper.intrinsicRangeCheck(len, Condition.LT, ConstantNode.forInt(0)); + + ValueNode arrayLength = b.add(new ArrayLengthNode(array)); + ValueNode limit = b.add(AddNode.create(off, len, NodeView.DEFAULT)); + helper.intrinsicRangeCheck(arrayLength, Condition.LT, limit); + } + private static void registerStringCodingPlugins(InvocationPlugins plugins, Replacements replacements) { Registration r = new Registration(plugins, "java.lang.StringCoding", replacements); - r.register(new InvocationPlugin("implEncodeISOArray", byte[].class, int.class, byte[].class, int.class, int.class) { + r.register(new InvocationPlugin("encodeISOArray0", byte[].class, int.class, byte[].class, int.class, int.class) { @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode sa, ValueNode sp, ValueNode da, ValueNode dp, ValueNode len) { try (InvocationPluginHelper helper = new InvocationPluginHelper(b, targetMethod)) { + if (b.getPlatformConfigurationProvider().shouldVerifyIntrinsicChecks()) { + generateStringRangeCheck(b, helper, sa, sp, len); + generateStringRangeCheck(b, helper, da, dp, len); + } + int charElementShift = CodeUtil.log2(b.getMetaAccess().getArrayIndexScale(JavaKind.Char)); ValueNode src = helper.arrayElementPointer(sa, JavaKind.Byte, LeftShiftNode.create(sp, ConstantNode.forInt(charElementShift), NodeView.DEFAULT)); ValueNode dst = helper.arrayElementPointer(da, JavaKind.Byte, dp); @@ -2613,11 +2627,16 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec } } }); - r.register(new InvocationPlugin("implEncodeAsciiArray", char[].class, int.class, byte[].class, int.class, int.class) { + r.register(new InvocationPlugin("encodeAsciiArray0", char[].class, int.class, byte[].class, int.class, int.class) { @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode sa, ValueNode sp, ValueNode da, ValueNode dp, ValueNode len) { try (InvocationPluginHelper helper = new InvocationPluginHelper(b, targetMethod)) { + if (b.getPlatformConfigurationProvider().shouldVerifyIntrinsicChecks()) { + generateStringRangeCheck(b, helper, sa, sp, len); + generateStringRangeCheck(b, helper, da, dp, len); + } + ValueNode src = helper.arrayElementPointer(sa, JavaKind.Char, sp); ValueNode dst = helper.arrayElementPointer(da, JavaKind.Byte, dp); b.addPush(JavaKind.Int, new EncodeArrayNode(src, dst, len, ASCII, JavaKind.Char)); @@ -2625,16 +2644,13 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec } } }); - r.register(new InvocationPlugin("countPositives", byte[].class, int.class, int.class) { + r.register(new InvocationPlugin("countPositives0", byte[].class, int.class, int.class) { @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode ba, ValueNode off, ValueNode len) { try (InvocationPluginHelper helper = new InvocationPluginHelper(b, targetMethod)) { - helper.intrinsicRangeCheck(off, Condition.LT, ConstantNode.forInt(0)); - helper.intrinsicRangeCheck(len, Condition.LT, ConstantNode.forInt(0)); - - ValueNode arrayLength = b.add(new ArrayLengthNode(ba)); - ValueNode limit = b.add(AddNode.create(off, len, NodeView.DEFAULT)); - helper.intrinsicRangeCheck(arrayLength, Condition.LT, limit); + if (b.getPlatformConfigurationProvider().shouldVerifyIntrinsicChecks()) { + generateStringRangeCheck(b, helper, ba, off, len); + } ValueNode array = helper.arrayElementPointer(ba, JavaKind.Byte, off); b.addPush(JavaKind.Int, new CountPositivesNode(array, len)); From 3716670b83617bdece17b3e87e9866bc670f3dd2 Mon Sep 17 00:00:00 2001 From: Yudi Zheng Date: Thu, 17 Jul 2025 15:55:24 +0200 Subject: [PATCH 2/3] Update galahad jdk. --- common.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.json b/common.json index 24d661824261..c31c3605bace 100644 --- a/common.json +++ b/common.json @@ -8,7 +8,7 @@ "COMMENT.jdks": "When adding or removing JDKs keep in sync with JDKs in ci/common.jsonnet", "jdks": { - "galahad-jdk": {"name": "jpg-jdk", "version": "26", "build_id": "jdk-26+7-655", "platformspecific": true, "extrabundles": ["static-libs"]}, + "galahad-jdk": {"name": "jpg-jdk", "version": "26", "build_id": "jdk-26+7-XXX", "platformspecific": true, "extrabundles": ["static-libs"]}, "oraclejdk17": {"name": "jpg-jdk", "version": "17.0.7", "build_id": "jdk-17.0.7+8", "platformspecific": true, "extrabundles": ["static-libs"]}, "labsjdk-ce-17": {"name": "labsjdk", "version": "ce-17.0.7+4-jvmci-23.1-b02", "platformspecific": true }, From e735b59de5294eddb2ad142375b7751e9cc1cc7e Mon Sep 17 00:00:00 2001 From: Yudi Zheng Date: Thu, 17 Jul 2025 17:16:14 +0200 Subject: [PATCH 3/3] drop support for VerifyIntrinsicChecks --- .../hotspot/GraalHotSpotVMConfig.java | 2 -- .../HotSpotPlatformConfigurationProvider.java | 7 ------ .../spi/PlatformConfigurationProvider.java | 9 -------- .../StandardGraphBuilderPlugins.java | 23 ------------------- 4 files changed, 41 deletions(-) diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java index b3f90a8c1f04..852e7047b872 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/GraalHotSpotVMConfig.java @@ -692,8 +692,6 @@ public boolean supportJVMTIVThreadNotification() { public final boolean deoptimizationSupportLargeAccessByteArrayVirtualization = // getConstant("Deoptimization::_support_large_access_byte_array_virtualization", Boolean.class); - public final boolean verifyIntrinsicChecks = getFlag("VerifyIntrinsicChecks", Boolean.class); - public final int l1LineSize = getFieldValue("CompilerToVM::Data::L1_line_size", Integer.class, "int", 0, "amd64".equals(osArch)); public final boolean supportsAvx512SimdSort = getFieldValue("CompilerToVM::Data::supports_avx512_simd_sort", Boolean.class, "bool", false, "amd64".equals(osArch)); diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java index ab86399b7425..8c33ff12857d 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotPlatformConfigurationProvider.java @@ -32,12 +32,10 @@ public class HotSpotPlatformConfigurationProvider implements PlatformConfigurati private final BarrierSet barrierSet; private final boolean canVirtualizeLargeByteArrayAccess; - private final boolean shouldVerifyIntrinsicChecks; public HotSpotPlatformConfigurationProvider(GraalHotSpotVMConfig config, BarrierSet barrierSet) { this.barrierSet = barrierSet; this.canVirtualizeLargeByteArrayAccess = config.deoptimizationSupportLargeAccessByteArrayVirtualization; - this.shouldVerifyIntrinsicChecks = config.verifyIntrinsicChecks; } @Override @@ -64,9 +62,4 @@ public boolean areLocksSideEffectFree() { public BarrierSet getBarrierSet() { return barrierSet; } - - @Override - public boolean shouldVerifyIntrinsicChecks() { - return shouldVerifyIntrinsicChecks; - } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java index 8f4c48383d24..c47e57dbbe97 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/spi/PlatformConfigurationProvider.java @@ -82,13 +82,4 @@ default boolean requiresStrictLockOrder() { default boolean areLocksSideEffectFree() { return true; } - - /** - * Callers of the intrinsics (e.g., StringCoding.countPositives) are responsible for intrinsic - * input validation. Enabling this option enforces additional checks within the intrinsic - * implementation. - */ - default boolean shouldVerifyIntrinsicChecks() { - return false; - } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java index 4e23c8996b00..8961067220a5 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/StandardGraphBuilderPlugins.java @@ -2598,15 +2598,6 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec }); } - private static void generateStringRangeCheck(GraphBuilderContext b, InvocationPluginHelper helper, ValueNode array, ValueNode off, ValueNode len) { - helper.intrinsicRangeCheck(off, Condition.LT, ConstantNode.forInt(0)); - helper.intrinsicRangeCheck(len, Condition.LT, ConstantNode.forInt(0)); - - ValueNode arrayLength = b.add(new ArrayLengthNode(array)); - ValueNode limit = b.add(AddNode.create(off, len, NodeView.DEFAULT)); - helper.intrinsicRangeCheck(arrayLength, Condition.LT, limit); - } - private static void registerStringCodingPlugins(InvocationPlugins plugins, Replacements replacements) { Registration r = new Registration(plugins, "java.lang.StringCoding", replacements); r.register(new InvocationPlugin("encodeISOArray0", byte[].class, int.class, byte[].class, int.class, int.class) { @@ -2614,11 +2605,6 @@ private static void registerStringCodingPlugins(InvocationPlugins plugins, Repla public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode sa, ValueNode sp, ValueNode da, ValueNode dp, ValueNode len) { try (InvocationPluginHelper helper = new InvocationPluginHelper(b, targetMethod)) { - if (b.getPlatformConfigurationProvider().shouldVerifyIntrinsicChecks()) { - generateStringRangeCheck(b, helper, sa, sp, len); - generateStringRangeCheck(b, helper, da, dp, len); - } - int charElementShift = CodeUtil.log2(b.getMetaAccess().getArrayIndexScale(JavaKind.Char)); ValueNode src = helper.arrayElementPointer(sa, JavaKind.Byte, LeftShiftNode.create(sp, ConstantNode.forInt(charElementShift), NodeView.DEFAULT)); ValueNode dst = helper.arrayElementPointer(da, JavaKind.Byte, dp); @@ -2632,11 +2618,6 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode sa, ValueNode sp, ValueNode da, ValueNode dp, ValueNode len) { try (InvocationPluginHelper helper = new InvocationPluginHelper(b, targetMethod)) { - if (b.getPlatformConfigurationProvider().shouldVerifyIntrinsicChecks()) { - generateStringRangeCheck(b, helper, sa, sp, len); - generateStringRangeCheck(b, helper, da, dp, len); - } - ValueNode src = helper.arrayElementPointer(sa, JavaKind.Char, sp); ValueNode dst = helper.arrayElementPointer(da, JavaKind.Byte, dp); b.addPush(JavaKind.Int, new EncodeArrayNode(src, dst, len, ASCII, JavaKind.Char)); @@ -2648,10 +2629,6 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode ba, ValueNode off, ValueNode len) { try (InvocationPluginHelper helper = new InvocationPluginHelper(b, targetMethod)) { - if (b.getPlatformConfigurationProvider().shouldVerifyIntrinsicChecks()) { - generateStringRangeCheck(b, helper, ba, off, len); - } - ValueNode array = helper.arrayElementPointer(ba, JavaKind.Byte, off); b.addPush(JavaKind.Int, new CountPositivesNode(array, len)); return true;