[lldb] Fix handling of cfi_restore in the unwinder

[jarin]

Summary

Currently, lldb's unwinder ignores cfi_restore opcodes for registers
that are not set in the first row of the unwinding info. This prevents
unwinding of failed assertion in Chrome/v8 (https://github.com/v8/v8).
The attached test is an x64 copy of v8's function that failed to unwind
correctly.

This patch changes handling of cfi_restore to reset the location if
the first unwind table row does not map the restored register.

[jasonmolenda]

I can see why the existing code is written as it is, given the dwarf spec for DW_CFA_restore: "The DW_CFA_restore instruction takes a single operand (encoded with the opcode) that represents a register number. The required action is to change the rule for the indicated register to the rule assigned it by the initial_instructions in the CIE". The mistake in the current code is that any register not mentioned in the CIE state (the unwind rules at Row 0 in this UnwindPlan) is the unmodified value of the caller.

This patch is correct; the register was not mentioned in the CIE so should say there is no unwind rule for this register - it is the unmodified value of the caller function. I probably would have added a comment to this new line in the else block saying that explicitly, but that's just a personal preference.

[jarin]

Added a comment explaining that we are restoring the caller's value of the register.

[Michael137]

Looks like this is failing on the Darwin x86_64 buildbots: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/56510/execution/node/74/log/

Exit Code: 1

Command Output (stderr):
--
clang: warning: argument unused during compilation: '-fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-shell' [-Wunused-command-line-argument]
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s:25:2: error: unknown directive
 .size g_hard_abort, 1
 ^

********************
Failed Tests (1):
  lldb-shell :: Unwind/eh-frame-dwarf-unwind-abort.test

[jarin]

In D153043#4427660, @Michael137 wrote:

Looks like this is failing on the Darwin x86_64 buildbots: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/56510/execution/node/74/log/

Exit Code: 1

Command Output (stderr):
--
clang: warning: argument unused during compilation: '-fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-shell' [-Wunused-command-line-argument]
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s:25:2: error: unknown directive
 .size g_hard_abort, 1
 ^

********************
Failed Tests (1):
  lldb-shell :: Unwind/eh-frame-dwarf-unwind-abort.test

I removed the offending directive. Feel free to revert all this if there are still problems.

[Michael137]

This is now failing with:

clang: warning: argument unused during compilation: '-fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-shell' [-Wunused-command-line-argument]
Undefined symbols for architecture x86_64:
  "abort", referenced from:
      asm_main in eh-frame-dwarf-unwind-abort-edbc93.o
     (maybe you meant: g_hard_abort)
ld: symbol(s) not found for architecture x86_64

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/56648/execution/node/74/log/?consoleFull

lldb/source/Symbol/DWARFCallFrameInfo.cpp

if (!HandleCommonDwarfOpcode(primary_opcode, extended_opcode, data_align,
           // our thread, and these are organized by the register index in that
           // state, so we need to convert our eh_frame register number from the
           // EH frame info, to a register index
 
           if (unwind_plan.IsValidRowIndex(0) &&
               unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num,
                                                             reg_location))
             row->SetRegisterInfo(reg_num, reg_location);
+          else {
+            // If the register was not set in the first row, remove the
+            // register info to keep the unmodified value from the caller.
+            row->RemoveRegisterInfo(reg_num);
+          }
           break;
         }
         }
       } else {
         switch (extended_opcode) {
         case DW_CFA_set_loc: // 0x1 (Row Creation Instruction)
         {
           // DW_CFA_set_loc takes a single argument that represents an address.

lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-abort.s

+        .text
+        .globl  asm_main
+asm_main:
+        .cfi_startproc
+        cmpb $0x0, g_hard_abort(%rip)
+        jne .L
+
+        pushq   %rbp
+        .cfi_def_cfa_offset 16
+        .cfi_offset 6, -16
+        movq    %rsp, %rbp
+        .cfi_def_cfa_register 6
+        callq   abort
+.L:
+        .cfi_def_cfa 7, 8
+        .cfi_restore 6
+        int3
+        ud2
+        .cfi_endproc
+
+  .data
+  .globl  g_hard_abort
+g_hard_abort:
+  .byte   1
+  .size   g_hard_abort, 1
 No newline at end of file

lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test

+# Test restoring of register values.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c %p/Inputs/eh-frame-dwarf-unwind-abort.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason = signal SIGTRAP
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`asm_main + 23
+# CHECK: frame #1: {{.*}}`main + {{.*}}
+
+target modules show-unwind -n asm_main
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]:    0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK: row[1]:   14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[2]:   17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8]
+# CHECK: row[3]:   22: CFA=rsp +8 => rip=[CFA-8]