From 156e92b9a24e1b8ce30a838d3fb1c8cb8c7c7ab3 Mon Sep 17 00:00:00 2001 From: Dominik Mascherbauer Date: Wed, 6 Aug 2025 15:58:36 +0200 Subject: [PATCH] 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. --- substratevm/debug/gdbpy/gdb-debughelpers.py | 57 ++++++++----------- substratevm/mx.substratevm/mx_substratevm.py | 2 +- .../svm/core/code/InstalledCodeObserver.java | 5 +- .../code/InstalledCodeObserverSupport.java | 2 +- .../svm/core/code/RuntimeCodeCache.java | 2 +- .../debug/SubstrateDebugInfoInstaller.java | 8 +-- .../svm/test/debug/helper/gdb_helper.py | 9 ++- .../debug/helper/test_runtime_compilation.py | 20 +++---- .../test/debug/helper/test_runtime_deopt.py | 21 +++++-- 9 files changed, 62 insertions(+), 64 deletions(-) diff --git a/substratevm/debug/gdbpy/gdb-debughelpers.py b/substratevm/debug/gdbpy/gdb-debughelpers.py index 8a25adbcbaed..1325626886fa 100644 --- a/substratevm/debug/gdbpy/gdb-debughelpers.py +++ b/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()) diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py index 5057a0d3801d..f018dc3a1258 100644 --- a/substratevm/mx.substratevm/mx_substratevm.py +++ b/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'] )) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserver.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserver.java index f03bc688972d..19b960e6f752 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserver.java +++ b/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) { - } } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserverSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserverSupport.java index cb68d519897b..887de155a77a 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserverSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/InstalledCodeObserverSupport.java @@ -141,7 +141,7 @@ public static void removeObserversOnTearDown(NonmovableArray.*)\)') -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') diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/test_runtime_compilation.py b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/test_runtime_compilation.py index 9c37bf21dc83..d1caf981a566 100644 --- a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/debug/helper/test_runtime_compilation.py +++ b/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(', originalArguments=)', backtrace) + self.assertIn('(this=, originalArguments=', backtrace) self.assertNotIn('this=