[GR-46169] Remove SubstrateOptions.UseOldDebugInfo.

PullRequest: graal/14606

Author: olpaw

docs/reference-manual/native-image/DebugInfo.md

@@ -991,6 +991,15 @@ When `callgrind` is used in combination with a viewer like
 information about native image execution aand relate it back to
 specific source code lines.
 
-### Related Documentation
+### Call-graph recording with `perf record`
+
+Normally when perf does stack frame recording (i.e. when `--call-graph` is used), it uses frame pointers to recognize the individual stack frames.
+This assumes that the executable that gets profiled actually preserves frame pointers whenever a function gets called.
+For native images, this can be achieved by using `-H:+PreserveFramePointer` as an image build argument.
+
+An alternative solution is to make perf use dwarf debug info (specifically debug_frame data) to help unwind stack frames.
+To make this work, the image needs to be built with `-g` (to generate debuginfo), and `perf record` needs to use the argument `--call-graph dwarf` to make sure dwarf debug info (instead of frame pointers) is used for stack unwinding.
+
+## Related Documentation
 
 - [Debug Native Executables with GDB](guides/debug-native-executables-with-gdb.md)

sdk/mx.sdk/mx_sdk_vm_impl.py

@@ -1140,7 +1140,7 @@ class SvmSupport(object):
     def __init__(self):
         self._svm_supported = has_component('svm', stage1=True)
         self._svm_ee_supported = self._svm_supported and has_component('svmee', stage1=True)
-        self._debug_supported = self._svm_supported and (mx.is_linux() or mx.is_windows() or (mx.is_darwin() and has_component('svmee', stage1=True)))
+        self._debug_supported = self._svm_supported and (mx.is_linux() or mx.is_windows())
         self._separate_debuginfo_ext = {
             'linux': '.debug',
             'windows': '.pdb',
@@ -1182,8 +1182,6 @@ def separate_debuginfo_ext(self):
     def get_debug_flags(self, image_config):
         assert self.is_debug_supported()
         flags = ['-g']
-        if mx.is_darwin():
-            flags += ['-H:+UseOldDebugInfo']
         if self.generate_separate_debug_info(image_config):
             flags += ['-H:+StripDebugInfo']
         return flags

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

@@ -631,7 +631,7 @@ public static int codeAlignment() {
 
     @APIOption(name = "-g", fixedValue = "2", customHelp = "generate debugging information")//
     @Option(help = "Insert debug info into the generated native image or library")//
-    public static final HostedOptionKey<Integer> GenerateDebugInfo = new HostedOptionKey<>(0, SubstrateOptions::validateGenerateDebugInfo) {
+    static final HostedOptionKey<Integer> GenerateDebugInfo = new HostedOptionKey<>(0, SubstrateOptions::validateGenerateDebugInfo) {
         @Override
         protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Integer oldValue, Integer newValue) {
             if (OS.WINDOWS.isCurrent()) {
@@ -642,39 +642,17 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Integer o
     };
 
     private static void validateGenerateDebugInfo(HostedOptionKey<Integer> optionKey) {
-        if (OS.getCurrent() == OS.DARWIN && optionKey.hasBeenSet() && optionKey.getValue() > 0 && !SubstrateOptions.UseOldDebugInfo.getValue()) {
+        if (OS.getCurrent() == OS.DARWIN && optionKey.hasBeenSet() && optionKey.getValue() > 0) {
             LogUtils.warning("Using %s is not supported on macOS", SubstrateOptionsParser.commandArgument(optionKey, optionKey.getValue().toString()));
         }
     }
 
-    @Option(help = "Control debug information output: 0 - no debuginfo, 1 - AOT code debuginfo, 2 - AOT and runtime code debuginfo (runtime code support only with -H:+UseOldDebugInfo).", //
-                    deprecated = true, deprecationMessage = "Please use the -g option.")//
-    public static final HostedOptionKey<Integer> Debug = new HostedOptionKey<>(0) {
-        @Override
-        public void update(EconomicMap<OptionKey<?>, Object> values, Object newValue) {
-            GenerateDebugInfo.update(values, newValue);
-        }
-    };
-
-    @Option(help = "Use old debuginfo", deprecated = true, deprecationMessage = "Please use the -g option.")//
-    public static final HostedOptionKey<Boolean> UseOldDebugInfo = new HostedOptionKey<>(false, SubstrateOptions::validateUseOldDebugInfo);
-
     public static boolean useDebugInfoGeneration() {
-        return useLIRBackend() && GenerateDebugInfo.getValue() > 0 && !UseOldDebugInfo.getValue();
+        return useLIRBackend() && GenerateDebugInfo.getValue() > 0;
     }
 
-    private static void validateUseOldDebugInfo(HostedOptionKey<Boolean> optionKey) {
-        if (optionKey.getValue() && SubstrateOptions.GenerateDebugInfo.getValue() < 1) {
-            throw UserError.abort("The option '%s' can only be used together with '%s'.",
-                            SubstrateOptionsParser.commandArgument(optionKey, "+"), SubstrateOptionsParser.commandArgument(SubstrateOptions.GenerateDebugInfo, "2"));
-        }
-    }
-
-    @Option(help = "Search path for source files for Application or GraalVM classes (list of comma-separated directories or jar files)")//
-    public static final HostedOptionKey<LocatableMultiOptionValue.Paths> DebugInfoSourceSearchPath = new HostedOptionKey<>(LocatableMultiOptionValue.Paths.buildWithCommaDelimiter());
-
     @Option(help = "Directory under which to create source file cache for Application or GraalVM classes")//
-    public static final HostedOptionKey<String> DebugInfoSourceCacheRoot = new HostedOptionKey<>("sources");
+    static final HostedOptionKey<String> DebugInfoSourceCacheRoot = new HostedOptionKey<>("sources");
 
     @Option(help = "Temporary option to disable checking of image builder module dependencies or increasing its verbosity", type = OptionType.Debug)//
     public static final HostedOptionKey<Integer> CheckBootModuleDependencies = new HostedOptionKey<>(ModuleSupport.modulePathBuild ? 1 : 0);

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

@@ -1525,7 +1525,6 @@ private static void deprecatedSanitizeJVMEnvironment(Map<String, String> environ
 
     private static void sanitizeJVMEnvironment(Map<String, String> environment, Map<String, String> imageBuilderEnvironment) {
         Set<String> requiredKeys = new HashSet<>(List.of("PATH", "PWD", "HOME", "LANG", "LC_ALL"));
-        requiredKeys.add("SRCHOME"); /* Remove once GR-44676 is fixed */
         Function<String, String> keyMapper;
         if (OS.WINDOWS.isCurrent()) {
             requiredKeys.addAll(List.of("TEMP", "INCLUDE", "LIB"));

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/codegen/CCompilerInvoker.java

@@ -516,7 +516,11 @@ protected boolean detectError(String line) {
     }
 
     public static Optional<Path> lookupSearchPath(String name) {
-        return Arrays.stream(System.getenv("PATH").split(File.pathSeparator))
+        String envPath = System.getenv("PATH");
+        if (envPath == null) {
+            return Optional.empty();
+        }
+        return Arrays.stream(envPath.split(File.pathSeparator))
                         .map(entry -> Paths.get(entry, name))
                         .filter(p -> Files.isExecutable(p) && !Files.isDirectory(p))
                         .findFirst();

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/CCLinkerInvocation.java

@@ -242,7 +242,8 @@ public void addNativeLinkerOption(String option) {
 
     protected List<String> getNativeLinkerOptions() {
         return Stream.of(nativeLinkerOptions, Options.NativeLinkerOption.getValue().values())
-                        .flatMap(Collection::stream).collect(Collectors.toList());
+                        .flatMap(Collection::stream)
+                        .collect(Collectors.toList());
     }
 
     private static class BinutilsCCLinkerInvocation extends CCLinkerInvocation {
@@ -484,7 +485,7 @@ public List<String> getCommand() {
             /* Put .lib and .exp files in a temp dir as we don't usually need them. */
             cmd.add("/IMPLIB:" + getTempDirectory().resolve(imageName + ".lib"));
 
-            if (SubstrateOptions.GenerateDebugInfo.getValue() > 0) {
+            if (SubstrateOptions.useDebugInfoGeneration()) {
                 cmd.add("/DEBUG");
 
                 if (SubstrateOptions.DeleteLocalSymbols.getValue()) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageDebugInfoStripFeature.java

@@ -52,7 +52,7 @@ public class NativeImageDebugInfoStripFeature implements InternalFeature {
 
     @Override
     public boolean isInConfiguration(IsInConfigurationAccess access) {
-        return SubstrateOptions.useDebugInfoGeneration() || SubstrateOptions.UseOldDebugInfo.getValue();
+        return SubstrateOptions.useDebugInfoGeneration() && SubstrateOptions.StripDebugInfo.getValue();
     }
 
     @SuppressWarnings("try")

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageViaCC.java

@@ -163,10 +163,7 @@ private void runLinkerCommand(String imageName, LinkerInvocation inv, List<Strin
                 BuildArtifacts.singleton().add(ArtifactType.IMPORT_LIBRARY, importLibCopy);
             }
 
-            if (SubstrateOptions.GenerateDebugInfo.getValue() > 0) {
-                if (SubstrateOptions.UseOldDebugInfo.getValue()) {
-                    return;
-                }
+            if (SubstrateOptions.useDebugInfoGeneration()) {
                 BuildArtifacts.singleton().add(ArtifactType.DEBUG_INFO, SubstrateOptions.getDebugInfoSourceCacheRoot());
                 if (Platform.includedIn(Platform.WINDOWS.class)) {
                     BuildArtifacts.singleton().add(ArtifactType.DEBUG_INFO, imagePath.resolveSibling(imageName + ".pdb"));

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/sources/SourceCache.java

@@ -39,11 +39,14 @@
 import java.util.HashMap;
 import java.util.List;
 
+import org.graalvm.compiler.options.Option;
 import org.graalvm.nativeimage.ImageSingletons;
 
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
+import com.oracle.svm.core.option.HostedOptionKey;
+import com.oracle.svm.core.option.LocatableMultiOptionValue;
 import com.oracle.svm.core.util.VMError;
 import com.oracle.svm.hosted.FeatureImpl;
 import com.oracle.svm.hosted.ImageClassLoader;
@@ -504,6 +507,12 @@ protected static void ensureTargetDirs(Path targetDir) {
 @AutomaticallyRegisteredFeature
 @SuppressWarnings("unused")
 class SourceCacheFeature implements InternalFeature {
+
+    public static class Options {
+        @Option(help = "Search path for source files for application or GraalVM classes (list of comma-separated directories or jar files)")//
+        static final HostedOptionKey<LocatableMultiOptionValue.Paths> DebugInfoSourceSearchPath = new HostedOptionKey<>(LocatableMultiOptionValue.Paths.buildWithCommaDelimiter());
+    }
+
     ImageClassLoader imageClassLoader;
 
     @Override
@@ -520,7 +529,7 @@ static List<Path> getModulePath() {
     }
 
     static List<Path> getSourceSearchPath() {
-        return SubstrateOptions.DebugInfoSourceSearchPath.getValue().values();
+        return Options.DebugInfoSourceSearchPath.getValue().values();
     }
 }