Cleanup and changes based on reviews

Author: dominikmascherbauer

substratevm/CHANGELOG.md

@@ -9,7 +9,7 @@ This changelog summarizes major changes to GraalVM Native Image.
 * (GR-59864) Added JVM version check to the Native Image agent. The agent will abort execution if the JVM major version does not match the version it was built with, and warn if the full JVM version is different.
 * (GR-59135) Verify if hosted options passed to `native-image` exist prior to starting the builder. Provide suggestions how to fix unknown options early on.
 * (GR-61492) The experimental JDWP option is now present in standard GraalVM builds.
-* (GR-54697) Parallelize debug info generator and add support run-time debug info generation. `-H:+RuntimeDebugInfo` adds a run-time debug info generator into a native image. The run-time debug info generator notifies GDB via the [JIT Compilation Interface](https://sourceware.org/gdb/current/onlinedocs/gdb.html/JIT-Interface.html) about new object files for run-time compilations.
+* (GR-54697) Parallelize debug info generation and add support for run-time debug info generation. `-H:+RuntimeDebugInfo` adds a run-time debug info generator into a native image for use with GDB.
 
 ## GraalVM for JDK 24 (Internal Version 24.2.0)
 * (GR-59717) Added `DuringSetupAccess.registerObjectReachabilityHandler` to allow registering a callback that is executed when an object of a specified type is marked as reachable during heap scanning.

substratevm/debug/gdbpy/gdb-debughelpers.py

@@ -97,14 +97,6 @@ class SVMUtil:
 
     hlreps = dict()
 
-    # AMD64 registers
-    AMD64_RSP = 7
-    AMD64_RIP = 16
-
-    # aarch64 registers
-    AARCH64_RSP = 30
-    AARCH64_RIP = 31
-
     # static methods
     @staticmethod
     def get_deopt_stub_adr() -> int:
@@ -143,24 +135,6 @@ def prompt_hook(cls, current_prompt: str = None):
         cls.selfref_cycles.clear()
         SVMCommandPrint.cache.clear()
 
-    @classmethod
-    def get_rsp(cls) -> int:
-        arch = gdb.selected_frame().architecture().name()
-        if "x86-64" in arch:
-            return cls.AMD64_RSP
-        elif "aarch64" in arch:
-            return cls.AARCH64_RSP
-        return 0
-
-    @classmethod
-    def get_rip(cls) -> int:
-        arch = gdb.selected_frame().architecture().name()
-        if "x86-64" in arch:
-            return cls.AMD64_RIP
-        elif "aarch64" in arch:
-            return cls.AARCH64_RIP
-        return 0
-
     @classmethod
     # checks if node this is reachable from node other (this node is parent of other node)
     def is_reachable(cls, this: hex, other: hex) -> bool:
@@ -1626,26 +1600,26 @@ def __call__(self, pending_frame: gdb.Frame):
         if self.deopt_stub_adr == 0:
             self.deopt_stub_adr = SVMUtil.get_deopt_stub_adr()
 
-        rsp = 0
+        sp = 0
         try:
-            rsp = pending_frame.read_register('sp')
-            rip = pending_frame.read_register('pc')
-            if int(rip) == self.deopt_stub_adr:
-                deopt_frame_stack_slot = rsp.cast(self.svm_util.stack_type.pointer()).dereference()
+            sp = pending_frame.read_register('sp')
+            pc = pending_frame.read_register('pc')
+            if int(pc) == self.deopt_stub_adr:
+                deopt_frame_stack_slot = sp.cast(self.svm_util.stack_type.pointer()).dereference()
                 deopt_frame = deopt_frame_stack_slot.cast(self.svm_util.get_compressed_type(self.svm_util.object_type).pointer())
                 rtt = self.svm_util.get_rtt(deopt_frame)
                 deopt_frame = self.svm_util.cast_to(deopt_frame, rtt)
                 encoded_frame_size = self.svm_util.get_int_field(deopt_frame, 'sourceEncodedFrameSize')
                 source_frame_size = encoded_frame_size & ~self.svm_util.frame_size_status_mask
                 # Now find the register-values for the caller frame
-                unwind_info = pending_frame.create_unwind_info(gdb.unwinder.FrameId(rsp, rip))
-                caller_rsp = rsp + int(source_frame_size)
-                unwind_info.add_saved_register(SVMUtil.get_rsp(), gdb.Value(caller_rsp))
-                caller_rip = gdb.Value(caller_rsp - 8).cast(self.svm_util.stack_type.pointer()).dereference()
-                unwind_info.add_saved_register(SVMUtil.get_rip(), gdb.Value(caller_rip))
+                unwind_info = pending_frame.create_unwind_info(gdb.unwinder.FrameId(sp, pc))
+                caller_sp = sp + int(source_frame_size)
+                unwind_info.add_saved_register('sp', gdb.Value(caller_sp))
+                caller_pc = gdb.Value(caller_sp - 8).cast(self.svm_util.stack_type.pointer()).dereference()
+                unwind_info.add_saved_register('pc', gdb.Value(caller_pc))
                 return unwind_info
         except Exception as ex:
-            trace(f'<SVMFrameUnwinder> - Failed to unwind frame at {hex(rsp)}')
+            trace(f'<SVMFrameUnwinder> - Failed to unwind frame at {hex(sp)}')
             trace(ex)
             # Fallback to default frame unwinding via debug_frame (dwarf)
 
@@ -1712,9 +1686,9 @@ def __init__(self, svm_util: SVMUtil, frame: gdb.Frame, deopt_stub_adr: int):
         super().__init__(frame)
 
         # fetch deoptimized frame from stack
-        rsp = frame.read_register('sp')
+        sp = frame.read_register('sp')
         assert deopt_stub_adr and frame.pc() == deopt_stub_adr
-        deopt_frame_stack_slot = rsp.cast(svm_util.stack_type.pointer()).dereference()
+        deopt_frame_stack_slot = sp.cast(svm_util.stack_type.pointer()).dereference()
         deopt_frame = deopt_frame_stack_slot.cast(svm_util.get_compressed_type(svm_util.object_type).pointer())
         rtt = svm_util.get_rtt(deopt_frame)
         deopt_frame = svm_util.cast_to(deopt_frame, rtt)
@@ -1754,11 +1728,13 @@ def filename(self):
             return None
 
         source_class = self.__svm_util.get_obj_field(self.__frame_info, 'sourceClass')
+        companion = self.__svm_util.get_obj_field(source_class, 'companion')
+        source_file_name = self.__svm_util.get_obj_field(companion, 'sourceFileName')
 
-        if self.__svm_util.is_null(source_class):
+        if self.__svm_util.is_null(source_file_name):
             source_file_name = ''
         else:
-            source_file_name = str(self.__svm_util.get_obj_field(source_class, 'sourceFileName'))[1:-1]
+            source_file_name = str(source_file_name)[1:-1]
 
         return source_file_name
 

substratevm/mx.substratevm/mx_substratevm.py

@@ -1146,7 +1146,7 @@ def run_debug_test(image_name: str, testfile: str, source_path: str, with_isolat
                 '-iex', f"set auto-load safe-path {join(build_dir, 'gdb-debughelpers.py')}",
                 '-x', testfile, join(build_dir, image_name)
             ]
-            mx.log(' '.join([(f"'{c}'" if ' ' in c else c) for c in gdb_command]))
+            log_gdb_command(gdb_command)
             # unittest may result in different exit code, nonZeroIsFatal ensures that we can go on with other test
             return mx.run(gdb_command, cwd=build_dir, nonZeroIsFatal=False)
         return 0
@@ -1173,7 +1173,11 @@ def run_debug_test(image_name: str, testfile: str, source_path: str, with_isolat
         mx.abort(status)
 
 
-def _runtimedebuginfotest(native_image, output_path, with_isolates_only, args=None):
+def log_gdb_command(gdb_command):
+    mx.log(' '.join([(f"'{c}'" if ' ' in c else c) for c in gdb_command]))
+
+
+def _runtimedebuginfotest(native_image, output_path, args=None):
     """Build and run the runtimedebuginfotest"""
 
     args = [] if args is None else args
@@ -1225,7 +1229,7 @@ def _runtimedebuginfotest(native_image, output_path, with_isolates_only, args=No
         '-iex', f"set auto-load safe-path {join(output_path, 'gdb-debughelpers.py')}",
         '-x', test_runtime_compilation_py, runtime_compile_binary
     ]
-    mx.log(' '.join([(f"'{c}'" if ' ' in c else c) for c in gdb_command]))
+    log_gdb_command(gdb_command)
     # unittest may result in different exit code, nonZeroIsFatal ensures that we can go on with other test
     status = mx.run(gdb_command, cwd=output_path, nonZeroIsFatal=False)
 
@@ -1247,7 +1251,7 @@ def _runtimedebuginfotest(native_image, output_path, with_isolates_only, args=No
         '-iex', f"set auto-load safe-path {join(output_path, 'gdb-debughelpers.py')}",
         '-x', test_runtime_deopt_py, '--args', js_launcher, testdeopt_js
     ]
-    mx.log(' '.join([(f"'{c}'" if ' ' in c else c) for c in gdb_command]))
+    log_gdb_command(gdb_command)
     # unittest may result in different exit code, nonZeroIsFatal ensures that we can go on with other test
     status |= mx.run(gdb_command, cwd=output_path, nonZeroIsFatal=False)
 
@@ -1855,14 +1859,12 @@ def runtimedebuginfotest(args, config=None):
     all_args = ['--output-path', '--with-isolates-only']
     masked_args = [_mask(arg, all_args) for arg in args]
     parser.add_argument(all_args[0], metavar='<output-path>', nargs=1, help='Path of the generated image', default=[join(svmbuild_dir(), "runtimedebuginfotest")])
-    parser.add_argument(all_args[1], action='store_true', help='Only build and test the native image with isolates')
     parser.add_argument('image_args', nargs='*', default=[])
     parsed = parser.parse_args(masked_args)
     output_path = unmask(parsed.output_path)[0]
-    with_isolates_only = parsed.with_isolates_only
     native_image_context_run(
         lambda native_image, a:
-        _runtimedebuginfotest(native_image, output_path, with_isolates_only, a), unmask(parsed.image_args),
+        _runtimedebuginfotest(native_image, output_path, a), unmask(parsed.image_args),
         config=config
     )
 

substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/ObjectFile.java

@@ -223,7 +223,7 @@ private static ObjectFile getNativeObjectFile(int pageSize, boolean runtimeDebug
             case ELF -> new ELFObjectFile(pageSize, runtimeDebugInfoGeneration);
             case MACH_O -> new MachOObjectFile(pageSize);
             case PECOFF -> new PECoffObjectFile(pageSize);
-            default -> throw new AssertionError("Unreachable");
+            case LLVM -> throw new AssertionError("Unsupported NativeObjectFile for format " + ObjectFile.getNativeFormat());
         };
     }
 

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

@@ -45,7 +45,6 @@
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
-import org.graalvm.nativeimage.ProcessProperties;
 
 import com.oracle.svm.core.c.libc.LibCBase;
 import com.oracle.svm.core.c.libc.MuslLibC;
@@ -476,22 +475,6 @@ public static void setImageLayerCreateEnabledHandler(OptionEnabledHandler<Boolea
     @Option(help = "Track NodeSourcePositions during runtime-compilation")//
     public static final HostedOptionKey<Boolean> IncludeNodeSourcePositions = new HostedOptionKey<>(false);
 
-    @Option(help = "Directory where Java source-files will be placed for the debugger")//
-    public static final RuntimeOptionKey<String> RuntimeSourceDestDir = new RuntimeOptionKey<>(null, RelevantForCompilationIsolates);
-
-    public static Path getRuntimeSourceDestDir() {
-        String sourceDestDir = RuntimeSourceDestDir.getValue();
-        if (sourceDestDir != null) {
-            return Paths.get(sourceDestDir);
-        }
-        Path result = Paths.get("sources");
-        Path exeDir = Paths.get(ProcessProperties.getExecutableName()).getParent();
-        if (exeDir != null) {
-            result = exeDir.resolve(result);
-        }
-        return result;
-    }
-
     @Option(help = "Provide debuginfo for runtime-compiled code.")//
     public static final HostedOptionKey<Boolean> RuntimeDebugInfo = new HostedOptionKey<>(false);
 
@@ -1023,6 +1006,17 @@ public static boolean useDebugInfoGeneration() {
         return useLIRBackend() && GenerateDebugInfo.getValue() > 0;
     }
 
+    // TODO: change default to 0
+    @Option(help = "Number of threads used to generate debug info.", deprecated = true) //
+    public static final HostedOptionKey<Integer> DebugInfoGenerationThreadCount = new HostedOptionKey<>(256, SubstrateOptions::validateDebugInfoGenerationThreadCount);
+
+    private static void validateDebugInfoGenerationThreadCount(HostedOptionKey<Integer> optionKey) {
+        int value = optionKey.getValue();
+        if (value <= 0) {
+            throw UserError.invalidOptionValue(optionKey, value, "The value must be bigger than 0");
+        }
+    }
+
     @Option(help = "Directory under which to create source file cache for Application or GraalVM classes")//
     static final HostedOptionKey<String> DebugInfoSourceCacheRoot = new HostedOptionKey<>("sources");
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserverSupport.java

@@ -45,13 +45,10 @@
 
 @AutomaticallyRegisteredImageSingleton
 public final class InstalledCodeObserverSupport {
-    private static final InstalledCodeObserverHandleAction ACTION_ATTACH = (h, a, i) -> getAccessor(h).attachToCurrentIsolate(h);
-    private static final InstalledCodeObserverHandleAction ACTION_DETACH = (h, a, i) -> getAccessor(h).detachFromCurrentIsolate(h);
-    private static final InstalledCodeObserverHandleAction ACTION_RELEASE = (h, a, i) -> {
-        getAccessor(h).release(h);
-        NonmovableArrays.setWord(a, i, Word.nullPointer());
-    };
-    private static final InstalledCodeObserverHandleAction ACTION_ACTIVATE = (h, a, i) -> getAccessor(h).activate(h);
+    private static final InstalledCodeObserverHandleAction ACTION_ATTACH = h -> getAccessor(h).attachToCurrentIsolate(h);
+    private static final InstalledCodeObserverHandleAction ACTION_DETACH = h -> getAccessor(h).detachFromCurrentIsolate(h);
+    private static final InstalledCodeObserverHandleAction ACTION_RELEASE = h -> getAccessor(h).release(h);
+    private static final InstalledCodeObserverHandleAction ACTION_ACTIVATE = h -> getAccessor(h).activate(h);
 
     private final List<InstalledCodeObserver.Factory> observerFactories = new ArrayList<>();
 
@@ -106,10 +103,11 @@ public static void attachToCurrentIsolate(NonmovableArray<InstalledCodeObserverH
 
     public static void removeObservers(NonmovableArray<InstalledCodeObserverHandle> observerHandles) {
         forEach(observerHandles, ACTION_RELEASE);
+        clearObserverHandles(observerHandles);
     }
 
     private interface InstalledCodeObserverHandleAction {
-        void invoke(InstalledCodeObserverHandle handle, NonmovableArray<InstalledCodeObserverHandle> observerHandles, int index);
+        void invoke(InstalledCodeObserverHandle handle);
     }
 
     private static void forEach(NonmovableArray<InstalledCodeObserverHandle> array, InstalledCodeObserverHandleAction action) {
@@ -118,7 +116,19 @@ private static void forEach(NonmovableArray<InstalledCodeObserverHandle> array,
             for (int i = 0; i < length; i++) {
                 InstalledCodeObserverHandle handle = NonmovableArrays.getWord(array, i);
                 if (handle.isNonNull()) {
-                    action.invoke(handle, array, i);
+                    action.invoke(handle);
+                }
+            }
+        }
+    }
+
+    private static void clearObserverHandles(NonmovableArray<InstalledCodeObserverHandle> array) {
+        if (array.isNonNull()) {
+            int length = NonmovableArrays.lengthOf(array);
+            for (int i = 0; i < length; i++) {
+                InstalledCodeObserverHandle handle = NonmovableArrays.getWord(array, i);
+                if (handle.isNonNull()) {
+                    NonmovableArrays.setWord(array, i, Word.nullPointer());
                 }
             }
         }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/debug/GDBJITInterface.java renamed to substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/debug/GdbJitInterface.java

@@ -50,10 +50,10 @@
 
 import jdk.graal.compiler.word.Word;
 
-@CContext(GDBJITInterface.GDBJITInterfaceDirectives.class)
-public class GDBJITInterface {
+@CContext(GdbJitInterface.GdbJitInterfaceDirectives.class)
+public class GdbJitInterface {
 
-    public static class GDBJITInterfaceDirectives implements CContext.Directives {
+    public static class GdbJitInterfaceDirectives implements CContext.Directives {
         @Override
         public boolean isInConfiguration() {
             return SubstrateOptions.RuntimeDebugInfo.getValue();

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/debug/SharedDebugInfoProvider.java

@@ -788,7 +788,7 @@ protected long getTypeSignature(String typeName) {
     protected String getMethodName(SharedMethod method) {
         String name = method.getName();
         // replace <init> (method name of a constructor) with the class name
-        if (name.equals("<init>")) {
+        if (method.isConstructor()) {
             name = method.format("%h");
             if (name.indexOf('$') >= 0) {
                 name = name.substring(name.lastIndexOf('$') + 1);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/debug/SubstrateDebugInfoFeature.java

@@ -55,6 +55,6 @@ public void registerCodeObserver(RuntimeConfiguration runtimeConfig) {
         // This is called at image build-time -> the factory then creates a RuntimeDebugInfoProvider
         // at runtime
         ImageSingletons.lookup(InstalledCodeObserverSupport.class).addObserverFactory(new SubstrateDebugInfoInstaller.Factory(runtimeConfig.getProviders().getMetaAccess(), runtimeConfig));
-        ImageSingletons.add(SubstrateDebugInfoInstaller.GDBJITAccessor.class, new SubstrateDebugInfoInstaller.GDBJITAccessor());
+        ImageSingletons.add(SubstrateDebugInfoInstaller.GdbJitAccessor.class, new SubstrateDebugInfoInstaller.GdbJitAccessor());
     }
 }