From a2b780b731969f63e2e9e9ea76e43cf86d47a5d3 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Fri, 10 May 2019 10:03:41 +0000 Subject: [PATCH] [DebugInfo] Use zero linenos for debug intrinsics when promoting dbg.declare In certain circumstances, optimizations pick line numbers from debug intrinsic instructions as the new location for altered instructions. This is problematic because the line number of a debugging intrinsic is meaningless (it doesn't produce any machine instruction), only the scope information is valid. The result can be the line number of a variable declaration "leaking" into real code from debugging intrinsics, making the line table un-necessarily jumpy, and potentially different with / without variable locations. Fix this by using zero line numbers when promoting dbg.declare intrinsics into dbg.values: this is safe for debug intrinsics as their line numbers are meaningless, and reduces the scope for damage / misleading stepping when optimizations pick locations from the wrong place. Differential Revision: https://reviews.llvm.org/D59272 llvm-svn: 360415 --- llvm/lib/Transforms/Utils/Local.cpp | 35 ++++-- .../Generic/dbg-value-lower-linenos.ll | 113 ++++++++++++++++++ llvm/test/DebugInfo/X86/formal_parameter.ll | 3 + 3 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 llvm/test/DebugInfo/Generic/dbg-value-lower-linenos.ll diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index bf57fcdbdeeb..2a4e9054273b 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -1288,6 +1288,19 @@ static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) { return false; } +/// Produce a DebugLoc to use for each dbg.declare/inst pair that are promoted +/// to a dbg.value. Because no machine insts can come from debug intrinsics, +/// only the scope and inlinedAt is significant. Zero line numbers are used in +/// case this DebugLoc leaks into any adjacent instructions. +static DebugLoc getDebugValueLoc(DbgVariableIntrinsic *DII, Instruction *Src) { + // Original dbg.declare must have a location. + DebugLoc DeclareLoc = DII->getDebugLoc(); + MDNode *Scope = DeclareLoc.getScope(); + DILocation *InlinedAt = DeclareLoc.getInlinedAt(); + // Produce an unknown location with the correct scope / inlinedAt fields. + return DebugLoc::get(0, 0, Scope, InlinedAt); +} + /// Inserts a llvm.dbg.value intrinsic before a store to an alloca'd value /// that has an associated llvm.dbg.declare or llvm.dbg.addr intrinsic. void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII, @@ -1298,6 +1311,8 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII, auto *DIExpr = DII->getExpression(); Value *DV = SI->getValueOperand(); + DebugLoc NewLoc = getDebugValueLoc(DII, SI); + if (!valueCoversEntireFragment(DV->getType(), DII)) { // FIXME: If storing to a part of the variable described by the dbg.declare, // then we want to insert a dbg.value for the corresponding fragment. @@ -1308,14 +1323,12 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII, // know nothing about the variable's content. DV = UndefValue::get(DV->getType()); if (!LdStHasDebugValue(DIVar, DIExpr, SI)) - Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, DII->getDebugLoc(), - SI); + Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI); return; } if (!LdStHasDebugValue(DIVar, DIExpr, SI)) - Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, DII->getDebugLoc(), - SI); + Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, SI); } /// Inserts a llvm.dbg.value intrinsic before a load of an alloca'd value @@ -1338,12 +1351,14 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII, return; } + DebugLoc NewLoc = getDebugValueLoc(DII, nullptr); + // We are now tracking the loaded value instead of the address. In the // future if multi-location support is added to the IR, it might be // preferable to keep tracking both the loaded value and the original // address in case the alloca can not be elided. Instruction *DbgValue = Builder.insertDbgValueIntrinsic( - LI, DIVar, DIExpr, DII->getDebugLoc(), (Instruction *)nullptr); + LI, DIVar, DIExpr, NewLoc, (Instruction *)nullptr); DbgValue->insertAfter(LI); } @@ -1370,12 +1385,13 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII, BasicBlock *BB = APN->getParent(); auto InsertionPt = BB->getFirstInsertionPt(); + DebugLoc NewLoc = getDebugValueLoc(DII, nullptr); + // The block may be a catchswitch block, which does not have a valid // insertion point. // FIXME: Insert dbg.value markers in the successors when appropriate. if (InsertionPt != BB->end()) - Builder.insertDbgValueIntrinsic(APN, DIVar, DIExpr, DII->getDebugLoc(), - &*InsertionPt); + Builder.insertDbgValueIntrinsic(APN, DIVar, DIExpr, NewLoc, &*InsertionPt); } /// Determine whether this alloca is either a VLA or an array. @@ -1430,10 +1446,11 @@ bool llvm::LowerDbgDeclare(Function &F) { // This is a call by-value or some other instruction that takes a // pointer to the variable. Insert a *value* intrinsic that describes // the variable by dereferencing the alloca. + DebugLoc NewLoc = getDebugValueLoc(DDI, nullptr); auto *DerefExpr = DIExpression::append(DDI->getExpression(), dwarf::DW_OP_deref); - DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr, - DDI->getDebugLoc(), CI); + DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr, NewLoc, + CI); } } DDI->eraseFromParent(); diff --git a/llvm/test/DebugInfo/Generic/dbg-value-lower-linenos.ll b/llvm/test/DebugInfo/Generic/dbg-value-lower-linenos.ll new file mode 100644 index 000000000000..892122df0f2f --- /dev/null +++ b/llvm/test/DebugInfo/Generic/dbg-value-lower-linenos.ll @@ -0,0 +1,113 @@ +; RUN: opt < %s -S -mem2reg -instcombine | FileCheck %s + +; The '%bar' alloca will be promoted to an SSA register by mem2reg: test that +; zero line number are assigned to the dbg.value intrinsics that are inserted +; to represent changes in variable value. No machine instructions are +; generated from these dbg.values so their lines are irrelevant, only the +; scope and inlining information must be correct. + +; In the second function here, LowerDbgDeclare will promote various variable +; accesses of a dbg.declare'd alloca into dbg.values. Check that their line +; numbers are sane too. (IR copied from DebugInfo/X86/formal_parameter.ll). + +; CHECK-LABEL: define i32 @foo + +; CHECK-LABEL: bb1: +; CHECK-NEXT: %bar.0 = phi i32 +; CHECK-NEXT: dbg.value(metadata i32 %bar.0,{{.*}}), !dbg ![[UNKNOWN:[0-9]+]] +; CHECK-NEXT: %totest = load +; CHECK-NEXT: %add = add i32 %bar.0 +; CHECK-NEXT: dbg.value(metadata i32 %add, {{.*}}), !dbg ![[UNKNOWN]] +; CHECK-NEXT: %cond = icmp ult +; CHECK-NEXT: br i1 %cond, label %bb1, label %bb2 +; +; CHECK-LABEL: bb2: +; CHECK-NEXT: %toret = add i32 %bar.0, 3 +; CHECK-NEXT: dbg.value(metadata i32 %toret, {{.*}}), !dbg ![[UNKNOWN]] +; CHECK-NEXT: ret i32 %toret + +define i32 @foo(i32 *%bees, i32 *%output) { +entry: + %bar = alloca i32 + call void @llvm.dbg.declare(metadata i32 *%bar, metadata !7, metadata !DIExpression()), !dbg !6 + store i32 0, i32 *%bar + br label %bb1, !dbg !6 + +bb1: + %totest = load i32, i32 *%bees, !dbg !8 + %load1 = load i32, i32 *%bar, !dbg !9 + %add = add i32 %load1, 1, !dbg !10 + store i32 %add, i32 *%bar, !dbg !11 + %toret = add i32 %add, 2, !dbg !12 + %cond = icmp ult i32 %totest, %load1, !dbg !13 + br i1 %cond, label %bb1, label %bb2, !dbg !14 + +bb2: + store i32 %toret, i32 *%bar, !dbg !16 + ret i32 %toret +} + +; In the following, the dbg.value created for the store should get the stores +; line number, the other dbg.values should be unknown. +; CHECK-LABEL: define void @bar +; +; CHECK: dbg.value(metadata i32 %map, metadata ![[MAPVAR:[0-9]+]],{{.*}}), +; CHECK-SAME: !dbg ![[UNKNOWN2:[0-9]+]] +; CHECK-NEXT: store +; CHECK-NEXT: dbg.value(metadata i32* %map.addr, metadata ![[MAPVAR]], +; CHECK-SAME: metadata !DIExpression(DW_OP_deref)), +; CHECK-SAME: !dbg ![[UNKNOWN2]] +; CHECK-NEXT: call +; CHECK-NEXT: load +; CHECK-NEXT: dbg.value(metadata i32 %{{[0-9]+}}, metadata ![[MAPVAR]], +; CHECK-SAME: !dbg ![[UNKNOWN2]] + +define void @bar(i32 %map) !dbg !20 { +entry: + %map.addr = alloca i32, align 4 + store i32 %map, i32* %map.addr, align 4, !dbg !27 + call void @llvm.dbg.declare(metadata i32* %map.addr, metadata !21, metadata !DIExpression()), !dbg !22 + %call = call i32 (i32*, ...) bitcast (i32 (...)* @lookup to i32 (i32*, ...)*)(i32* %map.addr), !dbg !23 +%0 = load i32, i32* %map.addr, align 4, !dbg !24 + %call1 = call i32 (i32, ...) bitcast (i32 (...)* @verify to i32 (i32, ...)*)(i32 %0), !dbg !25 + ret void, !dbg !26 +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) +declare void @llvm.dbg.declare(metadata, metadata, metadata) +declare i32 @verify(...) +declare i32 @lookup(...) + +; CHECK: ![[SUBPROG:[0-9]+]] = distinct !DISubprogram(name: "nope", +; CHECK: ![[UNKNOWN]] = !DILocation(line: 0, scope: ![[SUBPROG]]) + +; CHECK: ![[SUBPROG2:[0-9]+]] = distinct !DISubprogram(name: "thin", +; CHECK: ![[MAPVAR]] = !DILocalVariable(name: "floogie", +; CHECK: ![[UNKNOWN2]] = !DILocation(line: 0 + +!llvm.module.flags = !{!4} +!llvm.dbg.cu = !{!2} +!1 = !DILocalVariable(name: "bees", scope: !5, type: null) +!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug) +!3 = !DIFile(filename: "bees.cpp", directory: "") +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = distinct !DISubprogram(name: "nope", scope: !3, file: !3, line: 1, unit: !2) +!6 = !DILocation(line: 1, scope: !5) +!7 = !DILocalVariable(name: "flannel", scope: !5, type: null) +!8 = !DILocation(line: 2, scope: !5) +!9 = !DILocation(line: 3, scope: !5) +!10 = !DILocation(line: 4, scope: !5) +!11 = !DILocation(line: 5, scope: !5) +!12 = !DILocation(line: 6, scope: !5) +!13 = !DILocation(line: 7, scope: !5) +!14 = !DILocation(line: 8, scope: !5) +!15 = distinct !DISubprogram(name: "wat", scope: !2, file: !3, line: 10, unit: !2) +!16 = !DILocation(line: 9, scope: !15, inlinedAt: !14) +!20 = distinct !DISubprogram(name: "thin", scope: !3, file: !3, line: 20, unit: !2) +!21 = !DILocalVariable(name: "floogie", scope: !20, type: null) +!22 = !DILocation(line: 21, scope: !20) +!23 = !DILocation(line: 22, scope: !20) +!24 = !DILocation(line: 23, scope: !20) +!25 = !DILocation(line: 24, scope: !20) +!26 = !DILocation(line: 25, scope: !20) +!27 = !DILocation(line: 20, scope: !20) diff --git a/llvm/test/DebugInfo/X86/formal_parameter.ll b/llvm/test/DebugInfo/X86/formal_parameter.ll index e3d3c73e0f59..cb3e197dd1d4 100644 --- a/llvm/test/DebugInfo/X86/formal_parameter.ll +++ b/llvm/test/DebugInfo/X86/formal_parameter.ll @@ -41,6 +41,9 @@ entry: ret void, !dbg !22 } +; LOWERING: ![[SCOPE:[0-9]+]] = distinct !DISubprogram(name: "foo", +; LOWERING: ![[LOC]] = !DILocation(line: 0, scope: ![[SCOPE]] + ; Function Attrs: nounwind readnone declare void @llvm.dbg.declare(metadata, metadata, metadata) #1