Skip to content

[GR-68567] Improve GDB backtrace support for lazy deoptimizations. #11972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 25 additions & 32 deletions substratevm/debug/gdbpy/gdb-debughelpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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:
Expand All @@ -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():
Expand All @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion substratevm/mx.substratevm/mx_substratevm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ interface InstalledCodeObserverHandleAccessor {
default void activate(InstalledCodeObserverHandle handle) {
}

@Uninterruptible(reason = "Called during GC or teardown.")
default void release(InstalledCodeObserverHandle handle) {
}

Expand All @@ -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) {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)


Expand Down
Loading