From 8000e2734ec5f45c9c56a54b8a1b19375572ec4e Mon Sep 17 00:00:00 2001 From: Alexey Samsonov Date: Mon, 9 Jun 2014 21:53:47 +0000 Subject: [PATCH] Generate better location ranges for some register-described variables. Don't terminate location ranges for register-described variables at the end of machine basic block if this register is never modified in the function body, except for the prologue and epilogue. Prologue location is guessed by FrameSetup flags on MachineInstructions, while epilogue location is deduced from debug locations of instructions in the basic blocks ending with return instructions. This patch is mostly targeted to fix non-trivial debug locations for variables addressed via stack and frame pointers. It is not really a generic fix. We can still produce poor debug info for register-described variables if this register *is* modified somewhere in the function, but in unrelated places. This might be the case for the debug info in optimized binaries (e.g. for local variables in inlined functions). LiveDebugVariables pass in CodeGen attempts to fix this problem by adjusting DBG_VALUE instructions, but this pass is tied to greedy register allocator, which is used in optimized builds only. Proper fix would likely involve generalizing LiveDebugVariables to all register allocators. See more discussion in http://reviews.llvm.org/D3933 review thread. I'm proceeding with this patch to fix immediate severe problems and important cases, e.g. fix completely broken debug info with AddressSanitizer and fix PR19307 (missing debug info for by-value std::string arguments). llvm-svn: 210492 --- .../AsmPrinter/DbgValueHistoryCalculator.cpp | 79 ++++++--- llvm/test/DebugInfo/AArch64/struct_by_value.ll | 4 +- llvm/test/DebugInfo/X86/debug-loc-asan.ll | 186 +++++++++++++++++++++ llvm/test/DebugInfo/X86/debug-loc-offset.ll | 132 +++++++++------ llvm/test/DebugInfo/X86/pr19307.ll | 147 ++++++++++++++++ 5 files changed, 475 insertions(+), 73 deletions(-) create mode 100644 llvm/test/DebugInfo/X86/debug-loc-asan.ll create mode 100644 llvm/test/DebugInfo/X86/pr19307.ll diff --git a/llvm/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp b/llvm/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp index 6103254..a66d08e 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp @@ -15,6 +15,7 @@ #include "llvm/Target/TargetRegisterInfo.h" #include #include +#include #define DEBUG_TYPE "dwarfdebug" @@ -110,45 +111,73 @@ static void clobberRegisterUses(RegDescribedVarsMap &RegVars, unsigned RegNo, RegVars.erase(I); } -// \brief Terminate location ranges for all variables, described by registers -// clobbered by @MI. -static void clobberRegisterUses(RegDescribedVarsMap &RegVars, - const MachineInstr &MI, - const TargetRegisterInfo *TRI, - DbgValueHistoryMap &HistMap) { +// \brief Collect all registers clobbered by @MI and insert them to @Regs. +static void collectClobberedRegisters(const MachineInstr &MI, + const TargetRegisterInfo *TRI, + std::set &Regs) { for (const MachineOperand &MO : MI.operands()) { if (!MO.isReg() || !MO.isDef() || !MO.getReg()) continue; - for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid(); - ++AI) { - unsigned RegNo = *AI; - clobberRegisterUses(RegVars, RegNo, HistMap, MI); - } + for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid(); ++AI) + Regs.insert(*AI); } } -// \brief Terminate the location range for all register-described variables -// by inserting @ClobberingInstr to their history. -static void clobberAllRegistersUses(RegDescribedVarsMap &RegVars, - DbgValueHistoryMap &HistMap, - const MachineInstr &ClobberingInstr) { - for (const auto &I : RegVars) - for (const auto &Var : I.second) - HistMap.endInstrRange(Var, ClobberingInstr); - RegVars.clear(); +// \brief Returns the first instruction in @MBB which corresponds to +// the function epilogue, or nullptr if @MBB doesn't contain an epilogue. +static const MachineInstr *getFirstEpilogueInst(const MachineBasicBlock &MBB) { + auto LastMI = MBB.getLastNonDebugInstr(); + if (LastMI == MBB.end() || !LastMI->isReturn()) + return nullptr; + // Assume that epilogue starts with instruction having the same debug location + // as the return instruction. + DebugLoc LastLoc = LastMI->getDebugLoc(); + auto Res = LastMI; + for (MachineBasicBlock::const_reverse_iterator I(std::next(LastMI)); I != MBB.rend(); + ++I) { + if (I->getDebugLoc() != LastLoc) + return Res; + Res = std::prev(I.base()); + } + // If all instructions have the same debug location, assume whole MBB is + // an epilogue. + return MBB.begin(); +} + +// \brief Collect registers that are modified in the function body (their +// contents is changed only in the prologue and epilogue). +static void collectChangingRegs(const MachineFunction *MF, + const TargetRegisterInfo *TRI, + std::set &Regs) { + for (const auto &MBB : *MF) { + auto FirstEpilogueInst = getFirstEpilogueInst(MBB); + bool IsInEpilogue = false; + for (const auto &MI : MBB) { + IsInEpilogue |= &MI == FirstEpilogueInst; + if (!MI.getFlag(MachineInstr::FrameSetup) && !IsInEpilogue) + collectClobberedRegisters(MI, TRI, Regs); + } + } } void calculateDbgValueHistory(const MachineFunction *MF, const TargetRegisterInfo *TRI, DbgValueHistoryMap &Result) { - RegDescribedVarsMap RegVars; + std::set ChangingRegs; + collectChangingRegs(MF, TRI, ChangingRegs); + RegDescribedVarsMap RegVars; for (const auto &MBB : *MF) { for (const auto &MI : MBB) { if (!MI.isDebugValue()) { // Not a DBG_VALUE instruction. It may clobber registers which describe // some variables. - clobberRegisterUses(RegVars, MI, TRI, Result); + std::set MIClobberedRegs; + collectClobberedRegisters(MI, TRI, MIClobberedRegs); + for (unsigned RegNo : MIClobberedRegs) { + if (ChangingRegs.count(RegNo)) + clobberRegisterUses(RegVars, RegNo, Result, MI); + } continue; } @@ -167,8 +196,10 @@ void calculateDbgValueHistory(const MachineFunction *MF, // Make sure locations for register-described variables are valid only // until the end of the basic block (unless it's the last basic block, in // which case let their liveness run off to the end of the function). - if (!MBB.empty() && &MBB != &MF->back()) - clobberAllRegistersUses(RegVars, Result, MBB.back()); + if (!MBB.empty() && &MBB != &MF->back()) { + for (unsigned RegNo : ChangingRegs) + clobberRegisterUses(RegVars, RegNo, Result, MBB.back()); + } } } diff --git a/llvm/test/DebugInfo/AArch64/struct_by_value.ll b/llvm/test/DebugInfo/AArch64/struct_by_value.ll index 0023c3d..0e336f7 100644 --- a/llvm/test/DebugInfo/AArch64/struct_by_value.ll +++ b/llvm/test/DebugInfo/AArch64/struct_by_value.ll @@ -1,7 +1,9 @@ ; A by-value struct is a register-indirect value (breg). ; RUN: llc %s -filetype=asm -o - | FileCheck %s -; CHECK: DW_OP_breg0 +; CHECK: DW_AT_location +; CHECK-NEXT: .byte 112 +; 112 = 0x70 = DW_OP_breg0 ; rdar://problem/13658587 ; diff --git a/llvm/test/DebugInfo/X86/debug-loc-asan.ll b/llvm/test/DebugInfo/X86/debug-loc-asan.ll new file mode 100644 index 0000000..5f201ad --- /dev/null +++ b/llvm/test/DebugInfo/X86/debug-loc-asan.ll @@ -0,0 +1,186 @@ +; RUN: llc -O0 -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s + +; Verify that we have correct debug info for local variables in code +; instrumented with AddressSanitizer. + +; Generated from the source file test.cc: +; int bar(int y) { +; return y + 2; +; } +; with "clang++ -S -emit-llvm -fsanitize=address -O0 -g test.cc" + +; First, argument variable "y" resides in %rdi: +; CHECK: DEBUG_VALUE: bar:y <- RDI + +; Then its address is stored in a location on a stack: +; CHECK: movq %rdi, [[OFFSET:[0-9]+]](%rsp) +; CHECK-NEXT: [[START_LABEL:.Ltmp[0-9]+]] +; CHECK-NEXT: DEBUG_VALUE: bar:y <- [RSP+[[OFFSET]]] +; This location should be valid until the end of the function. + +; CHECK: .Ldebug_loc{{[0-9]+}}: +; We expect two location ranges for the variable. + +; First, it is stored in %rdi: +; CHECK: .Lset{{[0-9]+}} = .Lfunc_begin0-.Lfunc_begin0 +; CHECK-NEXT: .quad .Lset{{[0-9]+}} +; CHECK-NEXT: .Lset{{[0-9]+}} = [[START_LABEL]]-.Lfunc_begin0 +; CHECK-NEXT: .quad .Lset{{[0-9]+}} +; CHECK: DW_OP_reg5 + +; Then it's addressed via %rsp: +; CHECK: .Lset{{[0-9]+}} = [[START_LABEL]]-.Lfunc_begin0 +; CHECK-NEXT: .quad .Lset{{[0-9]+}} +; CHECK-NEXT: .Lset{{[0-9]+}} = .Lfunc_end0-.Lfunc_begin0 +; CHECK-NEXT: .quad .Lset{{[0-9]+}} +; CHECK: DW_OP_breg7 +; CHECK-NEXT: [[OFFSET]] +; CHECK: DW_OP_deref + +; ModuleID = 'test.cc' +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +@llvm.global_ctors = appending global [1 x { i32, void ()* }] [{ i32, void ()* } { i32 1, void ()* @asan.module_ctor }] +@__asan_option_detect_stack_use_after_return = external global i32 +@__asan_gen_ = private unnamed_addr constant [16 x i8] c"1 32 4 6 y.addr\00", align 1 + +; Function Attrs: nounwind sanitize_address uwtable +define i32 @_Z3bari(i32 %y) #0 { +entry: + %MyAlloca = alloca [64 x i8], align 32 + %0 = ptrtoint [64 x i8]* %MyAlloca to i64 + %1 = load i32* @__asan_option_detect_stack_use_after_return + %2 = icmp ne i32 %1, 0 + br i1 %2, label %3, label %5 + +;