Release code observers only if a CodeInfo is removed from the code cache and update handling of lazy deopt frames.

Update runtimedebuginfotests.
Combine release and releaseOnTeardown for code observers.

Author: dominikmascherbauer

substratevm/debug/gdbpy/gdb-debughelpers.py

@@ -1637,7 +1637,7 @@ def __init__(self, svm_util: SVMUtil):
         self.lazy_deopt_stub_object_adr = None
         self.svm_util = svm_util
 
-    def __call__(self, pending_frame: gdb.Frame):
+    def __call__(self, pending_frame: gdb.PendingFrame):
         if self.eager_deopt_stub_adr is None:
             self.eager_deopt_stub_adr = SVMUtil.get_eager_deopt_stub_adr()
             self.lazy_deopt_stub_primitive_adr = SVMUtil.get_lazy_deopt_stub_primitive_adr()
@@ -1656,20 +1656,23 @@ def __call__(self, pending_frame: gdb.Frame):
                 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(sp, pc))
                 caller_sp = sp + int(source_frame_size)
-                unwind_info.add_saved_register('sp', gdb.Value(caller_sp))
                 # try to fetch return address directly from stack
                 caller_pc = gdb.Value(caller_sp - 8).cast(self.svm_util.stack_type.pointer()).dereference()
+
+                # Build the unwind info
+                unwind_info = pending_frame.create_unwind_info(gdb.unwinder.FrameId(caller_sp, caller_pc))
+                unwind_info.add_saved_register('sp', gdb.Value(caller_sp))
                 unwind_info.add_saved_register('pc', gdb.Value(caller_pc))
                 return unwind_info
             elif int(pc) == self.lazy_deopt_stub_primitive_adr or int(pc) == self.lazy_deopt_stub_object_adr:
-                # Now find the register-values for the caller frame
-                # We only have the original pc for lazy deoptimization -> unwind to original pc with same sp
-                # This is the best guess we can make without knowing the return address and frame size of the lazily deoptimized frame
+                # We only need the original pc for lazy deoptimization -> unwind to original pc with same sp
+                # Since this is lazy deoptimization we can still use the debug frame info at the current sp
+                caller_pc = sp.cast(self.svm_util.stack_type.pointer()).dereference()
+
+                # build the unwind info
                 unwind_info = pending_frame.create_unwind_info(gdb.unwinder.FrameId(sp, pc))
                 unwind_info.add_saved_register('sp', gdb.Value(sp))
-                caller_pc = sp.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:
@@ -1696,18 +1699,26 @@ def filter(self, frame_iter: Iterable) -> FrameDecorator:
             self.lazy_deopt_stub_primitive_adr = SVMUtil.get_lazy_deopt_stub_primitive_adr()
             self.lazy_deopt_stub_object_adr = SVMUtil.get_lazy_deopt_stub_object_adr()
 
+        lazy_deopt = False
         for frame in frame_iter:
             frame = frame.inferior_frame()
             pc = int(frame.pc())
             if pc == self.eager_deopt_stub_adr:
-                yield SVMFrameEagerDeopt(self.svm_util, frame)
+                yield SVMFrameEagerDeopt(frame, self.svm_util)
             elif pc == self.lazy_deopt_stub_primitive_adr or pc == self.lazy_deopt_stub_object_adr:
-                yield SVMFrameLazyDeopt(self.svm_util, frame)
+                lazy_deopt = True
+                continue  # the next frame is the one with the corrected pc
             else:
-                yield SVMFrame(frame)
+                yield SVMFrame(frame, lazy_deopt)
+                lazy_deopt = False
 
 
 class SVMFrame(FrameDecorator):
+
+    def __init__(self, frame: gdb.Frame, lazy_deopt: bool):
+        super().__init__(frame)
+        self.__lazy_deopt = lazy_deopt
+
     def function(self) -> str:
         frame = self.inferior_frame()
         if not frame.name():
@@ -1725,7 +1736,8 @@ def function(self) -> str:
         else:
             eclipse_filename = ''
 
-        return func_name + eclipse_filename
+        prefix = '[LAZY DEOPT FRAME] ' if self.__lazy_deopt else ''
+        return prefix + func_name + eclipse_filename
 
 
 class SymValueWrapper:
@@ -1741,9 +1753,9 @@ def symbol(self):
         return self.sym
 
 
-class SVMFrameEagerDeopt(SVMFrame):
+class SVMFrameEagerDeopt(FrameDecorator):
 
-    def __init__(self, svm_util: SVMUtil, frame: gdb.Frame):
+    def __init__(self, frame: gdb.Frame, svm_util: SVMUtil):
         super().__init__(frame)
 
         # fetch deoptimized frame from stack
@@ -1827,25 +1839,6 @@ def frame_locals(self):
         return None
 
 
