]> granicus.if.org Git - llvm/commitdiff
Recommit r288212: Emit 'no line' information for interesting 'orphan' instructions.
authorPaul Robinson <paul.robinson@sony.com>
Mon, 12 Dec 2016 20:49:11 +0000 (20:49 +0000)
committerPaul Robinson <paul.robinson@sony.com>
Mon, 12 Dec 2016 20:49:11 +0000 (20:49 +0000)
DWARF specifies that "line 0" really means "no appropriate source
location" in the line table.  By default, use this for branch targets
and some other cases that have no specified source location, to
prevent inheriting unfortunate line numbers from physically preceding
instructions (which might be from completely unrelated source).

Updated patch allows enabling or suppressing this behavior for all
unspecified source locations.

Differential Revision: http://reviews.llvm.org/D24180

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

lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
lib/CodeGen/AsmPrinter/DebugHandlerBase.h
lib/CodeGen/AsmPrinter/DwarfDebug.cpp
test/CodeGen/X86/stack-protector.ll
test/CodeGen/X86/unknown-location.ll
test/DebugInfo/AArch64/line-header.ll
test/DebugInfo/MIR/X86/no-cfi-loc.mir
test/DebugInfo/X86/dwarf-no-source-loc.ll [new file with mode: 0644]
test/DebugInfo/X86/tail-merge.ll

index ce57f1730bfc8c6435f38ba9c33bc3be76c07835..aa71449b2c082279ec340bdd97dde4e3609a8435 100644 (file)
@@ -200,8 +200,10 @@ void DebugHandlerBase::endInstruction() {
   assert(CurMI != nullptr);
   // Don't create a new label after DBG_VALUE instructions.
   // They don't generate code.
-  if (!CurMI->isDebugValue())
+  if (!CurMI->isDebugValue()) {
     PrevLabel = nullptr;
+    PrevInstBB = CurMI->getParent();
+  }
 
   DenseMap<const MachineInstr *, MCSymbol *>::iterator I =
       LabelsAfterInsn.find(CurMI);
index 7219b05d33c70a88bae308450e15d346885b8a89..c00fa189d94af589a0a4735d79191e3df08c1ccb 100644 (file)
@@ -38,10 +38,12 @@ protected:
   MachineModuleInfo *MMI;
 
   /// Previous instruction's location information. This is used to
-  /// determine label location to indicate scope boundries in dwarf
-  /// debug info.
+  /// determine label location to indicate scope boundaries in debug info.
+  /// We track the previous instruction's source location (if not line 0),
+  /// whether it was a label, and its parent BB.
   DebugLoc PrevInstLoc;
   MCSymbol *PrevLabel = nullptr;
+  const MachineBasicBlock *PrevInstBB = nullptr;
 
   /// This location indicates end of function prologue and beginning of
   /// function body.
index 12949e0991df22644676dd0221159c49180f80c5..4ebc54555cc8f23615d44fd0c8e5feb8476174bb 100644 (file)
@@ -62,11 +62,6 @@ static cl::opt<bool>
 DisableDebugInfoPrinting("disable-debug-info-print", cl::Hidden,
                          cl::desc("Disable debug info printing"));
 
-static cl::opt<bool> UnknownLocations(
-    "use-unknown-locations", cl::Hidden,
-    cl::desc("Make an absence of debug location information explicit."),
-    cl::init(false));
-
 static cl::opt<bool>
 GenerateGnuPubSections("generate-gnu-dwarf-pub-sections", cl::Hidden,
                        cl::desc("Generate GNU-style pubnames and pubtypes"),
@@ -81,6 +76,13 @@ namespace {
 enum DefaultOnOff { Default, Enable, Disable };
 }
 
+static cl::opt<DefaultOnOff> UnknownLocations(
+    "use-unknown-locations", cl::Hidden,
+    cl::desc("Make an absence of debug location information explicit."),
+    cl::values(clEnumVal(Default, "At top of block or after label"),
+               clEnumVal(Enable, "In all cases"), clEnumVal(Disable, "Never")),
+    cl::init(Default));
+
 static cl::opt<DefaultOnOff>
 DwarfAccelTables("dwarf-accel-tables", cl::Hidden,
                  cl::desc("Output prototype dwarf accelerator tables."),
@@ -1010,31 +1012,73 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
   if (MI->isDebugValue() || MI->isCFIInstruction())
     return;
   const DebugLoc &DL = MI->getDebugLoc();
-  if (DL == PrevInstLoc)
+  // When we emit a line-0 record, we don't update PrevInstLoc; so look at
+  // the last line number actually emitted, to see if it was line 0.
+  unsigned LastAsmLine =
+      Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine();
+
+  if (DL == PrevInstLoc) {
+    // If we have an ongoing unspecified location, nothing to do here.
+    if (!DL)
+      return;
+    // We have an explicit location, same as the previous location.
+    // But we might be coming back to it after a line 0 record.
+    if (LastAsmLine == 0 && DL.getLine() != 0) {
+      // Reinstate the source location but not marked as a statement.
+      const MDNode *Scope = DL.getScope();
+      recordSourceLine(DL.getLine(), DL.getCol(), Scope, /*Flags=*/0);
+    }
     return;
+  }
 
   if (!DL) {
     // We have an unspecified location, which might want to be line 0.
-    if (UnknownLocations) {
-      PrevInstLoc = DL;
-      recordSourceLine(0, 0, nullptr, 0);
+    // If we have already emitted a line-0 record, don't repeat it.
+    if (LastAsmLine == 0)
+      return;
+    // If user said Don't Do That, don't do that.
+    if (UnknownLocations == Disable)
+      return;
+    // See if we have a reason to emit a line-0 record now.
+    // Reasons to emit a line-0 record include:
+    // - User asked for it (UnknownLocations).
+    // - Instruction has a label, so it's referenced from somewhere else,
+    //   possibly debug information; we want it to have a source location.
+    // - Instruction is at the top of a block; we don't want to inherit the
+    //   location from the physically previous (maybe unrelated) block.
+    if (UnknownLocations == Enable || PrevLabel ||
+        (PrevInstBB && PrevInstBB != MI->getParent())) {
+      // Preserve the file number, if we can, to save space in the line table.
+      // Do not update PrevInstLoc, it remembers the last non-0 line.
+      // FIXME: Also preserve the column number, to save more space?
+      const MDNode *Scope = PrevInstLoc ? PrevInstLoc.getScope() : nullptr;
+      recordSourceLine(0, 0, Scope, 0);
     }
     return;
   }
 
-  // We have a new, explicit location.
+  // We have an explicit location, different from the previous location.
+  // Don't repeat a line-0 record, but otherwise emit the new location.
+  // (The new location might be an explicit line 0, which we do emit.)
+  if (DL.getLine() == 0 && LastAsmLine == 0)
+    return;
   unsigned Flags = 0;
-  PrevInstLoc = DL;
   if (DL == PrologEndLoc) {
     Flags |= DWARF2_FLAG_PROLOGUE_END | DWARF2_FLAG_IS_STMT;
     PrologEndLoc = DebugLoc();
   }
-  if (DL.getLine() !=
-      Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine())
+  // If the line changed, we call that a new statement; unless we went to
+  // line 0 and came back, in which case it is not a new statement.
+  unsigned OldLine = PrevInstLoc ? PrevInstLoc.getLine() : LastAsmLine;
+  if (DL.getLine() && DL.getLine() != OldLine)
     Flags |= DWARF2_FLAG_IS_STMT;
 
   const MDNode *Scope = DL.getScope();
   recordSourceLine(DL.getLine(), DL.getCol(), Scope, Flags);
+
+  // If we're not at line 0, remember this location.
+  if (DL.getLine())
+    PrevInstLoc = DL;
 }
 
 static DebugLoc findPrologueEndLoc(const MachineFunction *MF) {
index d3ff61743ed4aefb522c4b3200b48911f1098325..5166ed5b02aaf38792f5dfc4b80524d12ce56516 100644 (file)
@@ -3888,23 +3888,27 @@ entry:
 define void @test32() #1 !dbg !7 {
 entry:
 ; LINUX-I386-LABEL: test32:
-; LINUX-I386:       .loc 1 0 0 prologue_end
+; LINUX-I386:       .loc 1 4 2 prologue_end
+; LINUX-I386:       .loc 1 0 0
 ; LINUX-I386-NEXT:  calll __stack_chk_fail
 
 ; LINUX-X64-LABEL: test32:
-; LINUX-X64:       .loc 1 0 0 prologue_end
+; LINUX-X64:       .loc 1 4 2 prologue_end
+; LINUX-X64:       .loc 1 0 0
 ; LINUX-X64-NEXT:  callq __stack_chk_fail
 
 ; LINUX-KERNEL-X64-LABEL: test32:
-; LINUX-KERNEL-X64:       .loc 1 0 0 prologue_end
+; LINUX-KERNEL-X64:       .loc 1 4 2 prologue_end
+; LINUX-KERNEL-X64:       .loc 1 0 0
 ; LINUX-KERNEL-X64-NEXT:  callq __stack_chk_fail
 
 ; OPENBSD-AMD64-LABEL: test32:
-; OPENBSD-AMD64:       .loc 1 0 0 prologue_end
+; OPENBSD-AMD64:       .loc 1 4 2 prologue_end
+; OPENBSD-AMD64:       .loc 1 0 0
 ; OPENBSD-AMD64-NEXT:  movl
 ; OPENBSD-AMD64-NEXT:  callq __stack_smash_handler
   %0 = alloca [5 x i8], align 1
-  ret void
+  ret void, !dbg !9
 }
 
 declare double @testi_aux()
@@ -3940,3 +3944,4 @@ attributes #5 = { ssp "stack-protector-buffer-size"="6" }
 !6 = distinct !DISubprogram(name: "__stack_chk_fail", scope: !1, type: !8, unit: !0)
 !7 = distinct !DISubprogram(name: "test32", scope: !1, type: !8, unit: !0)
 !8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 4, column: 2, scope: !7)
index 1058994d0ee173f6797ca52a30fa65e64824f4f8..b4c44fc3e46e94582d8242fec2fc754af3c7fee7 100644 (file)
@@ -1,4 +1,4 @@
-; RUN: llc < %s -asm-verbose=false -mtriple=x86_64-apple-darwin10 -use-unknown-locations | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -mtriple=x86_64-apple-darwin10 -use-unknown-locations=Enable | FileCheck %s
 
 ; The divide instruction does not have a debug location. CodeGen should
 ; represent this in the debug information. This is done by setting line
index f8220233faab305048a92d41bee6610c8de4b5ef..38d06f263e25fc3cfbb59773f9356ee2b76097a0 100644 (file)
@@ -3,4 +3,4 @@
 
 ; check line table length is correctly calculated for both big and little endian
 CHECK-LABEL: .debug_line contents:
-CHECK: total_length: 0x0000003c
+CHECK: total_length: 0x0000003e
index 28d4bf24b743dfa48fe90ef1475feb3efa5ea860..4ea84592cfc944f42d29b394d70234663da12087 100644 (file)
@@ -1,6 +1,6 @@
 # Verify that a CFI instruction with no debug location
 # does not result in a line-0 location in the assembler.
-# RUN: %llc_dwarf -start-after=prologepilog -march=x86-64 -use-unknown-locations %s -o - | FileCheck %s
+# RUN: %llc_dwarf -start-after=prologepilog -march=x86-64 -use-unknown-locations=Enable %s -o - | FileCheck %s
 #
 # CHECK-NOT: .loc 1 0
 # CHECK:     .cfi_def_cfa_offset
diff --git a/test/DebugInfo/X86/dwarf-no-source-loc.ll b/test/DebugInfo/X86/dwarf-no-source-loc.ll
new file mode 100644 (file)
index 0000000..1230e97
--- /dev/null
@@ -0,0 +1,77 @@
+; Verify "no source location" directives appear in appropriate places,
+; but don't appear when we explicitly suppress them.
+; RUN: llc < %s -o - | FileCheck %s
+; RUN: llc < %s -o - -use-unknown-locations=Disable | FileCheck %s --check-prefix=DISABLE
+
+; Generated from this .cpp targeting linux using -g
+; and then removed function attributes as clutter.
+;
+; void bar(int *);
+; void baz(int *);
+; # 5 "no-source-loc.cpp"
+; void foo(int x) {
+;   int z;
+;   if (x)
+; # 20 "include.h"
+;     bar(&z);
+; # 10 "no-source-loc.cpp"
+;   baz(&z);
+; }
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: uwtable
+define void @_Z3fooi(i32 %x) !dbg !6 {
+entry:
+  %x.addr = alloca i32, align 4
+  %z = alloca i32, align 4
+  store i32 %x, i32* %x.addr, align 4
+  %0 = load i32, i32* %x.addr, align 4, !dbg !8
+  %tobool = icmp ne i32 %0, 0, !dbg !8
+  br i1 %tobool, label %if.then, label %if.end, !dbg !8
+
+if.then:                                          ; preds = %entry
+  call void @_Z3barPi(i32* %z), !dbg !9
+  br label %if.end, !dbg !9
+
+if.end:                                           ; preds = %if.then, %entry
+  call void @_Z3bazPi(i32* %z), !dbg !12
+  ret void, !dbg !14
+}
+
+; CHECK:      .loc 1 7 7
+; CHECK-NOT:  .loc
+; CHECK:      .loc 1 0 0 is_stmt 0
+; CHECK-NOT:  .loc
+; CHECK:      .loc 2 20 5 is_stmt 1
+; CHECK:      .LBB0_2:
+; CHECK-NEXT: .loc 2 0 0 is_stmt 0
+; CHECK-NOT:  .loc
+; CHECK:      .loc 1 10 3 is_stmt 1
+;
+; DISABLE-NOT: .loc 1 0
+
+declare void @_Z3barPi(i32*)
+
+declare void @_Z3bazPi(i32*)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 4.0.0 (trunk 278782)", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "no-source-loc.cpp", directory: "/tests")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{!"clang version 4.0.0 (trunk 278782)"}
+!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 5, type: !7, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!7 = !DISubroutineType(types: !2)
+!8 = !DILocation(line: 7, column: 7, scope: !6)
+!9 = !DILocation(line: 20, column: 5, scope: !10)
+!10 = !DILexicalBlockFile(scope: !6, file: !11, discriminator: 0)
+!11 = !DIFile(filename: "include.h", directory: "/tests")
+!12 = !DILocation(line: 10, column: 3, scope: !13)
+!13 = !DILexicalBlockFile(scope: !6, file: !1, discriminator: 0)
+!14 = !DILocation(line: 11, column: 1, scope: !13)
index d87866d58b84978af8ef118cc3d34238dc1e6932..cbdcf1ac29c8cf895fec96836730164cd0356764 100644 (file)
@@ -1,4 +1,4 @@
-; RUN: llc %s -mtriple=x86_64-unknown-unknown -use-unknown-locations=true -o - | FileCheck %s
+; RUN: llc %s -mtriple=x86_64-unknown-unknown -use-unknown-locations=Enable -o - | FileCheck %s
 
 ; Generated with "clang -gline-tables-only -c -emit-llvm -o - | opt -sroa -S"
 ; from source: