[Lldb-commits] [lldb] 5ed8765 - [lldb/Unwind] Use eh_frame plan directly when it doesn't need to be augmented

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-06-26T11:46:47+02:00
New Revision: 5ed8765e2f007e56a34e3aa0ab01f65c5aaa7e27

URL: https://github.com/llvm/llvm-project/commit/5ed8765e2f007e56a34e3aa0ab01f65c5aaa7e27
DIFF: https://github.com/llvm/llvm-project/commit/5ed8765e2f007e56a34e3aa0ab01f65c5aaa7e27.diff

LOG: [lldb/Unwind] Use eh_frame plan directly when it doesn't need to be augmented

Summary:
This fixes a bug in the logic for choosing the unwind plan. Based on the
comment in UnwindAssembly-x86, the intention was that a plan which
describes the function epilogue correctly does not need to be augmented
(and it should be used directly). However, the way this was implemented
(by returning false) meant that the higher level code
(FuncUnwinders::GetEHFrameAugmentedUnwindPlan) interpreted this as a
failure to produce _any_ plan and proceeded with other fallback options.
The fallback usually chosed for "asynchronous" plans was the
"instruction emulation" plan, which tended to fall over on certain
functions with multiple epilogues (that's a separate bug).

This patch simply changes the function to return true, which signals the
caller that the unmodified plan is ready to be used.

The attached test case demonstrates the case where we would previously
fall back to the instruction emulation plan, and unwind incorrectly --
the test asserts that the "augmented" eh_frame plan is used, and that
the unwind is correct.

Reviewers: jasonmolenda, jankratochvil

Subscribers: davide, echristo, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D82378

Added: 
    lldb/test/Shell/Unwind/Inputs/eh-frame-augment-noop.s
    lldb/test/Shell/Unwind/eh-frame-augment-noop.test

Modified: 
    lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
index dc62e957c1a1..fe1275d5b0cf 100644
--- a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
@@ -141,7 +141,7 @@ bool UnwindAssembly_x86::AugmentUnwindPlanFromCallSite(
           // and we don't need to modify it at all.
 
           if (first_row_pc_loc.GetOffset() == -wordsize) {
-            do_augment_unwindplan = false;
+            return true;
           }
         }
       }

diff  --git a/lldb/test/Shell/Unwind/Inputs/eh-frame-augment-noop.s b/lldb/test/Shell/Unwind/Inputs/eh-frame-augment-noop.s
new file mode 100644
index 000000000000..2bcf534f04fc
--- /dev/null
+++ b/lldb/test/Shell/Unwind/Inputs/eh-frame-augment-noop.s
@@ -0,0 +1,22 @@
+# A function whose prologue and epilogue are described perfectly.  eh_frame
+# augmentation machinery should detect that no augmentation is needed and use
+# eh_frame directly.
+
+        .text
+        .globl  foo
+foo:
+        .cfi_startproc
+        pushq   %rax
+        .cfi_def_cfa_offset 16
+        int3
+        pop %rcx
+        .cfi_def_cfa_offset 8
+        retq
+        .cfi_endproc
+
+        .globl asm_main
+asm_main:
+        .cfi_startproc
+        callq foo
+        retq
+        .cfi_endproc

diff  --git a/lldb/test/Shell/Unwind/eh-frame-augment-noop.test b/lldb/test/Shell/Unwind/eh-frame-augment-noop.test
new file mode 100644
index 000000000000..b94c5abcc978
--- /dev/null
+++ b/lldb/test/Shell/Unwind/eh-frame-augment-noop.test
@@ -0,0 +1,22 @@
+# Test handing of dwarf expressions specifying the location of registers, if
+# those expressions refer to the frame's CFA value.
+
+# UNSUPPORTED: system-windows
+# REQUIRES: target-x86_64, native
+
+# RUN: %clang_host %p/Inputs/call-asm.c %p/Inputs/eh-frame-augment-noop.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+process launch
+# CHECK: stop reason
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`foo + 2
+# CHECK: frame #1: {{.*}}`asm_main + 5
+
+target modules show-unwind -n foo
+# CHECK: Asynchronous (not restricted to call-sites) UnwindPlan is 'eh_frame CFI'
+# CHECK: eh_frame augmented UnwindPlan:
+# CHECK:      row[0]:  0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK-NEXT: row[1]:  1: CFA=rsp+16 => rip=[CFA-8]
+# CHECK-NEXT: row[2]:  3: CFA=rsp +8 => rip=[CFA-8]