[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


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@360415 91177308-0d34-0410-b5e6-96231b3b80d8

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();

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)

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