-class SVMFrameLazyDeopt(SVMFrame):
-
-    def __init__(self, svm_util: SVMUtil, frame: gdb.Frame):
-        super().__init__(frame)
-
-        # fetch deoptimized frame from stack
-        sp = frame.read_register('sp')
-        real_pc = sp.cast(svm_util.stack_type.pointer()).dereference().cast(svm_util.stack_type.pointer())
-        self.__gdb_text = str(real_pc)
-        self.__svm_util = svm_util
-
-    def function(self) -> str:
-        if self.__gdb_text is None:
-            # we have no more information about the frame
-            return '[LAZY DEOPT FRAME ...]'
-
-        return '[LAZY DEOPT FRAME] at ' + self.__gdb_text
-
-
 try:
     svminitfile = os.path.expandvars('${SVMGDBINITFILE}')
     exec(open(svminitfile).read())

substratevm/mx.substratevm/mx_substratevm.py

@@ -1242,7 +1242,7 @@ def run_js_test(eager: bool = False):
             svm_experimental_options([
                 '-H:+SourceLevelDebug',
                 '-H:+RuntimeDebugInfo',
-                '-H:+LazyDeoptimization' if eager else '-H:-LazyDeoptimization',
+                '-H:-LazyDeoptimization' if eager else '-H:+LazyDeoptimization',
             ]) +
             ['-g', '-O0', '--macro:jsvm-library']
         ))

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

@@ -70,6 +70,7 @@ interface InstalledCodeObserverHandleAccessor {
         default void activate(InstalledCodeObserverHandle handle) {
         }
 
+        @Uninterruptible(reason = "Called during GC or teardown.")
         default void release(InstalledCodeObserverHandle handle) {
         }
 
@@ -78,9 +79,5 @@ default void detachFromCurrentIsolate(InstalledCodeObserverHandle handle) {
 
         default void attachToCurrentIsolate(InstalledCodeObserverHandle handle) {
         }
-
-        @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-        default void releaseOnTearDown(InstalledCodeObserverHandle handle) {
-        }
     }
 }

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

@@ -141,7 +141,7 @@ public static void removeObserversOnTearDown(NonmovableArray<InstalledCodeObserv
             for (int i = 0; i < length; i++) {
                 InstalledCodeObserverHandle handle = NonmovableArrays.getWord(observerHandles, i);
                 if (handle.isNonNull()) {
-                    getAccessor(handle).releaseOnTearDown(handle);
+                    getAccessor(handle).release(handle);
                     NonmovableArrays.setWord(observerHandles, i, Word.nullPointer());
                 }
             }

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

@@ -292,9 +292,9 @@ private void prepareInvalidation(CodeInfo info) {
     }
 
     private void continueInvalidation(CodeInfo info, boolean removeNow) {
-        InstalledCodeObserverSupport.removeObservers(RuntimeCodeInfoAccess.getCodeObserverHandles(info));
         if (removeNow) {
             /* If removeNow, then the CodeInfo is immediately removed from the code cache. */
+            InstalledCodeObserverSupport.removeObservers(RuntimeCodeInfoAccess.getCodeObserverHandles(info));
             removeFromCodeCache(info);
             RuntimeCodeInfoHistory.singleton().logInvalidate(info);
         } else {

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

@@ -168,7 +168,7 @@ public void activate(InstalledCodeObserverHandle installedCodeObserverHandle) {
         }
 
         @Override
-        @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
+        @Uninterruptible(reason = "Called during GC or teardown.")
         public void release(InstalledCodeObserverHandle installedCodeObserverHandle) {
             Handle handle = (Handle) installedCodeObserverHandle;
             GdbJitInterface.JITCodeEntry entry = handle.getRawHandle();
@@ -193,12 +193,6 @@ public void attachToCurrentIsolate(InstalledCodeObserverHandle installedCodeObse
             Handle handle = (Handle) installedCodeObserverHandle;
             NonmovableArrays.trackUnmanagedArray(handle.getDebugInfoData());
         }
-
-        @Override
-        @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-        public void releaseOnTearDown(InstalledCodeObserverHandle installedCodeObserverHandle) {
-            release(installedCodeObserverHandle);
-        }
     }
 
     @Override

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/gdb_helper.py

@@ -45,7 +45,9 @@
 boolean_rexp = re.compile(r"(true|false)")
 array_rexp = re.compile(r'.+\[\d+]\s*=\s*{.*}')
 args_rexp = re.compile(r'.*\(.*\)\s*\((?P<args>.*)\)')
-hex_rexp = re.compile(r"[\da-fA-F]+")
+hex_pattern = r"[\da-fA-F]+"
+hex_rexp = re.compile(hex_pattern)
+value_pattern = r"[a-zA-Z0-9$_<> ]+"
 
 
 def gdb_execute(command: str) -> str:
@@ -118,6 +120,11 @@ def gdb_delete_breakpoints() -> None:
     gdb_execute("delete breakpoints")
 
 
+def gdb_disable_breakpoints() -> None:
+    logger.info("Disabling all breakpoints")
+    gdb_execute("disable breakpoints")
+
+
 def gdb_run() -> None:
     logger.info('Run current program')
     gdb_execute('run')

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/test_runtime_compilation.py

@@ -69,15 +69,14 @@ def test_update_breakpoint(self):
         # new breakpoint address is always added after
         self.assertEqual(breakpoint_info_after.split(breakpoint_info_before.split('0x')[-1])[-1].count('com.oracle.svm.test.debug.helper.RuntimeCompilations::inlineTest'), 1)
 
-        # run until the runtime compilation is invalidated and check if the breakpoint is removed
-        gdb_set_breakpoint('com.oracle.svm.graal.meta.SubstrateInstalledCodeImpl::invalidate')
-        gdb_continue()  # run until invalidation
-        gdb_finish()  # finish invalidation - this should trigger an unregister call to gdb
+        # run until the end and check if breakpoints for the run-time debuginfo are removed
+        gdb_disable_breakpoints()
+        gdb_continue()  # run until the end
         breakpoint_info_after_invalidation = gdb_execute('info breakpoints')
-        # check if additional breakpoint is cleared after invalidate
+        # check if additional breakpoint is removed
         # breakpoint info is still printed as multi-breakpoint, thus we check if exactly one valid breakpoint is remaining
         self.assertEqual(breakpoint_info_after_invalidation.count('com.oracle.svm.test.debug.helper.RuntimeCompilations::inlineTest'), 1)
-        # breakpoint info must change after invalidation
+        # breakpoint info must change
         self.assertNotEqual(breakpoint_info_after, breakpoint_info_after_invalidation)
 
     # this test requires gdb to first load a new objfile at runtime and then remove it as the compilation is invalidated
@@ -93,11 +92,10 @@ def test_load_objfile(self):
         objfiles = gdb.objfiles()
         self.assertTrue(any([o.filename.startswith('<in-memory@') for o in objfiles]))
 
-        # run until the runtime compilation is invalidated and check if the objfile is removed
-        gdb_set_breakpoint('com.oracle.svm.graal.meta.SubstrateInstalledCodeImpl::invalidate')
-        gdb_continue()  # run until invalidation
-        gdb_finish()  # finish invalidation - this should trigger an unregister call to gdb
-        # compilation is invalidated, check if objfile was removed
+        # run until the end and check if run-time debuginfo object file is removed
+        gdb_disable_breakpoints()
+        gdb_continue()  # run until the end
+        # check if objfiles are removed
         objfiles = gdb.objfiles()
         self.assertFalse(any([o.filename.startswith('<in-memory@') for o in objfiles]))
 

substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/test_runtime_deopt.py

@@ -33,6 +33,7 @@
 sys.path.insert(0, os.path.join(os.path.dirname(os.path.realpath(__file__))))
 
 from gdb_helper import *
+import re
 
 
 # requires the gdb patch to be available
@@ -95,13 +96,21 @@ def test_backtrace_with_deopt(self):
             else:
                 self.assertIn('SubstrateOptimizedCallTarget = {...}, __1=java.lang.Object[5] = {...})', backtrace)
                 self.assertIn('SubstrateOptimizedCallTargetInstalledCode::doInvoke', backtrace)
-
-            self.assertNotIn('??', backtrace)
-            self.assertNotIn('Unknown Frame at', backtrace)
         else:
             # must be lazy deopt frame
-            # we can't be sure it is handled properly, but at least it should show up as lazy deopt frame in the backtrace
-            self.assertIn('[LAZY DEOPT FRAME] at', backtrace)
+            self.assertIn('[LAZY DEOPT FRAME]', backtrace)
+
+            if 'SubstrateEnterpriseOptimizedCallTarget' in backtrace:
+                # check if we still see the parameters of the lazily deoptimized method on the backtrace
+                self.assertRegex(backtrace, re.compile(rf'0x{hex_pattern} in \[LAZY DEOPT FRAME\] .* \(callTarget={value_pattern}, args={value_pattern}, __Object2={value_pattern}, __int3={value_pattern}, __int4={value_pattern}, __boolean5={value_pattern}\)'))
+            else:
+                # must be a SubstrateOptimizedCallTarget
+                self.assertIn('SubstrateOptimizedCallTarget', backtrace)
+                self.assertRegex(backtrace, re.compile(rf'0x{hex_pattern} in \[LAZY DEOPT FRAME\] .* \(this={value_pattern}, originalArguments={value_pattern}\)'))
+
+        # the backtrace should contain no unknown frames
+        self.assertNotIn('??', backtrace)
+        self.assertNotIn('Unknown Frame at', backtrace)
 
     # the js deopt test uses the jsvm-library
     # so the debugging symbols come from the js shared library
@@ -117,7 +126,7 @@ def test_opaque_types_with_shared_library(self):
             self.assertNotIn('<unknown type in <in-memory@', backtrace)
         else:
             self.assertIn('com.oracle.truffle.runtime.OptimizedCallTarget::profiledPERoot', backtrace)
-            self.assertIn('(this=<optimized out>, originalArguments=)', backtrace)
+            self.assertIn('(this=<optimized out>, originalArguments=', backtrace)
             self.assertNotIn('this=<unknown type in <in-memory@', backtrace)