[llvm] r343874 - DwarfDebug: Pick next location in case of missing location at block begin

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 5 16:11:22 PDT 2018


Sadly, after this commit TestDataFormatterLibcxxList.py started failing with:

```
output: Process 67333 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = step over
    frame #0: 0x0000000100000eb0 a.out`main at main.cpp:33:16
   30  	    (text_list.push_back(std::string("smart")));
   31  	    
   32  	    printf("// Set second break point at this line.");
-> 33  	    (text_list.push_back(std::string("!!!"))); 
    	               ^
   34  	    
   35  	    std::list<int> countingList = {3141, 3142, 3142,3142,3142, 3142, 3142, 3141};
   36  	    countingList.sort();


runCmd: frame variable text_list[0]
output: (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >) [0] = "goofy"

Expecting sub string: goofy
Matched

runCmd: frame variable text_list[3]
output: None

Expecting sub string: !!!
```

URL: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/10854 <http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/10854>

I confirmed that reverting this fixes the issue.

I think the problem is that we’re breaking on an instruction that looks like it’s on line 33, but it’s actually “before” the full effect of the push_back() is visible.

In which case the test is wrong, because it’s setting the breakpoint too early…

vedant

> On Oct 5, 2018, at 11:29 AM, Matthias Braun via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: matze
> Date: Fri Oct  5 11:29:24 2018
> New Revision: 343874
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=343874&view=rev
> Log:
> DwarfDebug: Pick next location in case of missing location at block begin
> 
> Context: Compiler generated instructions do not have a debug location
> assigned to them. However emitting 0-line records for all of them bloats
> the line tables for very little benefit so we usually avoid doing that.
> 
> Not emitting anything will lead to the previous debug location getting
> applied to the locationless instructions. This is not desirable for
> block begin and after labels. Previously we would emit simply emit
> line-0 records in this case, this patch changes the behavior to do a
> forward search for a debug location in these cases before emitting a
> line-0 record to further reduce line table bloat.
> 
> Inspired by the discussion in https://reviews.llvm.org/D52862
> 
> Added:
>    llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.mir
> Modified:
>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>    llvm/trunk/test/DebugInfo/AArch64/line-header.ll
>    llvm/trunk/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll
>    llvm/trunk/test/DebugInfo/Mips/delay-slot.ll
>    llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll
>    llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=343874&r1=343873&r2=343874&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Fri Oct  5 11:29:24 2018
> @@ -1313,6 +1313,49 @@ void DwarfDebug::collectEntityInfo(Dwarf
>   }
> }
> 
> +static const DebugLoc &
> +findNextDebugLoc(MachineBasicBlock::const_iterator MBBI,
> +                 MachineBasicBlock::const_iterator MBBE) {
> +  static DebugLoc NoLocation;
> +  for ( ; MBBI != MBBE; ++MBBI) {
> +    if (MBBI->isDebugInstr())
> +      continue;
> +    const DebugLoc &DL = MBBI->getDebugLoc();
> +    if (DL)
> +      return DL;
> +  }
> +  return NoLocation;
> +}
> +
> +void DwarfDebug::emitDebugLoc(const DebugLoc &DL) {
> +  unsigned LastAsmLine =
> +      Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine();
> +
> +  // 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.)
> +  unsigned Line = DL.getLine();
> +  if (PrevInstLoc && Line == 0 && LastAsmLine == 0)
> +    return;
> +  unsigned Flags = 0;
> +  if (DL == PrologEndLoc) {
> +    Flags |= DWARF2_FLAG_PROLOGUE_END | DWARF2_FLAG_IS_STMT;
> +    PrologEndLoc = DebugLoc();
> +  }
> +  // 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 (Line && Line != OldLine)
> +    Flags |= DWARF2_FLAG_IS_STMT;
> +
> +  const MDNode *Scope = DL.getScope();
> +  recordSourceLine(Line, DL.getCol(), Scope, Flags);
> +
> +  // If we're not at line 0, remember this location.
> +  if (Line)
> +    PrevInstLoc = DL;
> +}
> +
> // Process beginning of an instruction.
> void DwarfDebug::beginInstruction(const MachineInstr *MI) {
>   DebugHandlerBase::beginInstruction(MI);
> @@ -1352,54 +1395,41 @@ void DwarfDebug::beginInstruction(const
>     // If we have already emitted a line-0 record, don't repeat it.
>     if (LastAsmLine == 0)
>       return;
> +    // By default we emit nothing to avoid line table bloat. However at the
> +    // beginning of a basic block or after a label it is undesirable to let
> +    // the previous location unchanged. In these cases do a forward search for
> +    // the next valid debug location.
> +    if (UnknownLocations == Default) {
> +      const MachineBasicBlock &MBB = *MI->getParent();
> +      if (!PrevLabel && PrevInstBB == &MBB)
> +        return;
> +
> +      const DebugLoc &NextDL = findNextDebugLoc(MI->getIterator(), MBB.end());
> +      if (NextDL) {
> +        emitDebugLoc(NextDL);
> +        return;
> +      }
> +    }
> +
> +    // We should emit a line-0 record.
>     // 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 and column numbers, if we can, to save space in
> -      // the encoded line table.
> -      // Do not update PrevInstLoc, it remembers the last non-0 line.
> -      const MDNode *Scope = nullptr;
> -      unsigned Column = 0;
> -      if (PrevInstLoc) {
> -        Scope = PrevInstLoc.getScope();
> -        Column = PrevInstLoc.getCol();
> -      }
> -      recordSourceLine(/*Line=*/0, Column, Scope, /*Flags=*/0);
> +    // Emit a line-0 record now.
> +    // Preserve the file and column numbers, if we can, to save space in
> +    // the encoded line table.
> +    // Do not update PrevInstLoc, it remembers the last non-0 line.
> +    const MDNode *Scope = nullptr;
> +    unsigned Column = 0;
> +    if (PrevInstLoc) {
> +      Scope = PrevInstLoc.getScope();
> +      Column = PrevInstLoc.getCol();
>     }
> +    recordSourceLine(/*Line=*/0, Column, Scope, /*Flags=*/0);
>     return;
>   }
> 
> -  // 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 (PrevInstLoc && DL.getLine() == 0 && LastAsmLine == 0)
> -    return;
> -  unsigned Flags = 0;
> -  if (DL == PrologEndLoc) {
> -    Flags |= DWARF2_FLAG_PROLOGUE_END | DWARF2_FLAG_IS_STMT;
> -    PrologEndLoc = DebugLoc();
> -  }
> -  // 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;
> +  emitDebugLoc(DL);
> }
> 
> static DebugLoc findPrologueEndLoc(const MachineFunction *MF) {
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=343874&r1=343873&r2=343874&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Fri Oct  5 11:29:24 2018
> @@ -719,6 +719,9 @@ public:
>   bool tuneForLLDB() const { return DebuggerTuning == DebuggerKind::LLDB; }
>   bool tuneForSCE() const { return DebuggerTuning == DebuggerKind::SCE; }
>   /// @}
> +
> +private:
> +  void emitDebugLoc(const DebugLoc &DL);
> };
> 
> } // end namespace llvm
> 
> Modified: llvm/trunk/test/DebugInfo/AArch64/line-header.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/AArch64/line-header.ll?rev=343874&r1=343873&r2=343874&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/AArch64/line-header.ll (original)
> +++ llvm/trunk/test/DebugInfo/AArch64/line-header.ll Fri Oct  5 11:29:24 2018
> @@ -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: 0x0000003f
> +CHECK: total_length: 0x0000003c
> 
> Modified: llvm/trunk/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll?rev=343874&r1=343873&r2=343874&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll (original)
> +++ llvm/trunk/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll Fri Oct  5 11:29:24 2018
> @@ -31,11 +31,10 @@ if.then:
> 
> if.end:                                           ; preds = %entry
> ; Materialize the constant.
> -; CHECK:      .loc    1 0
> +; CHECK:      .loc    1 7 5
> ; CHECK-NEXT: mvn     r0, #0
> 
> ; The backend performs the store to %retval first, for some reason.
> -; CHECK-NEXT: .loc    1 7 5
> ; CHECK-NEXT: str     r0, [sp, #4]
>   store i32 -1, i32* %x, align 4, !dbg !19
> 
> 
> Modified: llvm/trunk/test/DebugInfo/Mips/delay-slot.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Mips/delay-slot.ll?rev=343874&r1=343873&r2=343874&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/Mips/delay-slot.ll (original)
> +++ llvm/trunk/test/DebugInfo/Mips/delay-slot.ll Fri Oct  5 11:29:24 2018
> @@ -16,7 +16,7 @@
> ; CHECK: 0x0000000000000004      2      0      1   0             0  is_stmt prologue_end
> ; CHECK: 0x0000000000000024      3      0      1   0             0  is_stmt
> ; CHECK: 0x0000000000000034      4      0      1   0             0  is_stmt
> -; CHECK: 0x0000000000000048      5      0      1   0             0  is_stmt
> +; CHECK: 0x0000000000000044      5      0      1   0             0  is_stmt
> ; CHECK: 0x0000000000000058      5      0      1   0             0  is_stmt end_sequence
> 
> 
> 
> Modified: llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll?rev=343874&r1=343873&r2=343874&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll (original)
> +++ llvm/trunk/test/DebugInfo/NVPTX/debug-info.ll Fri Oct  5 11:29:24 2018
> @@ -36,6 +36,7 @@
> ; CHECK: setp.ge.s32     %p{{.+}}, %r{{.+}}, %r{{.+}};
> ; CHECK: .loc [[DEBUG_INFO_CU]] 7 7
> ; CHECK: @%p{{.+}} bra   [[BB:.+]];
> +; CHECK: .loc [[DEBUG_INFO_CU]] 8 13
> ; CHECK: ld.param.f32    %f{{.+}}, [{{.+}}];
> ; CHECK: ld.param.u64    %rd{{.+}}, [{{.+}}];
> ; CHECK: cvta.to.global.u64      %rd{{.+}}, %rd{{.+}};
> @@ -43,7 +44,6 @@
> ; CHECK: cvta.to.global.u64      %rd{{.+}}, %rd{{.+}};
> ; CHECK: mul.wide.u32    %rd{{.+}}, %r{{.+}}, 4;
> ; CHECK: add.s64         %rd{{.+}}, %rd{{.+}}, %rd{{.+}};
> -; CHECK: .loc [[DEBUG_INFO_CU]] 8 13
> ; CHECK: ld.global.f32   %f{{.+}}, [%rd{{.+}}];
> ; CHECK: add.s64         %rd{{.+}}, %rd{{.+}}, %rd{{.+}};
> ; CHECK: .loc [[DEBUG_INFO_CU]] 8 19
> 
> Modified: llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll?rev=343874&r1=343873&r2=343874&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll (original)
> +++ llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.ll Fri Oct  5 11:29:24 2018
> @@ -40,15 +40,14 @@ if.end:
>   ret void, !dbg !14
> }
> 
> -; CHECK:      .loc 1 7 7
> +; CHECK:      .loc 1 7 7 prologue_end
> ; CHECK-NOT:  .loc
> -; CHECK:      .loc 1 0 7 is_stmt 0
> +; CHECK:      # %bb.1
> +; CHECK-NEXT: .file 2 "/tests/include.h"
> +; CHECK-NEXT: .loc 2 20 5
> ; CHECK-NOT:  .loc
> -; CHECK:      .loc 2 20 5 is_stmt 1
> ; CHECK:      .LBB0_2:
> -; CHECK-NEXT: .loc 2 0 5 is_stmt 0
> -; CHECK-NOT:  .loc
> -; CHECK:      .loc 1 10 3 is_stmt 1
> +; CHECK:      .loc 1 10 3
> ;
> ; DISABLE-NOT: .loc 1 0
> 
> 
> Added: llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.mir?rev=343874&view=auto
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.mir (added)
> +++ llvm/trunk/test/DebugInfo/X86/dwarf-no-source-loc.mir Fri Oct  5 11:29:24 2018
> @@ -0,0 +1,74 @@
> +# RUN: llc -o - %s -start-before=patchable-function -use-unknown-locations=Default | FileCheck %s --check-prefixes=CHECK,DEFAULT
> +# RUN: llc -o - %s -start-before=patchable-function -use-unknown-locations=Enable | FileCheck %s --check-prefixes=CHECK,ENABLE
> +# RUN: llc -o - %s -start-before=patchable-function -use-unknown-locations=Disable | FileCheck %s --check-prefixes=CHECK,DISABLE
> +--- |
> +  target triple = "x86_64--"
> +  
> +  !0 = !DIFile(filename: "dwarf-no-source-loc.mir", directory: "/")
> +  !1 = distinct !DICompileUnit(file: !0, language: DW_LANG_C, emissionKind: LineTablesOnly)
> +  !2 = distinct !DISubprogram(name: "func", unit: !1)
> +  !3 = !DILocation(line: 17, scope: !2)
> +  !4 = !DILocation(line: 42, scope: !2)
> +
> +  !llvm.dbg.cu = !{!1}
> +  !llvm.module.flags = !{!10, !11}
> +  !10 = !{i32 2, !"Dwarf Version", i32 4}
> +  !11 = !{i32 2, !"Debug Info Version", i32 3}
> +  
> +  define void @func() !dbg !2 {
> +    unreachable
> +  }
> +...
> +---
> +name: func
> +body: |
> +  bb.0:
> +    NOOP
> +    NOOP
> +    $eax = MOV32ri 1, debug-location !3
> +    ; CHECK-LABEL: bb.0
> +    ; CHECK: nop
> +    ; CHECK: nop
> +    ; CHECK: .loc 1 17 0 prologue_end
> +    ; CHECK: movl $1, %eax
> +
> +  bb.1:
> +    NOOP
> +    $ebx = MOV32ri 2, debug-location !4
> +    ; CHECK-LABEL: bb.1
> +    ; DEFAULT: .loc 1 42 0
> +    ; ENABLE: .loc 1 0
> +    ; DISABLE-NOT: .loc 1 0
> +    ; CHECK: nop
> +    ; ENABLE: .loc 1 42 0
> +    ; CHECK: movl $2, %ebx
> +
> +  bb.2:
> +    NOOP
> +    ; CHECK-LABEL: bb.2
> +    ; DEFAULT: .loc 1 0 0 is_stmt 0
> +    ; ENABLE: .loc 1 0 0 is_stmt 0
> +    ; DISABLE-NOT: .loc 1 0
> +    ; CHECK: nop
> +
> +  bb.3:
> +    NOOP
> +    $ecx = MOV32ri 3, debug-location !3
> +    ; CHECK-LABEL: bb.3
> +    ; CHECK: nop
> +    ; DEFAULT: .loc 1 17 0 is_stmt 1
> +    ; ENABLE: .loc 1 17 0 is_stmt 1
> +    ; DISABLE-NOT: .loc 1 0
> +    ; CHECK: movl $3, %ecx
> +
> +  bb.4:
> +    NOOP
> +    $edx = MOV32ri 4, debug-location !4
> +    ; CHECK: bb.4
> +    ; DEFAULT: .loc 1 42 0
> +    ; ENABLE: .loc 1 0 0 is_stmt 0
> +    ; DISABLE-NOT: .loc 1 0
> +    ; CHECK: nop
> +    ; ENABLE: .loc 1 42 0 is_stmt 1
> +    ; CHECK: movl $4, %edx
> +...
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181005/f5f4d254/attachment.html>


More information about the llvm-commits mailing list