From 0d9056d7660a6fecfdd704395e7f629e4f65c11c Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Sun, 1 Jun 2014 03:38:13 +0000 Subject: [PATCH] DebugInfo: Assert that DbgVariables have associated DIEs This was previously committed in r209680 and reverted in r209683 after it caused sanitizer builds to crash. The issue seems to be that the DebugLoc associated with dbg.value IR intrinsics isn't necessarily accurate. Instead, we duplicate the DIVariables and add an InlinedAt field to them to record their location. We were using this InlinedAt field to compute the LexicalScope for the variable, but not using it in the abstract DbgVariable construction and mapping. This resulted in a formal parameter to the current concrete function, correctly having no InlinedAt information, but incorrectly having a DebugLoc that described an inlined location within the function... thus an abstract DbgVariable was created for the variable, but its DIE was never constructed (since the LexicalScope had no such variable). This DbgVariable was silently ignored (by testing for a non-null DIE on the abstract DbgVariable). So, fix this by using the right scoping information when constructing abstract DbgVariables. In the long run, I suspect we want to undo the work that added this second kind of location tracking and fix the places where the DebugLoc propagation on the dbg.value intrinsic fails. This will shrink debug info (by not duplicating DIVariables), make it more efficient (by not having to construct new DIVariable metadata nodes to try to map back to a single variable), and benefit all instructions. But perhaps there are insurmountable issues with DebugLoc quality that I'm unaware of... I just don't know how we can't /just keep the DebugLoc from the dbg.declare to the dbg.values and never get this wrong/. Some history context: http://llvm.org/viewvc/llvm-project?view=revision&revision=135629 http://llvm.org/viewvc/llvm-project?view=revision&revision=137253 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@209984 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 2 +- lib/CodeGen/AsmPrinter/DwarfUnit.cpp | 3 +- test/DebugInfo/incorrect-variable-debugloc.ll | 391 ++++++++++++++++++ 3 files changed, 393 insertions(+), 3 deletions(-) create mode 100644 test/DebugInfo/incorrect-variable-debugloc.ll diff --git a/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/lib/CodeGen/AsmPrinter/DwarfDebug.cpp index b2ae9ad36da..73496b0c382 100644 --- a/lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ b/lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -1175,7 +1175,7 @@ DwarfDebug::collectVariableInfo(SmallPtrSet &Processed) { Processed.insert(DV); const MachineInstr *MInsn = Ranges.front().first; assert(MInsn->isDebugValue() && "History must begin with debug value"); - DbgVariable *AbsVar = findAbstractVariable(DV, MInsn->getDebugLoc()); + DbgVariable *AbsVar = findAbstractVariable(DV, Scope->getScopeNode()); DbgVariable *RegVar = new DbgVariable(MInsn, AbsVar, this); if (!addCurrentFnArgument(RegVar, Scope)) addScopeVariable(Scope, RegVar); diff --git a/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/lib/CodeGen/AsmPrinter/DwarfUnit.cpp index 29ebad40256..fddc7fa137f 100644 --- a/lib/CodeGen/AsmPrinter/DwarfUnit.cpp +++ b/lib/CodeGen/AsmPrinter/DwarfUnit.cpp @@ -1781,8 +1781,7 @@ std::unique_ptr DwarfUnit::constructVariableDIEImpl(const DbgVariable &DV, // Define variable debug information entry. auto VariableDie = make_unique(DV.getTag()); - DbgVariable *AbsVar = DV.getAbstractVariable(); - if (AbsVar && AbsVar->getDIE()) + if (DbgVariable *AbsVar = DV.getAbstractVariable()) addDIEEntry(*VariableDie, dwarf::DW_AT_abstract_origin, *AbsVar->getDIE()); else { if (!Name.empty()) diff --git a/test/DebugInfo/incorrect-variable-debugloc.ll b/test/DebugInfo/incorrect-variable-debugloc.ll new file mode 100644 index 00000000000..284704c54a9 --- /dev/null +++ b/test/DebugInfo/incorrect-variable-debugloc.ll @@ -0,0 +1,391 @@ +; REQUIRES: object-emission + +; RUN: %llc_dwarf -O2 -filetype=obj < %s | llvm-dwarfdump -debug-dump=info - | FileCheck %s + +; This is a test case that's as reduced as I can get it, though I haven't fully +; understood the mechanisms by which this bug occurs, so perhaps there's further +; simplification to be had (it's certainly a bit non-obvious what's going on). I +; hesitate to hand-craft or otherwise simplify the IR compared to what Clang +; generates as this is a particular tickling of optimizations and debug location +; propagation I want a realistic example of. + +; Generated with clang-tot -cc1 -g -O2 -w -std=c++11 -fsanitize=address,use-after-return -fcxx-exceptions -fexceptions -x c++ incorrect-variable-debug-loc.cpp -emit-llvm + +; struct A { +; int m_fn1(); +; }; +; +; struct B { +; void __attribute__((always_inline)) m_fn2() { i = 0; } +; int i; +; }; +; +; struct C { +; void m_fn3(); +; int j; +; B b; +; }; +; +; int fn1() { +; C A; +; A.b.m_fn2(); +; A.m_fn3(); +; } +; void C::m_fn3() { +; A().m_fn1(); +; b.m_fn2(); +; } + +; CHECK: DW_TAG_structure_type +; CHECK-NEXT: DW_AT_name {{.*}} "C" +; CHECK: [[FN3_DECL:.*]]: DW_TAG_subprogram +; CHECK-NOT: DW_TAG +; CHECK: DW_AT_name {{.*}} "m_fn3" + +; CHECK: DW_AT_specification {{.*}} {[[FN3_DECL]]} +; CHECK-NOT: DW_TAG +; CHECK: DW_TAG_formal_parameter +; CHECK-NOT: DW_TAG +; CHECK: DW_AT_name {{.*}} "this" + +%struct.C = type { i32, %struct.B } +%struct.B = type { i32 } +%struct.A = type { i8 } + +@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 [11 x i8] c"1 32 8 1 A\00", align 1 +@__asan_gen_1 = private unnamed_addr constant [13 x i8] c"1 32 1 3 tmp\00", align 1 + +; Function Attrs: noreturn sanitize_address +define i32 @_Z3fn1v() #0 { +entry: + %MyAlloca = alloca [64 x i8], align 32, !dbg !39 + %0 = ptrtoint [64 x i8]* %MyAlloca to i64, !dbg !39 + %1 = load i32* @__asan_option_detect_stack_use_after_return, !dbg !39 + %2 = icmp ne i32 %1, 0, !dbg !39 + br i1 %2, label %3, label %5 + +;