Skip to content

[DebugInfo] Fix line 0 records incorrectly having is_stmt set #166627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[DebugInfo] Fix line 0 records incorrectly having is_stmt set #166627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2199,12 +2199,16 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
if (DL.getLine() == 0 && LastAsmLine == 0)
return;
if (MI == PrologEndLoc) {
Flags |= DWARF2_FLAG_PROLOGUE_END | DWARF2_FLAG_IS_STMT;
Flags |= DWARF2_FLAG_PROLOGUE_END;
// Don't set is_stmt for line 0
Copy link
Contributor

@OCHyams OCHyams Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Don't set is_stmt for line 0
// Don't set is_stmt for line 0.

if (DL.getLine() != 0)
Flags |= DWARF2_FLAG_IS_STMT;
PrologEndLoc = nullptr;
}

if (ScopeUsesKeyInstructions) {
if (IsKey)
// Don't set is_stmt for line 0
Copy link
Contributor

@OCHyams OCHyams Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Don't set is_stmt for line 0
// Don't set is_stmt for line 0.

if (IsKey && DL.getLine() != 0)
Flags |= DWARF2_FLAG_IS_STMT;
} else {
// If the line changed, we call that a new statement; unless we went to
Expand Down Expand Up @@ -2400,8 +2404,11 @@ DwarfDebug::emitInitialLocDirective(const MachineFunction &MF, unsigned CUID) {
(void)getOrCreateDwarfCompileUnit(SP->getUnit());
// We'd like to list the prologue as "not statements" but GDB behaves
// poorly if we do that. Revisit this with caution/GDB (7.5+) testing.
::recordSourceLine(*Asm, SP->getScopeLine(), 0, SP, DWARF2_FLAG_IS_STMT,
CUID, getDwarfVersion(), getUnits());
// However, we should not set is_stmt for line 0.
unsigned ScopeLine = SP->getScopeLine();
unsigned Flags = (ScopeLine != 0) ? DWARF2_FLAG_IS_STMT : 0;
::recordSourceLine(*Asm, ScopeLine, 0, SP, Flags, CUID, getDwarfVersion(),
Copy link
Contributor

@OCHyams OCHyams Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder line zero scope lines can really occur "naturally", I would be slightly surprised. I suppose there's little harm in adding coverage here too but it feels to me like a scope line of 0 would probably mean there's a bug somewhere.

@jmorse do you have a gut instinct on this?

Copy link
Member

@jmorse jmorse Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gut instinct, given that the scope line is directly extracted from the subprogram, is that it can't be a legitimate outcome of optimisation: the scope line is a static property of the source code, not of the program under transformation.

Copy link
Contributor

@OCHyams OCHyams Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case I feel it's better to keep this code as it was previously, as it's almost certainly a bug if this is a line 0 and not particularly harmful if that occurs. So overall better to minimise change and complexity.

I don't feel massively strongly about it, as long as it's consistent between this line and the other scopeLine usage in this file, so if others feel differently I wouldn't insist.

getUnits());
return PrologEndLoc;
}

Expand Down
33 changes: 33 additions & 0 deletions llvm/test/DebugInfo/X86/line-0-no-is-stmt.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
; Test that line 0 records don't have is_stmt set
; This tests the fix for LLVM issue #33870 (Bugzilla #34522)
;
; When scopeLine is 0, the initial location directive should not have is_stmt set.

; RUN: %llc_dwarf -mtriple=x86_64-unknown-linux -O0 -filetype=obj < %s | llvm-dwarfdump --debug-line - | FileCheck %s

; The line table entry for line 0 should exist but not have "is_stmt" in its Flags
; CHECK: Address
; CHECK: 0x{{[0-9a-f]+}} 0 0
; CHECK-NOT: is_stmt
; CHECK: end_sequence
Comment on lines +10 to +12
Copy link
Contributor

@OCHyams OCHyams Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we CHECK: the entire line instead? I feel that will be clearer to understand visually


define void @foo() !dbg !6 {
entry:
ret void, !dbg !9
}

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!2, !3, !4}

!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "test", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
!1 = !DIFile(filename: "test.c", directory: "/tmp")
!2 = !{i32 2, !"Dwarf Version", i32 4}
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = !{i32 1, !"wchar_size", i32 4}
!5 = !{}
; scopeLine is 0 (line 0 should not have is_stmt)
!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 10, type: !7, scopeLine: 0, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !5)
!7 = !DISubroutineType(types: !8)
!8 = !{null}
; Line 0 location (should not have is_stmt)
!9 = !DILocation(line: 0, column: 0, scope: !6)
30 changes: 30 additions & 0 deletions llvm/test/DebugInfo/X86/line-nonzero-has-is-stmt.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
; Test that non-zero line records DO have is_stmt set
; This is for comparison with line 0 behavior (issue #33870 / Bugzilla #34522)
;
; When scopeLine is non-zero, the initial location directive should have is_stmt set.

; RUN: %llc_dwarf -mtriple=x86_64-unknown-linux -O0 -filetype=obj < %s | llvm-dwarfdump --debug-line - | FileCheck %s

; With scopeLine=10, we should see line 10 with is_stmt and prologue_end
; CHECK: 0x{{[0-9a-f]+}} 10 {{[0-9]+}} {{[0-9]+}} {{[0-9]+}} {{[0-9]+}} {{[0-9]+}} is_stmt prologue_end
Comment on lines +8 to +9
Copy link
Contributor

@OCHyams OCHyams Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly worth adding table headers so it's clear what each column is?

alternatively, I think ; CHECK: 0x{{[0-9a-f]+}} 10 {{.*}} is_stmt prologue_end is probably fine.


define void @bar() !dbg !6 {
entry:
ret void, !dbg !9
}

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!2, !3, !4}

!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "test", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
!1 = !DIFile(filename: "test.c", directory: "/tmp")
!2 = !{i32 2, !"Dwarf Version", i32 4}
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = !{i32 1, !"wchar_size", i32 4}
!5 = !{}
; scopeLine is 10 (non-zero line should have is_stmt)
!6 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 10, type: !7, scopeLine: 10, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !5)
!7 = !DISubroutineType(types: !8)
!8 = !{null}
; Line 10 location (should have is_stmt)
!9 = !DILocation(line: 10, column: 5, scope: !6)