Skip to content

Conversation

@dbjwhs-perforce
Copy link
Contributor

@dbjwhs-perforce dbjwhs-perforce commented Nov 18, 2025

DLPX-95965 Replace fragile string parsing in _framestr() with drgn StackFrame API

Problem

@mmaybee called out in #352 that the _framestr() function was using string parsing on str(frame) output to extract frame information. This approach was brittle and could break if drgn's string representation format changed.

Solution

This PR replaces the string parsing with direct use of drgn StackFrame API properties:

  • frame.pc - Program counter (address)
  • frame.function_name - Function name (or None if unavailable)
  • frame.symbol() - Symbol object for fallback name lookup
  • frame.source() - Returns (filename, line, column) tuple
  • frame.is_inline - Boolean for inline frame detection

Changes

  1. Updated _framestr() signature: Added required frame_index: int parameter.

  2. Rewrote frame formatting logic: Uses API properties directly with error handling via try/except for LookupError

  3. Updated call sites:

    • Changed for frame in stack_trace: to for frame_index, frame in enumerate(stack_trace):
    • Updated calls from _framestr(frame, False) to _framestr(frame, False, frame_index)
    • Updated calls from _framestr(frame, True) to _framestr(frame, True, self.frame_id)

Testing

Tested with crash dump from kernel 6.14.0-27-dx2025082915-8ba235c2d-generic:

sdb> trace 0xffff8f1a285e5400
TASK: 0xffff8f1a285e5400 INTERRUPTIBLE PID: 12546
#0  0xffffffff92718fd0 in context_switch() at core.c:5378:2 (inlined)
#1  0xffffffff92718fd0 in __schedule() at core.c:6765:8
#2  0xffffffff92719389 in __schedule_loop() at core.c:6842:3 (inlined)
#3  0xffffffff92719389 in schedule() at core.c:6857:2
#4  0xffffffffc06c8a40
#5  0xffffffff917ab35b in kthread() at kthread.c:464:9
#6  0xffffffff916d9dd4 in ret_from_fork() at process.c:153:3

All features working correctly:

  • Frame indices from enumerate
  • Addresses from frame.pc
  • Function names from frame.function_name
  • Source locations from frame.source() showing file:line:col
  • Inline markers from frame.is_inline

Benefits

  • More robust: No longer dependent on string format
  • More maintainable: Uses documented API instead of parsing heuristics
  • Better error handling: Explicit try/except for missing data
  • Clearer code: Intent is obvious from property names

Ensuring no regressions

Verified Command Compatibility

All related stack inspection commands continue to work correctly with the updated _framestr() implementation:

trace command - Displays full stack trace with frame indices, addresses, functions, and source locations:

sdb> trace 0xffff8f1a285e5400
TASK: 0xffff8f1a285e5400 INTERRUPTIBLE PID: 12546
#0  0xffffffff92718fd0 in context_switch() at core.c:5378:2 (inlined)
#1  0xffffffff92718fd0 in __schedule() at core.c:6765:8
#2  0xffffffff92719389 in __schedule_loop() at core.c:6842:3 (inlined)
#3  0xffffffff92719389 in schedule() at core.c:6857:2
...

frame command - Shows detailed frame information with function arguments (uses _framestr() with args=True):

sdb> frame 0
#0  0xffffffff92718fd0 in context_switch (rq=0xffff8f566bbb6e80, prev=0xffff8f1a285e5400, 
    next=0xffff8f1840f32a00, rf=0xffffa36c7599bd20) at core.c:5378:2 (inlined)

locals command - Displays local variables in the current frame:

sdb> locals
rf = (struct rq_flags *)0xffffa36c7599bd20
next = (struct task_struct *)0xffff8f1840f32a00
prev = (struct task_struct *)0xffff8f1a285e5400
rq = (struct rq *)0xffff8f566bbb6e80

registers command - Shows CPU register values at the current frame:

sdb> registers
rbx = 18446620200070246016
rbp = 18446642284957646200
rsp = 18446642284957646096
r12 = 18446619941242033152
r13 = 18446619933064505856
r14 = 0
r15 = 18446619941242036240
rip = 18446744071871500240

These commands demonstrate that the refactored _framestr() function maintains full backward compatibility while providing more robust frame formatting through the drgn StackFrame API.

@dbjwhs-perforce dbjwhs-perforce force-pushed the dlpx/pr/dbjwhs-perforce/581e6b6d-3271-4387-a745-9a307ceb8902 branch from 15e262f to 4625f1a Compare November 18, 2025 02:11
@dbjwhs-perforce dbjwhs-perforce force-pushed the dlpx/pr/dbjwhs-perforce/581e6b6d-3271-4387-a745-9a307ceb8902 branch from 4625f1a to 6306f9f Compare November 18, 2025 18:52
@dbjwhs-perforce dbjwhs-perforce marked this pull request as ready for review November 18, 2025 18:59
Copy link

@lyriclake lyriclake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on!

@dbjwhs-perforce dbjwhs-perforce force-pushed the dlpx/pr/dbjwhs-perforce/581e6b6d-3271-4387-a745-9a307ceb8902 branch from 6306f9f to c0a6d03 Compare November 20, 2025 02:26
@dbjwhs-perforce
Copy link
Contributor Author

@mmaybee if you look back at this heads up I removed the <no address found> handling because @lyriclake pointed out that frame.pc is a Final[int] attribute that cannot fail, and we already access it without protection at line 268. So all that got removed.

@dbjwhs-perforce dbjwhs-perforce merged commit 419ba54 into develop Nov 20, 2025
6 of 27 checks passed
@dbjwhs-perforce dbjwhs-perforce deleted the dlpx/pr/dbjwhs-perforce/581e6b6d-3271-4387-a745-9a307ceb8902 branch November 20, 2025 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants