[lldb-dev] [llvm] r343874 - DwarfDebug: Pick next location in case of missing location at block begin
Vedant Kumar via lldb-dev
lldb-dev 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/lldb-dev/attachments/20181005/f5f4d254/attachment-0001.html>
More information about the lldb-dev
mailing list