Skip to content

Commit 3c29adf

Browse files
fniephauszapster
fniephaus
authored and
zapster
committed
[GR-47922] Enforce -H:±UnlockExperimentalVMOptions in CI.
PullRequest: graal/15747
2 parents bb8ff30 + 4ae86f7 commit 3c29adf

File tree

8 files changed

+101
-67
lines changed

8 files changed

+101
-67
lines changed

‎.github/workflows/main.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ env:
4242
MX_GIT_CACHE: refcache
4343
MX_PATH: ${{ github.workspace }}/mx
4444
MX_PYTHON: python3.8
45+
# Enforce experimental option checking in CI (GR-47922)
46+
NATIVE_IMAGE_EXPERIMENTAL_OPTIONS_ARE_FATAL: "true"
4547

4648
permissions:
4749
contents: read # to fetch code (actions/checkout)

‎ci/common.jsonnet‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,10 @@ local common_json = import "../common.json";
277277
# Keep in sync with com.oracle.svm.hosted.NativeImageOptions#DEFAULT_ERROR_FILE_NAME
278278
" (?P<filename>.+/svm_err_b_\\d+T\\d+\\.\\d+_pid\\d+\\.md)",
279279
],
280+
environment+: {
281+
# Enforce experimental option checking in CI (GR-47922)
282+
NATIVE_IMAGE_EXPERIMENTAL_OPTIONS_ARE_FATAL: "true",
283+
},
280284
},
281285

282286
// OS specific file handling

‎substratevm/mx.substratevm/mx_substratevm.py‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ def native_image_context(common_args=None, hosted_assertions=True, native_image_
268268
raise mx.abort('The built GraalVM for config ' + str(config) + ' does not contain a native-image command')
269269

270270
def _native_image(args, **kwargs):
271-
mx.run([native_image_cmd] + _maybe_convert_to_args_file(args), **kwargs)
271+
return mx.run([native_image_cmd] + _maybe_convert_to_args_file(args), **kwargs)
272272

273273
def is_launcher(launcher_path):
274274
with open(launcher_path, 'rb') as fp:
@@ -282,32 +282,45 @@ def is_launcher(launcher_path):
282282
verbose_image_build_option = ['--verbose'] if mx.get_opts().verbose else []
283283
_native_image(verbose_image_build_option + ['--macro:native-image-launcher'])
284284

285-
def query_native_image(all_args, option):
286-
285+
def query_native_image(all_args):
287286
stdoutdata = []
288287
def stdout_collector(x):
289288
stdoutdata.append(x.rstrip())
290-
_native_image(['--dry-run', '--verbose'] + all_args, out=stdout_collector)
289+
stderrdata = []
290+
def stderr_collector(x):
291+
stderrdata.append(x.rstrip())
292+
exit_code = _native_image(['--dry-run', '--verbose'] + all_args, nonZeroIsFatal=False, out=stdout_collector, err=stderr_collector)
293+
if exit_code != 0:
294+
for line in stdoutdata:
295+
print(line)
296+
for line in stderrdata:
297+
print(line)
298+
mx.abort('Failed to query native-image.')
291299

292300
def remove_quotes(val):
293301
if len(val) >= 2 and val.startswith("'") and val.endswith("'"):
294302
return val[1:-1].replace("\\'", "'")
295303
else:
296304
return val
297305

298-
result = None
306+
path_regex = re.compile(r'^-H:Path(@[^=]*)?=')
307+
name_regex = re.compile(r'^-H:Name(@[^=]*)?=')
308+
path = name = None
299309
for line in stdoutdata:
300310
arg = remove_quotes(line.rstrip('\\').strip())
301-
m = re.match(option, arg)
302-
if m:
303-
result = arg[m.end():]
311+
path_matcher = path_regex.match(arg)
312+
if path_matcher:
313+
path = arg[path_matcher.end():]
314+
name_matcher = name_regex.match(arg)
315+
if name_matcher:
316+
name = arg[name_matcher.end():]
304317

305-
return result
318+
assert path is not None and name is not None
319+
return path, name
306320

307321
def native_image_func(args, **kwargs):
308322
all_args = base_args + common_args + args
309-
path = query_native_image(all_args, r'^-H:Path(@[^=]*)?=')
310-
name = query_native_image(all_args, r'^-H:Name(@[^=]*)?=')
323+
path, name = query_native_image(all_args)
311324
image = join(path, name)
312325
_native_image(all_args, **kwargs)
313326
return image

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,4 +1022,12 @@ public enum ReportingMode {
10221022

10231023
@Option(help = "Include all classes, methods, fields, and resources from given paths", type = OptionType.Debug) //
10241024
public static final HostedOptionKey<LocatableMultiOptionValue.Strings> IncludeAllFromPath = new HostedOptionKey<>(LocatableMultiOptionValue.Strings.build());
1025+
1026+
public static class TruffleStableOptions {
1027+
1028+
@Option(help = "Automatically copy the necessary language resources to the resources/languages directory next to the produced image." +
1029+
"Language resources for each language are specified in the native-image-resources.filelist file located in the language home directory." +
1030+
"If there is no native-image-resources.filelist file in the language home directory or the file is empty, then no resources are copied.", type = User, stability = OptionStability.STABLE)//
1031+
public static final HostedOptionKey<Boolean> CopyLanguageResources = new HostedOptionKey<>(true);
1032+
}
10251033
}

‎substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/APIOptionHandler.java‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ static final class PathsOptionInfo {
9797
private final Map<String, PathsOptionInfo> pathOptions;
9898
private final Set<String> stableOptionNames;
9999

100-
private boolean experimentalOptionsAreUnlocked = false;
100+
private int numberOfActiveUnlockExperimentalVMOptions = 0;
101101
private Set<String> illegalExperimentalOptions = new HashSet<>(0);
102102

103103
APIOptionHandler(NativeImage nativeImage) {
@@ -307,11 +307,21 @@ boolean consume(ArgumentQueue args) {
307307
return true;
308308
}
309309
if (ENTER_UNLOCK_SCOPE.equals(headArg)) {
310-
experimentalOptionsAreUnlocked = true;
310+
if (args.numberOfFirstObservedActiveUnlockExperimentalVMOptions < 0) {
311+
/*
312+
* Remember numberOfExperimentalOptionsUnlocks per ArgumentQueue for verification
313+
* purposes only. Each queue cannot lock more than it unlocks.
314+
*/
315+
args.numberOfFirstObservedActiveUnlockExperimentalVMOptions = numberOfActiveUnlockExperimentalVMOptions;
316+
}
317+
numberOfActiveUnlockExperimentalVMOptions++;
311318
} else if (LEAVE_UNLOCK_SCOPE.equals(headArg)) {
312-
VMError.guarantee(experimentalOptionsAreUnlocked, "ensureConsistentUnlockScopes() missed an open unlock scope");
313-
experimentalOptionsAreUnlocked = false;
314-
} else if (!experimentalOptionsAreUnlocked && !OptionOrigin.isAPI(args.argumentOrigin) && headArg.startsWith(NativeImage.oH) && stableOptionNames.stream().noneMatch(p -> headArg.matches(p))) {
319+
if (numberOfActiveUnlockExperimentalVMOptions <= 0 || numberOfActiveUnlockExperimentalVMOptions <= args.numberOfFirstObservedActiveUnlockExperimentalVMOptions) {
320+
throw VMError.shouldNotReachHere("Unlocking of experimental options in inconsistent state: trying to lock more scopes than exist or allowed.");
321+
}
322+
numberOfActiveUnlockExperimentalVMOptions--;
323+
} else if (numberOfActiveUnlockExperimentalVMOptions == 0 && !OptionOrigin.isAPI(args.argumentOrigin) && headArg.startsWith(NativeImage.oH) &&
324+
stableOptionNames.stream().noneMatch(p -> headArg.matches(p))) {
315325
illegalExperimentalOptions.add(headArg);
316326
}
317327
for (Entry<String, GroupInfo> entry : groupInfos.entrySet()) {

‎substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ static class ArgumentQueue {
161161

162162
private final ArrayDeque<String> queue;
163163
public final String argumentOrigin;
164+
public int numberOfFirstObservedActiveUnlockExperimentalVMOptions = -1;
164165

165166
ArgumentQueue(String argumentOrigin) {
166167
queue = new ArrayDeque<>();
@@ -734,11 +735,13 @@ public boolean isExcluded(Path resourcePath, Path entry) {
734735

735736
private ArrayList<String> createFallbackBuildArgs() {
736737
ArrayList<String> buildArgs = new ArrayList<>();
738+
buildArgs.add(oHEnabled(SubstrateOptions.UnlockExperimentalVMOptions));
737739
Collection<String> fallbackSystemProperties = customJavaArgs.stream()
738740
.filter(s -> s.startsWith("-D"))
739741
.collect(Collectors.toCollection(LinkedHashSet::new));
742+
String fallbackExecutorSystemPropertyOption = oH(FallbackExecutor.Options.FallbackExecutorSystemProperty);
740743
for (String property : fallbackSystemProperties) {
741-
buildArgs.add(oH(FallbackExecutor.Options.FallbackExecutorSystemProperty) + property);
744+
buildArgs.add(injectHostedOptionOrigin(fallbackExecutorSystemPropertyOption + property, OptionOrigin.originDriver));
742745
}
743746

744747
List<String> runtimeJavaArgs = imageBuilderArgs.stream()
@@ -755,7 +758,6 @@ private ArrayList<String> createFallbackBuildArgs() {
755758
buildArgs.add(fallbackExecutorJavaArg);
756759
}
757760

758-
buildArgs.add(oHEnabled(SubstrateOptions.UnlockExperimentalVMOptions));
759761
buildArgs.add(oHEnabled(SubstrateOptions.BuildOutputSilent));
760762
buildArgs.add(oHEnabled(SubstrateOptions.ParseRuntimeOptions));
761763
Path imagePathPath;

‎substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/TruffleBaseFeature.java‎

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import static com.oracle.svm.core.util.VMError.shouldNotReachHere;
2828
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
29-
import static jdk.graal.compiler.options.OptionType.User;
3029

3130
import java.io.IOException;
3231
import java.lang.annotation.Annotation;
@@ -60,23 +59,6 @@
6059
import java.util.stream.Stream;
6160

6261
import org.graalvm.collections.Pair;
63-
import jdk.graal.compiler.api.replacements.SnippetReflectionProvider;
64-
import jdk.graal.compiler.nodes.ConstantNode;
65-
import jdk.graal.compiler.nodes.ValueNode;
66-
import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration.Plugins;
67-
import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderContext;
68-
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin;
69-
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin.RequiredInlineOnlyInvocationPlugin;
70-
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin.RequiredInvocationPlugin;
71-
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugins;
72-
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugins.Registration;
73-
import jdk.graal.compiler.options.Option;
74-
import jdk.graal.compiler.options.OptionStability;
75-
import jdk.graal.compiler.phases.tiers.Suites;
76-
import jdk.graal.compiler.phases.util.Providers;
77-
import jdk.graal.compiler.truffle.host.InjectImmutableFrameFieldsPhase;
78-
import jdk.graal.compiler.truffle.host.TruffleHostEnvironment;
79-
import jdk.graal.compiler.truffle.substitutions.TruffleInvocationPlugins;
8062
import org.graalvm.home.HomeFinder;
8163
import org.graalvm.home.impl.DefaultHomeFinder;
8264
import org.graalvm.nativeimage.AnnotationAccess;
@@ -95,6 +77,7 @@
9577
import com.oracle.svm.core.NeverInline;
9678
import com.oracle.svm.core.ParsingReason;
9779
import com.oracle.svm.core.RuntimeAssertionsSupport;
80+
import com.oracle.svm.core.SubstrateOptions;
9881
import com.oracle.svm.core.SubstrateUtil;
9982
import com.oracle.svm.core.annotate.Alias;
10083
import com.oracle.svm.core.annotate.AnnotateOriginal;
@@ -150,6 +133,22 @@
150133
import com.oracle.truffle.api.staticobject.StaticProperty;
151134
import com.oracle.truffle.api.staticobject.StaticShape;
152135

136+
import jdk.graal.compiler.api.replacements.SnippetReflectionProvider;
137+
import jdk.graal.compiler.nodes.ConstantNode;
138+
import jdk.graal.compiler.nodes.ValueNode;
139+
import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration.Plugins;
140+
import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderContext;
141+
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin;
142+
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin.RequiredInlineOnlyInvocationPlugin;
143+
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin.RequiredInvocationPlugin;
144+
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugins;
145+
import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugins.Registration;
146+
import jdk.graal.compiler.options.Option;
147+
import jdk.graal.compiler.phases.tiers.Suites;
148+
import jdk.graal.compiler.phases.util.Providers;
149+
import jdk.graal.compiler.truffle.host.InjectImmutableFrameFieldsPhase;
150+
import jdk.graal.compiler.truffle.host.TruffleHostEnvironment;
151+
import jdk.graal.compiler.truffle.substitutions.TruffleInvocationPlugins;
153152
import jdk.internal.misc.Unsafe;
154153
import jdk.vm.ci.meta.JavaKind;
155154
import jdk.vm.ci.meta.ResolvedJavaMethod;
@@ -183,11 +182,6 @@ public static Class<?> lookupClass(String className) {
183182

184183
public static class Options {
185184

186-
@Option(help = "Automatically copy the necessary language resources to the resources/languages directory next to the produced image." +
187-
"Language resources for each language are specified in the native-image-resources.filelist file located in the language home directory." +
188-
"If there is no native-image-resources.filelist file in the language home directory or the file is empty, then no resources are copied.", type = User, stability = OptionStability.STABLE)//
189-
public static final HostedOptionKey<Boolean> CopyLanguageResources = new HostedOptionKey<>(true);
190-
191185
@Option(help = "Check that context pre-initialization does not introduce absolute TruffleFiles into the image heap.")//
192186
public static final HostedOptionKey<Boolean> TruffleCheckPreinitializedFiles = new HostedOptionKey<>(true);
193187
}
@@ -402,7 +396,7 @@ public void duringSetup(DuringSetupAccess access) {
402396
StaticObjectSupport.duringSetup(access);
403397

404398
HomeFinder hf = HomeFinder.getInstance();
405-
if (Options.CopyLanguageResources.getValue()) {
399+
if (SubstrateOptions.TruffleStableOptions.CopyLanguageResources.getValue()) {
406400
if (!(hf instanceof DefaultHomeFinder)) {
407401
VMError.shouldNotReachHere(String.format("HomeFinder %s cannot be used if CopyLanguageResources option of TruffleBaseFeature is enabled", hf.getClass().getName()));
408402
}
@@ -1070,7 +1064,7 @@ static void onBuildInvocation(Class<?> storageSuperClass, Class<?> factoryInterf
10701064

10711065
@Override
10721066
public void afterImageWrite(AfterImageWriteAccess access) {
1073-
if (Options.CopyLanguageResources.getValue()) {
1067+
if (SubstrateOptions.TruffleStableOptions.CopyLanguageResources.getValue()) {
10741068
Path buildDir = access.getImagePath();
10751069
if (buildDir != null) {
10761070
Path parent = buildDir.getParent();
@@ -1415,7 +1409,7 @@ public ValueAvailability valueAvailability() {
14151409

14161410
@Override
14171411
public Object transform(Object receiver, Object originalValue) {
1418-
return TruffleBaseFeature.Options.CopyLanguageResources.getValue();
1412+
return SubstrateOptions.TruffleStableOptions.CopyLanguageResources.getValue();
14191413
}
14201414
}
14211415
}

‎vm/mx.vm/mx_vm_benchmark.py‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -172,58 +172,59 @@ def __init__(self, vm, bm_suite, args):
172172
self.latest_profile_path = self.profile_path_no_extension + '-latest' + self.profile_file_extension
173173
self.config_dir = os.path.join(self.output_dir, 'config')
174174
self.log_dir = self.output_dir
175-
self.base_image_build_args = [os.path.join(vm.home(), 'bin', 'native-image')]
176-
self.base_image_build_args += ['--no-fallback', '-g']
177-
self.base_image_build_args += svm_experimental_options(['-H:+VerifyGraalGraphs', '-H:+VerifyPhases', '--diagnostics-mode']) if vm.is_gate else []
178-
self.base_image_build_args += ['-H:+ReportExceptionStackTraces']
179-
self.base_image_build_args += bm_suite.build_assertions(self.benchmark_name, vm.is_gate)
175+
base_image_build_args = ['--no-fallback', '-g']
176+
base_image_build_args += ['-H:+VerifyGraalGraphs', '-H:+VerifyPhases', '--diagnostics-mode'] if vm.is_gate else []
177+
base_image_build_args += ['-H:+ReportExceptionStackTraces']
178+
base_image_build_args += bm_suite.build_assertions(self.benchmark_name, vm.is_gate)
180179

181-
self.base_image_build_args += self.system_properties
180+
base_image_build_args += self.system_properties
182181
self.bundle_path = self.get_bundle_path_if_present()
183182
self.bundle_create_path = self.get_bundle_create_path_if_present()
184183
if not self.bundle_path:
185-
self.base_image_build_args += self.classpath_arguments
186-
self.base_image_build_args += self.modulepath_arguments
187-
self.base_image_build_args += self.executable
188-
self.base_image_build_args += svm_experimental_options(['-H:Path=' + self.output_dir])
189-
self.base_image_build_args += svm_experimental_options([
184+
base_image_build_args += self.classpath_arguments
185+
base_image_build_args += self.modulepath_arguments
186+
base_image_build_args += self.executable
187+
base_image_build_args += ['-H:Path=' + self.output_dir]
188+
base_image_build_args += [
190189
'-H:ConfigurationFileDirectories=' + self.config_dir,
191190
'-H:+PrintAnalysisStatistics',
192191
'-H:+PrintCallEdges',
193192
'-H:+CollectImageBuildStatistics',
194-
])
193+
]
195194
self.image_build_reports_directory = os.path.join(self.output_dir, 'reports')
196195
if self.bundle_create_path is not None:
197196
self.image_build_reports_directory = os.path.join(self.output_dir, self.bundle_create_path)
198197
self.image_build_stats_file = os.path.join(self.image_build_reports_directory, 'image_build_statistics.json')
199198

200199
if vm.is_quickbuild:
201-
self.base_image_build_args += ['-Ob']
200+
base_image_build_args += ['-Ob']
202201
if vm.use_string_inlining:
203-
self.base_image_build_args += svm_experimental_options(['-H:+UseStringInlining'])
202+
base_image_build_args += ['-H:+UseStringInlining']
204203
if vm.is_llvm:
205-
self.base_image_build_args += ['--features=org.graalvm.home.HomeFinderFeature'] + svm_experimental_options(['-H:CompilerBackend=llvm', '-H:DeadlockWatchdogInterval=0'])
204+
base_image_build_args += ['--features=org.graalvm.home.HomeFinderFeature'] + ['-H:CompilerBackend=llvm', '-H:DeadlockWatchdogInterval=0']
206205
if vm.gc:
207-
self.base_image_build_args += ['--gc=' + vm.gc] + svm_experimental_options(['-H:+SpawnIsolates'])
206+
base_image_build_args += ['--gc=' + vm.gc] + ['-H:+SpawnIsolates']
208207
if vm.native_architecture:
209-
self.base_image_build_args += ['-march=native']
208+
base_image_build_args += ['-march=native']
210209
if vm.analysis_context_sensitivity:
211-
self.base_image_build_args += svm_experimental_options(['-H:AnalysisContextSensitivity=' + vm.analysis_context_sensitivity, '-H:-RemoveSaturatedTypeFlows', '-H:+AliasArrayTypeFlows'])
210+
base_image_build_args += ['-H:AnalysisContextSensitivity=' + vm.analysis_context_sensitivity, '-H:-RemoveSaturatedTypeFlows', '-H:+AliasArrayTypeFlows']
212211
if vm.no_inlining_before_analysis:
213-
self.base_image_build_args += svm_experimental_options(['-H:-InlineBeforeAnalysis'])
212+
base_image_build_args += ['-H:-InlineBeforeAnalysis']
214213
if vm.optimization_level:
215-
self.base_image_build_args += ['-' + vm.optimization_level]
214+
base_image_build_args += ['-' + vm.optimization_level]
216215
if vm.async_sampler:
217-
self.base_image_build_args += ['-R:+FlightRecorder',
216+
base_image_build_args += ['-R:+FlightRecorder',
218217
'-R:StartFlightRecording=filename=default.jfr',
219218
'--enable-monitoring=jfr']
220219
for stage in ('instrument-image', 'instrument-run'):
221220
if stage in self.stages:
222221
self.stages.remove(stage)
223222
if self.image_vm_args is not None:
224-
self.base_image_build_args += self.image_vm_args
223+
base_image_build_args += self.image_vm_args
225224
self.is_runnable = self.check_runnable()
226-
self.base_image_build_args += self.extra_image_build_arguments
225+
base_image_build_args += self.extra_image_build_arguments
226+
# benchmarks are allowed to use experimental options
227+
self.base_image_build_args = [os.path.join(vm.home(), 'bin', 'native-image')] + svm_experimental_options(base_image_build_args)
227228

228229
def check_runnable(self):
229230
# TODO remove once there is load available for the specified benchmarks

0 commit comments

Comments
 (0)