Skip to content

Commit

Permalink
[GR-51524] Fix and improve invalid option reporting.
Browse files Browse the repository at this point in the history 
PullRequest: graal/16681
  • Loading branch information
@fniephaus
fniephaus committed Jan 23, 2024
2 parents d29bb9f + 0e5dbbd commit 8cde43a
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 36 deletions.
11 changes: 5 additions & 6 deletions ...e.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/SerialAndEpsilonGCOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@
package com.oracle.svm.core.genscavenge;

import org.graalvm.collections.EconomicMap;
import jdk.graal.compiler.options.Option;
import jdk.graal.compiler.options.OptionKey;
import jdk.graal.compiler.options.OptionType;

import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.option.HostedOptionKey;
import com.oracle.svm.core.option.NotifyGCRuntimeOptionKey;
import com.oracle.svm.core.option.RuntimeOptionKey;
import com.oracle.svm.core.util.InterruptImageBuilding;
import com.oracle.svm.core.util.UserError;

import jdk.graal.compiler.options.Option;
import jdk.graal.compiler.options.OptionKey;
import jdk.graal.compiler.options.OptionType;

/** Common options that can be specified for both the serial and the epsilon GC. */
public final class SerialAndEpsilonGCOptions {
@Option(help = "The maximum heap size as percent of physical memory. Serial and epsilon GC only.", type = OptionType.User) //
Expand Down Expand Up @@ -77,8 +77,7 @@ private SerialAndEpsilonGCOptions() {

public static void serialOrEpsilonGCOnly(OptionKey<?> optionKey) {
if (!SubstrateOptions.UseSerialGC.getValue() && !SubstrateOptions.UseEpsilonGC.getValue()) {
throw new InterruptImageBuilding(
"The option '" + optionKey.getName() + "' can only be used together with the serial ('--gc=serial') or the epsilon garbage collector ('--gc=epsilon').");
throw UserError.abort("The option '" + optionKey.getName() + "' can only be used together with the serial ('--gc=serial') or the epsilon garbage collector ('--gc=epsilon').");
}
}
}
3 changes: 1 addition & 2 deletions .../com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/SerialGCOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.option.HostedOptionKey;
import com.oracle.svm.core.option.RuntimeOptionKey;
import com.oracle.svm.core.util.InterruptImageBuilding;
import com.oracle.svm.core.util.UserError;

import jdk.graal.compiler.options.Option;
Expand Down Expand Up @@ -112,7 +111,7 @@ private SerialGCOptions() {

private static void serialGCOnly(OptionKey<?> optionKey) {
if (!SubstrateOptions.UseSerialGC.getValue()) {
throw new InterruptImageBuilding("The option '" + optionKey.getName() + "' can only be used together with the serial garbage collector ('--gc=serial').");
throw UserError.abort("The option '" + optionKey.getName() + "' can only be used together with the serial garbage collector ('--gc=serial').");
}
}
}
20 changes: 10 additions & 10 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
import com.oracle.svm.core.option.RuntimeOptionKey;
import com.oracle.svm.core.option.SubstrateOptionsParser;
import com.oracle.svm.core.thread.VMOperationControl;
import com.oracle.svm.core.util.InterruptImageBuilding;
import com.oracle.svm.core.util.UserError;
import com.oracle.svm.util.LogUtils;
import com.oracle.svm.util.ModuleSupport;
Expand Down Expand Up @@ -107,7 +106,7 @@ public class SubstrateOptions {
@Option(help = "Build statically linked executable (requires static libc and zlib)")//
public static final HostedOptionKey<Boolean> StaticExecutable = new HostedOptionKey<>(false, key -> {
if (!Platform.includedIn(Platform.LINUX.class)) {
throw new InterruptImageBuilding("Building static executable images is currently only supported on Linux. Remove the '--static' option or build on a Linux machine.");
throw UserError.invalidOptionValue(key, key.getValue(), "Building static executable images is currently only supported on Linux. Remove the '--static' option or build on a Linux machine");
}
});

Expand Down Expand Up @@ -291,7 +290,7 @@ private static OptimizationLevel parseOptimizationLevel(String value) {
// We allow all positive numbers, and treat that as our current highest supported level.
return OptimizationLevel.O3;
} else {
throw UserError.abort("Invalid value '%s' provided for option Optimize (expected 'b' or numeric value >= 0)", value);
throw UserError.invalidOptionValue(Optimize, value, "Accepted values are 'b' or numeric value >= 0");
}
}

Expand Down Expand Up @@ -489,7 +488,7 @@ public static final boolean hasColorsEnabled(OptionValues values) {
yield false;
}
case "never" -> false;
default -> throw UserError.abort("Unsupported value '%s' for '--color' option. Only 'always', 'never', and 'auto' are accepted.", value);
default -> throw UserError.invalidOptionValue(Color, value, "Only 'always', 'never', and 'auto' are accepted values");
};
}
return false;
Expand Down Expand Up @@ -563,7 +562,7 @@ public static final boolean hasColorsEnabled(OptionValues values) {
private static void validateAdditionalHeaderBytes(HostedOptionKey<Integer> optionKey) {
int value = optionKey.getValue();
if (value < 0 || value % 4 != 0) {
throw UserError.abort("The option '%s' must be 0 or a multiple of 4.", optionKey.getName());
throw UserError.invalidOptionValue(optionKey, value, "The value must be 0 or a positive multiple of 4.");
}
}

Expand Down Expand Up @@ -639,8 +638,8 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, String ol
isLLVMBackendMissing = ReflectionUtil.lookupClass(true, "com.oracle.svm.core.graal.llvm.LLVMFeature") == null;
}
if (isLLVMBackendMissing) {
throw UserError.abort(
"The LLVM backend for GraalVM Native Image is missing and needs to be build from source. " +
throw UserError.invalidOptionValue(CompilerBackend, newValue,
"The LLVM backend for GraalVM Native Image is missing and needs to be built from source. " +
"For instructions, please see https://github.com/oracle/graal/blob/master/docs/reference-manual/native-image/LLVMBackend.md.");
}

Expand Down Expand Up @@ -772,19 +771,20 @@ public static Path getDebugInfoSourceCacheRoot() {
try {
return Paths.get(Path.getValue()).resolve(DebugInfoSourceCacheRoot.getValue());
} catch (InvalidPathException ipe) {
throw UserError.abort("Invalid path provided for option DebugInfoSourceCacheRoot %s", DebugInfoSourceCacheRoot.getValue());
throw UserError.invalidOptionValue(DebugInfoSourceCacheRoot, DebugInfoSourceCacheRoot.getValue(), "The path is invalid");
}
}

@Option(help = "Use a separate file for debug info.")//
public static final HostedOptionKey<Boolean> StripDebugInfo = new HostedOptionKey<>(OS.getCurrent() != OS.DARWIN, SubstrateOptions::validateStripDebugInfo);

private static void validateStripDebugInfo(HostedOptionKey<Boolean> optionKey) {
Boolean optionValue = optionKey.getValue();
if (OS.getCurrent() == OS.DARWIN && optionKey.hasBeenSet() && optionKey.getValue()) {
throw UserError.abort("Using %s is not supported on macOS", SubstrateOptionsParser.commandArgument(SubstrateOptions.StripDebugInfo, "+"));
throw UserError.invalidOptionValue(optionKey, optionValue, "The option is not supported on macOS");
}
if (OS.getCurrent() == OS.WINDOWS && optionKey.hasBeenSet() && !optionKey.getValue()) {
throw UserError.abort("Using %s is not supported on Windows: debug info is always generated in a separate file", SubstrateOptionsParser.commandArgument(optionKey, "-"));
throw UserError.invalidOptionValue(optionKey, optionValue, "The option is not supported on Windows (debug info is always generated in a separate file)");
}
}

Expand Down
29 changes: 29 additions & 0 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/UserError.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
import org.graalvm.nativeimage.Platform;
import org.graalvm.nativeimage.Platforms;

import com.oracle.svm.core.option.SubstrateOptionsParser;

import jdk.graal.compiler.options.OptionKey;
import jdk.vm.ci.meta.ResolvedJavaField;
import jdk.vm.ci.meta.ResolvedJavaMethod;
import jdk.vm.ci.meta.ResolvedJavaType;
Expand Down Expand Up @@ -134,4 +137,30 @@ static Object[] formatArguments(Object... args) {
public static UserException abort(Iterable<String> messages) {
throw new UserException(messages);
}

/**
* Stop compilation immediately and report the invalid use of an option to the user.
*
* @param option the option incorrectly used.
* @param value the value passed to the option, possibly invalid.
* @param reason the reason why the option-value pair is rejected that can be understood by the
* user.
*/
public static UserException invalidOptionValue(OptionKey<?> option, String value, String reason) {
return abort("Invalid option '%s'. %s.", SubstrateOptionsParser.commandArgument(option, value), reason);
}

/**
* @see #invalidOptionValue(OptionKey, String, String)
*/
public static UserException invalidOptionValue(OptionKey<?> option, Boolean value, String reason) {
return invalidOptionValue(option, value ? "+" : "-", reason);
}

/**
* @see #invalidOptionValue(OptionKey, String, String)
*/
public static UserException invalidOptionValue(OptionKey<?> option, Number value, String reason) {
return invalidOptionValue(option, String.valueOf(value), reason);
}
}
6 changes: 5 additions & 1 deletion substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import com.oracle.svm.core.option.HostedOptionKey;
import com.oracle.svm.core.option.LocatableMultiOptionValue;
import com.oracle.svm.core.option.SubstrateOptionsParser;
import com.oracle.svm.core.util.InterruptImageBuilding;
import com.oracle.svm.core.util.UserError;
import com.oracle.svm.core.util.UserError.UserException;
import com.oracle.svm.core.util.VMError;
Expand Down Expand Up @@ -275,13 +276,16 @@ public void dumpAllFeatures(PrintWriter out) {
}

private static UserException handleFeatureError(Feature feature, Throwable throwable) {
/* Avoid wrapping UserErrors and VMErrors. */
/* Avoid wrapping UserError, VMError, and InterruptImageBuilding throwables. */
if (throwable instanceof UserException userError) {
throw userError;
}
if (throwable instanceof HostedError vmError) {
throw vmError;
}
if (throwable instanceof InterruptImageBuilding iib) {
throw iib;
}

String featureClassName = feature.getClass().getName();
String throwableClassName = throwable.getClass().getName();
Expand Down
10 changes: 5 additions & 5 deletions ...atevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGeneratorRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ private int buildImage(ImageClassLoader classLoader) {
}

ProgressReporter reporter = new ProgressReporter(parsedHostedOptions);
Throwable vmError = null;
Throwable unhandledThrowable = null;
boolean wasSuccessfulBuild = false;
try (StopTimer ignored = totalTimer.start()) {
Timer classlistTimer = timerCollection.get(TimerCollection.Registry.CLASSLIST);
Expand Down Expand Up @@ -571,18 +571,18 @@ private int buildImage(ImageClassLoader classLoader) {
}
return ExitStatus.BUILDER_ERROR.getValue();
} catch (Throwable e) {
vmError = e;
unhandledThrowable = e;
return ExitStatus.BUILDER_ERROR.getValue();
} finally {
reportEpilog(imageName, reporter, classLoader, wasSuccessfulBuild, vmError, parsedHostedOptions);
reportEpilog(imageName, reporter, classLoader, wasSuccessfulBuild, unhandledThrowable, parsedHostedOptions);
NativeImageGenerator.clearSystemPropertiesForImage();
ImageSingletonsSupportImpl.HostedManagement.clear();
}
return ExitStatus.OK.getValue();
}

protected void reportEpilog(String imageName, ProgressReporter reporter, ImageClassLoader classLoader, boolean wasSuccessfulBuild, Throwable vmError, OptionValues parsedHostedOptions) {
reporter.printEpilog(Optional.ofNullable(imageName), Optional.ofNullable(generator), classLoader, wasSuccessfulBuild, Optional.ofNullable(vmError), parsedHostedOptions);
protected void reportEpilog(String imageName, ProgressReporter reporter, ImageClassLoader classLoader, boolean wasSuccessfulBuild, Throwable unhandledThrowable, OptionValues parsedHostedOptions) {
reporter.printEpilog(Optional.ofNullable(imageName), Optional.ofNullable(generator), classLoader, wasSuccessfulBuild, Optional.ofNullable(unhandledThrowable), parsedHostedOptions);
}

protected NativeImageGenerator createImageGenerator(ImageClassLoader classLoader, HostedOptionParser optionParser, Pair<Method, CEntryPointData> mainEntryPointData, ProgressReporter reporter) {
Expand Down
25 changes: 13 additions & 12 deletions substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ProgressReporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,21 +686,22 @@ private void printRecommendations() {
}

public void printEpilog(Optional<String> optionalImageName, Optional<NativeImageGenerator> optionalGenerator, ImageClassLoader classLoader, boolean wasSuccessfulBuild,
Optional<Throwable> optionalError, OptionValues parsedHostedOptions) {
Optional<Throwable> optionalUnhandledThrowable, OptionValues parsedHostedOptions) {
executor.shutdown();

if (optionalError.isPresent()) {
if (optionalUnhandledThrowable.isPresent()) {
Path errorReportPath = NativeImageOptions.getErrorFilePath(parsedHostedOptions);
Optional<FeatureHandler> featureHandler = optionalGenerator.map(nativeImageGenerator -> nativeImageGenerator.featureHandler);
ReportUtils.report("GraalVM Native Image Error Report", errorReportPath, p -> VMErrorReporter.generateErrorReport(p, buildOutputLog, classLoader, featureHandler, optionalError.get()),
ReportUtils.report("GraalVM Native Image Error Report", errorReportPath,
p -> VMErrorReporter.generateErrorReport(p, buildOutputLog, classLoader, featureHandler, optionalUnhandledThrowable.get()),
false);
if (ImageSingletonsSupport.isInstalled()) {
BuildArtifacts.singleton().add(ArtifactType.BUILD_INFO, errorReportPath);
}
}

if (optionalImageName.isEmpty() || optionalGenerator.isEmpty()) {
printErrorMessage(optionalError, parsedHostedOptions);
printErrorMessage(optionalUnhandledThrowable, parsedHostedOptions);
return;
}
String imageName = optionalImageName.get();
Expand All @@ -723,26 +724,26 @@ public void printEpilog(Optional<String> optionalImageName, Optional<NativeImage
} else {
timeStats = String.format("%dm %ds", (int) totalSeconds / 60, (int) totalSeconds % 60);
}
l().a(optionalError.isEmpty() ? "Finished" : "Failed").a(" generating '").bold().a(imageName).reset().a("' ")
.a(optionalError.isEmpty() ? "in" : "after").a(" ").a(timeStats).a(".").println();
l().a(wasSuccessfulBuild ? "Finished" : "Failed").a(" generating '").bold().a(imageName).reset().a("' ")
.a(wasSuccessfulBuild ? "in" : "after").a(" ").a(timeStats).a(".").println();

printErrorMessage(optionalError, parsedHostedOptions);
printErrorMessage(optionalUnhandledThrowable, parsedHostedOptions);
}

private void printErrorMessage(Optional<Throwable> optionalError, OptionValues parsedHostedOptions) {
if (optionalError.isEmpty()) {
private void printErrorMessage(Optional<Throwable> optionalUnhandledThrowable, OptionValues parsedHostedOptions) {
if (optionalUnhandledThrowable.isEmpty()) {
return;
}
Throwable error = optionalError.get();
Throwable unhandledThrowable = optionalUnhandledThrowable.get();
l().println();
l().redBold().a("The build process encountered an unexpected error:").reset().println();
if (NativeImageOptions.ReportExceptionStackTraces.getValue(parsedHostedOptions)) {
l().dim().println();
error.printStackTrace(builderIO.getOut());
unhandledThrowable.printStackTrace(builderIO.getOut());
l().reset().println();
} else {
l().println();
l().dim().a("> %s", error).reset().println();
l().dim().a("> %s", unhandledThrowable).reset().println();
l().println();
l().a("Please inspect the generated error report at:").println();
l().link(NativeImageOptions.getErrorFilePath(parsedHostedOptions)).println();
Expand Down

0 comments on commit 8cde43a

Please sign in to comment.