[UnwindAssembly/x86] Add support for "lea imm(%ebp), %esp" pattern

[labath]

Summary

The instruction pattern:
and $-16, %esp
sub $imm, %esp
...
lea imm(%ebp), %esp

appears when the compiler is realigning the stack (for example in
main(), or almost everywhere with -mstackrealign switch). The "and"
instruction is very difficult to model, but that's not necessary, as
these frames are always %ebp-based (the compiler also needs a way to
restore the original %esp). Therefore the plans we were generating for
these function were almost correct already. The only place we were doing
it wrong were the last instructions of the epilogue (usually just
"ret"), where we had to revert to %esp-based unwinding, as the %ebp had
been popped already.

This was wrong because our "distance of esp from cfa" counter had picked
up the "sub" instruction (and incremented the counter) but it had not
seen that the register was reset by the "lea" instruction.

This patch fixes that shortcoming, and adds a test for handling
functions like this.

I have not been able to tickle the compiler into producing a 64-bit
function with this pattern, but I don't see a reason why it couldn't
produce it, if it chose to, so I add a x86_64 test as well.

[tberghammer]

Looks good with one possible question

source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp

875–876 ↗

(On Diff #104411)

Shouldn't you change the unwind information for the CFA here? For me saying CFA=rbp seems like an incorrect thing to do, but not sure what would be the correct value (Undefined? IsSame?). The impact is if an other register (or a local variable) have a location specified as CFA+off then after this instruction it will point to bogus location.

[labath]

source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp

875–876 ↗

(On Diff #104411)

I think there has been some misunderstanding, as the your comment makes no sense to me. :)

This code only fires if CFA=rbp+offset, and that remains valid even after this instruction -- lea does not change the value of the rbp register, so any register rule that was valid before this instruction will remain valid after it. This only begins to make a difference after we process the pop %rbp instruction -- then we will update the CFA rule to read CFA=rsp+current_sp_bytes_offset_from_cfa.

[tberghammer]

source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp

875–876 ↗

(On Diff #104411)

You are right, please ignore my comment. I somehow assumed the lea instruction will change the value of rbp as well not just rsp.

[jasonmolenda]

Yeah, looks good. You'll only see this on i386 because the SysV ABI require 4-byte stack alignment. On x86_64 it's 16-byte so the compiler doesn't need to emit the AND instruction to realign it. On darwin we required 16 byte alignment on i386 too so we never saw this problem there.

lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h

private:
   bool push_0_pattern_p();
   bool push_imm_pattern_p();
   bool push_extended_pattern_p();
   bool push_misc_reg_p();
   bool mov_rsp_rbp_pattern_p();
   bool sub_rsp_pattern_p(int &amount);
   bool add_rsp_pattern_p(int &amount);
   bool lea_rsp_pattern_p(int &amount);
+  bool lea_rbp_rsp_pattern_p(int &amount);
   bool push_reg_p(int &regno);
   bool pop_reg_p(int &regno);
   bool pop_rbp_pattern_p();
   bool pop_misc_reg_p();
   bool leave_pattern_p();
   bool call_next_insn_pattern_p();
   bool mov_reg_to_local_stack_frame_p(int &regno, int &rbp_offset);
   bool ret_pattern_p();

lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp

bool x86AssemblyInspectionEngine::lea_rsp_pattern_p(int &amount) {
   if (*(p + 1) == 0xa4 && (*(p + 2) & 0x3f) == 0x24) {
     amount = (int32_t)extract_4(p + 3);
     return true;
   }
 
   return false;
 }
 
+// lea -0x28(%ebp), %esp
+// (32-bit and 64-bit variants, 8-bit and 32-bit displacement)
+bool x86AssemblyInspectionEngine::lea_rbp_rsp_pattern_p(int &amount) {
+  uint8_t *p = m_cur_insn;
+  if (m_wordsize == 8 && *p == 0x48)
+    p++;
+
+  // Check opcode
+  if (*p != 0x8d)
+    return false;
+  ++p;
+
+  // 8 bit displacement
+  if (*p == 0x65) {
+    amount = (int8_t)p[1];
+    return true;
+  }
+
+  // 32 bit displacement
+  if (*p == 0xa5) {
+    amount = (int32_t)extract_4(p + 1);
+    return true;
+  }
+
+  return false;
+}
+
 // popq %rbx
 // popl %ebx
 bool x86AssemblyInspectionEngine::pop_reg_p(int &regno) {
   uint8_t *p = m_cur_insn;
   int regno_prefix_bit = 0;
   // If we have a rex prefix byte, check to see if a B bit is set
   if (m_wordsize == 8 && *p == 0x41) {
     regno_prefix_bit = 1 << 3;
else if (lea_rsp_pattern_p(stack_offset)) {
       if (row->GetCFAValue().GetRegisterNumber() == m_lldb_sp_regnum) {
         row->GetCFAValue().SetOffset(current_sp_bytes_offset_from_cfa);
         row_updated = true;
       }
       if (stack_offset > 0)
         in_epilogue = true;
     }
 
+    else if (lea_rbp_rsp_pattern_p(stack_offset) &&
+             row->GetCFAValue().GetRegisterNumber() == m_lldb_fp_regnum) {
+      current_sp_bytes_offset_from_cfa =
+          row->GetCFAValue().GetOffset() - stack_offset;
+    }
+
     else if (ret_pattern_p() && prologue_completed_row.get()) {
       // Reinstate the saved prologue setup for any instructions
       // that come after the ret instruction
 
       UnwindPlan::Row *newrow = new UnwindPlan::Row;
       *newrow = *prologue_completed_row.get();
       row.reset(newrow);
       current_sp_bytes_offset_from_cfa =

lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp

 
 #include <vector>
 
 #include "Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Core/AddressRange.h"
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Utility/StreamString.h"
 
 #include "llvm/Support/TargetSelect.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 class Testx86AssemblyInspectionEngine : public testing::Test {
 public:
for (const auto &name : i386_reg_names) {
     ri.lldb_regnum = i++;
     lldb_regnums.push_back(ri);
   }
 
   engine->Initialize(lldb_regnums);
   return engine;
 }
 
+namespace lldb_private {
+static std::ostream &operator<<(std::ostream &OS,
+                                const UnwindPlan::Row::CFAValue &CFA) {
+  StreamString S;
+  CFA.Dump(S, nullptr, nullptr);
+  return OS << S.GetData();
+}
+} // namespace lldb_private
+
 TEST_F(Testx86AssemblyInspectionEngine, TestSimple64bitFrameFunction) {
   std::unique_ptr<x86AssemblyInspectionEngine> engine = Getx86_64Inspector();
 
   // 'int main() { }' compiled for x86_64-apple-macosx with clang
   uint8_t data[] = {
       0x55,             // offset 0 -- pushq %rbp
       0x48, 0x89, 0xe5, // offset 1 -- movq %rsp, %rbp
       0x31, 0xc0,       // offset 4 -- xorl %eax, %eax
TEST_F(Testx86AssemblyInspectionEngine, Test32BitOnlyInstruction) {
   row_sp = unwind_plan.GetRowForFunctionOffset(2);
   EXPECT_EQ(0ull, row_sp->GetOffset());
   EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rsp);
   EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
   EXPECT_EQ(8, row_sp->GetCFAValue().GetOffset());
 
   EXPECT_FALSE(row_sp->GetRegisterInfo(k_rbp, regloc));
 }
+
+TEST_F(Testx86AssemblyInspectionEngine, TestStackRealign8BitDisp_i386) {
+  std::unique_ptr<x86AssemblyInspectionEngine> engine = Geti386Inspector();
+
+  uint8_t data[] = {
+      0x55,             // pushl %ebp
+      0x89, 0xe5,       // movl %esp, %ebp
+      0x53,             // pushl %ebx
+      0x83, 0xe4, 0xf0, // andl $-16, %esp
+      0x83, 0xec, 0x10, // subl $16, %esp
+      0x8d, 0x65, 0xfc, // leal -4(%ebp), %esp
+      0x5b,             // popl %ebx
+      0x5d,             // popl %ebp
+      0xc3,             // retl
+  };
+
+  AddressRange sample_range(0x1000, sizeof(data));
+  UnwindPlan plan(eRegisterKindLLDB);
+  ASSERT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(data, sizeof(data),
+                                                           sample_range, plan));
+
+  UnwindPlan::Row::CFAValue esp_plus_4, esp_plus_8, ebp_plus_8;
+  esp_plus_4.SetIsRegisterPlusOffset(k_esp, 4);
+  esp_plus_8.SetIsRegisterPlusOffset(k_esp, 8);
+  ebp_plus_8.SetIsRegisterPlusOffset(k_ebp, 8);
+
+  EXPECT_EQ(esp_plus_4, plan.GetRowForFunctionOffset(0)->GetCFAValue());
+  EXPECT_EQ(esp_plus_8, plan.GetRowForFunctionOffset(1)->GetCFAValue());
+  for (size_t i = 3; i < sizeof(data) - 2; ++i)
+    EXPECT_EQ(ebp_plus_8, plan.GetRowForFunctionOffset(i)->GetCFAValue())
+        << "i: " << i;
+  EXPECT_EQ(esp_plus_4,
+            plan.GetRowForFunctionOffset(sizeof(data) - 1)->GetCFAValue());
+}
+
+TEST_F(Testx86AssemblyInspectionEngine, TestStackRealign32BitDisp_x86_64) {
+  std::unique_ptr<x86AssemblyInspectionEngine> engine = Getx86_64Inspector();
+
+  uint8_t data[] = {
+      0x55,                                     // pushq %rbp
+      0x48, 0x89, 0xe5,                         // movq %rsp, %rbp
+      0x53,                                     // pushl %rbx
+      0x48, 0x83, 0xe4, 0xf0,                   // andq $-16, %rsp
+      0x48, 0x81, 0xec, 0x00, 0x01, 0x00, 0x00, // subq $256, %rsp
+      0x48, 0x8d, 0x65, 0xf8,                   // leaq -8(%rbp), %rsp
+      0x5b,                                     // popq %rbx
+      0x5d,                                     // popq %rbp
+      0xc3,                                     // retq
+  };
+
+  AddressRange sample_range(0x1000, sizeof(data));
+  UnwindPlan plan(eRegisterKindLLDB);
+  ASSERT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(data, sizeof(data),
+                                                           sample_range, plan));
+
+  UnwindPlan::Row::CFAValue rsp_plus_8, rsp_plus_16, rbp_plus_16;
+  rsp_plus_8.SetIsRegisterPlusOffset(k_rsp, 8);
+  rsp_plus_16.SetIsRegisterPlusOffset(k_rsp, 16);
+  rbp_plus_16.SetIsRegisterPlusOffset(k_rbp, 16);
+
+  EXPECT_EQ(rsp_plus_8, plan.GetRowForFunctionOffset(0)->GetCFAValue());
+  EXPECT_EQ(rsp_plus_16, plan.GetRowForFunctionOffset(1)->GetCFAValue());
+  for (size_t i = 4; i < sizeof(data) - 2; ++i)
+    EXPECT_EQ(rbp_plus_16, plan.GetRowForFunctionOffset(i)->GetCFAValue())
+        << "i: " << i;
+  EXPECT_EQ(rsp_plus_8,
+            plan.GetRowForFunctionOffset(sizeof(data) - 1)->GetCFAValue());
+}