[RFC][lldb] Use PC from the deepest consecutive inlined frame

[ntesic]

Summary

Please, consider LLDB behaviour in the case where we have inlined functions, explained below.

(lldb) bt
* thread #1, name = 'inline', stop reason = signal SIGSEGV
  * frame #0: 0x00000000004005e4 inline`print_info(node=0x00007ffcd1425cd8) at inline.c:16:32
    frame #1: 0x0000000000400645 inline`main [inlined] func1 at inline.c:28:3
    frame #2: 0x000000000040060f inline`main [inlined] func2 at inline.c:35
    frame #3: 0x000000000040060f inline`main at inline.c:39
    frame #4: 0x00007fab30fdb7b3 libc.so.6`__libc_start_main + 243
    frame #5: 0x000000000040050e inline`_start + 46

Parents of the inlined frames (#2 and #3) have PC which points to the beginning of the inlined sequence (0x40060f)
regardless of where the current PC is in the same inlined sequence (backtrace above).
Because of the inlining, PC register value is the same for frames #1, #2 and #3.
However, if we use LLDB GetPC() interface, we would get the frame PC value as is in the backtrace,
which could be misleading.

GDB just shows PC value for the first inlined frame (output below).

(gdb) bt
#0  0x00000000004005e4 in print_info (node=0x7fff7f41bc18) at double.c:16
#1  0x0000000000400645 in func1 () at double.c:28
#2  func2 () at double.c:35
#3  main () at double.c:39

Is the explained LLDB behaviour (different from GDB) expected?

If not, please, consider the solution proposed by this patch.
When doing stack unwinding of inlined functions, we start from the inlined frame and iterate through parents,
recursively, until we find the non inlined frame. During that process, we create StackFrame instances
for each frame and we are using GetParentOfInlinedScope hook to get needed info.
This patch proposes using PC from the deepest inlined frame, for all consecutive inlined parents (recursively)
and the first non inlined (where the inlined code is located).

After this patch the backtrace from the example above has the following form

* thread #1, name = 'double', stop reason = signal SIGSEGV
  * frame #0: 0x00000000004005e4 double`print_info(node=0x00007fff7f41bc18) at double.c:16:32
    frame #1: 0x0000000000400645 double`main [inlined] func1 at double.c:28:3
    frame #2: 0x0000000000400645 double`main [inlined] func2 at double.c:35:3
    frame #3: 0x0000000000400645 double`main at double.c:39:1
    frame #4: 0x00007f03adf0a7b3 libc.so.6`__libc_start_main + 243
    frame #5: 0x000000000040050e double`_start + 46

Any feedback is welcome, thanks!

[clayborg]

Please, consider LLDB behaviour in the case where we have inlined functions, explained below.

(lldb) bt
 * thread #1, name = 'inline', stop reason = signal SIGSEGV
   * frame #0: 0x00000000004005e4 inline`print_info(node=0x00007ffcd1425cd8) at inline.c:16:32
     frame #1: 0x0000000000400645 inline`main [inlined] func1 at inline.c:28:3
     frame #2: 0x000000000040060f inline`main [inlined] func2 at inline.c:35
     frame #3: 0x000000000040060f inline`main at inline.c:39
     frame #4: 0x00007fab30fdb7b3 libc.so.6`__libc_start_main + 243
     frame #5: 0x000000000040050e inline`_start + 46

Parents of the inlined frames (#2 and #3) have PC which points to the beginning of the inlined sequence (0x40060f)
regardless of where the current PC is in the same inlined sequence (backtrace above).
Because of the inlining, PC register value is the same for frames #1, #2 and #3.
However, if we use LLDB GetPC() interface, we would get the frame PC value as is in the backtrace,
which could be misleading.

This is expected behavior. We backup the PC to match the start address of the inlined function's start address in each case. So the PC is 0x0000000000400645 for frame #1, but the location in the code that falls through to "func1" is actually 0x000000000040060f. So this is actually more accurate from a code perspective. We get this information because we know the address ranges of all inline functions and we always correctly calculate the PC values for the last instruction of the current function ("func2" for frame #2 for example). I am not sure if GDB tracks the information the same way so GDB might not be able to represent this correctly. Many inline functions calls when chained together can end up with the same PC value, so for frames #2 and #3, there were no instructions between the fall through from "main" to "func2" so the PC doesn't actually have to change.

So I would vote to not change anything as I think what LLDB shows is more correct.

lldb/source/Target/StackFrameList.cpp

do {
     Block *unwind_block = unwind_sc.block;
     TargetSP target_sp = m_thread.CalculateTarget();
     if (unwind_block) {
       Address curr_frame_address(
           unwind_frame_sp->GetFrameCodeAddressForSymbolication());
 
       SymbolContext next_frame_sc;
       Address next_frame_address;
+      // Use PC from the top inlined frame for all consecutive inlined parent frames
+      // and the first non inlined (example below).
+      // next_frame_address points to the first instruction in the inlined sequence
+      // regardless where the current PC points to.
+      //
+      // frame #1: 0x0000000000400645 inline`main [inlined] func1 at inline.c:28:3
+      // frame #2: 0x0000000000400645 inline`main [inlined] func2 at inline.c:35
+      // frame #3: 0x0000000000400645 inline`main at inline.c:39
+      Address deepest_inline_pc_addr = unwind_frame_sp.get()->GetFrameCodeAddress();
 
       while (unwind_sc.GetParentOfInlinedScope(
           curr_frame_address, next_frame_sc, next_frame_address)) {
         next_frame_sc.line_entry.ApplyFileMappings(target_sp);
         behaves_like_zeroth_frame = false;
         StackFrameSP frame_sp(new StackFrame(
             m_thread.shared_from_this(), m_frames.size(), idx,
-            unwind_frame_sp->GetRegisterContextSP(), cfa, next_frame_address,
+            unwind_frame_sp->GetRegisterContextSP(), cfa, deepest_inline_pc_addr,
             behaves_like_zeroth_frame, &next_frame_sc));
 
         m_frames.push_back(frame_sp);
         unwind_sc = next_frame_sc;
         curr_frame_address = next_frame_address;
       }
     }
   } while (m_frames.size() - 1 < end_idx);

lldb/test/Shell/Unwind/Inputs/inlined-scope-pc.c

+typedef struct inside_str {
+  int y;
+  int x;
+} nested_str;
+
+typedef struct node {
+  char *x;
+  int *val;
+  nested_str * val2;
+} info;
+
+__attribute__((noinline))
+void crash (info *node) {
+   node->val2->x = 4;
+}
+
+__attribute__((always_inline))
+static void func1() {
+  info node;
+  char c;
+  nested_str n = {10,20};
+  node.val = 0;
+  node.val2 = &n;
+  crash (&node);
+  node.val2 = 0;
+  crash (&node);
+  node.x = &c;
+  crash (&node);
+}
+
+__attribute__((always_inline))
+static void func2() {
+  func1();
+}
+
+int main() {
+func2();
+return 0;
+}

lldb/test/Shell/Unwind/inlined-scope-pc.test

+
+# RUN: %clang_host -g -O0 %p/Inputs/inlined-scope-pc.c -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck  %s
+
+run
+# CHECK: stop reason = signal SIGSEGV
+
+bt
+# CHECK: frame #0: {{.*}}`crash
+# CHECK: frame #1: 0x[[INLINE_PC:[0-9a-f]+]]  {{.*}}`main [inlined] func1 at
+# CHECK: frame #2: 0x[[INLINE_PC]] {{.*}}`main [inlined] func2 at
+# CHECK: frame #3: 0x[[INLINE_PC]] {{.*}}`main at