[DebugInfo] Enable CodeView DebugInfo for basic block sections

[TaoPan]

Summary

With basic block sections, there are multiple flexible text sections for
a function, allow multiple .cv_loc directives for a function.

Signed-off-by: Pan, Tao <[e-mail]>

[TaoPan]

Could you please have a review?

[rnk]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1734 ↗

(On Diff #341053)

I wrote some comments about this on D99487, since there was discussion there about this.

llvm/lib/MC/MCStreamer.cpp

331 ↗

(On Diff #341053)

This check prevents us from crashing later in CodeViewContext::emitLineTableForFunction when it calls:

OS.emitAbsoluteSymbolDiff(J->getLabel(), FuncBegin, 4);

If those two labels are in different sections, this just moves the crash later. Which makes me wonder if any BB section splitting happens in the test case you added. Does it exercise that yet, or does that come later?

The codeview line table format assumes functions are contiguous to avoid extra relocations. There is a relocation to the function start at the beginning, and the rest is a sequence of code offsets and line numbers. I wonder if it would be better to plan to produce a line table per section instead of trying to make one holistic line table. Then we could keep this check. So, if that is the long term plan, maybe we can leave this code here in this change.

llvm/test/DebugInfo/COFF/basic-block-sections.ll

14

Can you include the .section directives in this test case?

[TaoPan]

Add .section directives test

[TaoPan]

Forgot .section directives in the first CHECK of the last update

[TaoPan]

Thanks for your review comment!
I'll take leave between 5.1~5.6 for national holiday and personal affair, I'll response further review comment after then.

llvm/lib/MC/MCStreamer.cpp

331 ↗

(On Diff #341053)

Both removing and restoring this check, OS.emitAbsoluteSymbolDiff(J->getLabel(), FuncBegin, 4); of CodeViewContext::emitLineTableForFunction won't be called in my test.
I compared to llvm/test/DebugInfo/X86/basic-block-sections_1.ll (Linux ELF test), it's also not per section line table, it's per file line table, the format is as below, no function and section information.
Address Line Column File ISA Discriminator Flags
0x0000000000000000 1 0 1 0 0 is_stmt
Is there any reason that need per section line table?

llvm/test/DebugInfo/COFF/basic-block-sections.ll

14

I added .section directives

[rnk]

llvm/lib/MC/MCStreamer.cpp

331 ↗

(On Diff #341053)

I'm pretty confident that we do reach emitAbsoluteSymbolDiff when emitting emitLineTableForFunction, but I discovered that it doesn't assert, it produces a relocation instead. I haven't reasoned through what the relocation does, but I see it in your test case. I confused emitAbsoluteSymbolDiff with the computeLabelDiff helper in the same file, see here:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/MC/MCCodeView.cpp#L454

So, I agree, removing this assertion doesn't cause a crash now, but it will if you introduce inlining into the test case and attempt to produce an inline line table. (.cv_inline_linetable).

Even though we don't crash in the non-inline line table case, it's not clear to me what a debugger would do with a an unsorted, non-ascending, possibly negative line table, which is what you would get after the linker applies relocations.

[TaoPan]

Remove same modification of parent D99487 and add .cv_inline_linetable to test

[TaoPan]

Remove same modification of D100735

[TaoPan]

git rebase

[TaoPan]

llvm/lib/MC/MCStreamer.cpp

331 ↗

(On Diff #341053)

Thanks for your guidance!
I added inline function to test, I can produce an inline line table (.cv_inline_linetable) with llc to asm then llvm-mc to obj (line 2 and 4 in my test). I can reproduce your mentioned problem if llc directly to obj, I'm investigating the problem that Success is false in https://github.com/llvm/llvm-project/blob/main/llvm/lib/MC/MCCodeView.cpp#L454

[TaoPan]

llvm/lib/MC/MCStreamer.cpp

331 ↗

(On Diff #341053)

Thanks Rnk!
We are facing two problems in turn.

1. At first, with BB section splitting, relocations at the beginning of all BB sections are needed, so this check should be removed, it blocks emit later relocations after the first one.
2. Then emit line table, I understood the codeview line table format depends on beginning relocation and contiguous text. Before enabling BB section splitting, function is the smallest unit satisfying the condition, after enabling BB section splitting, section become the smallest unit satisfying the condition. Your design that produce a line table per section is good.

I think it’s ok fix 1 and 2 at the same time.
As you know, 2 is not easy handling, it should be a long term plan.
How do you think of decoupling D99487 BB sections on Windows COFF(release mode, is controlled by default off option) and BB sections CodeView DebugInfo on Windows COFF (debug mode)?

[TaoPan]

Disable .cv_loc directive emission outside the primary function section.

[rnk]

llvm/lib/MC/MCStreamer.cpp

328 ↗

(On Diff #349178)

Please make this change in the AsmPrinter, not the assembler. The assembler should still emit an error: if the user directly uses cv_loc directives, the assembler should produce an error. The compiler needs to avoid asking the assembler to do the impossible.

[TaoPan]

Don't emit location of BB sections

[TaoPan]

llvm/lib/MC/MCStreamer.cpp

328 ↗

(On Diff #349178)

Thanks for your review comments!
I moved change from assembler to AsmPrinter, the new added condition of not emitting location is same as the condition of creating BB section (calling to getSectionForMachineBasicBlock()) in AsmPrinter.cpp.

[rnk]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

2947

Please add a comment here.

Also, are you sure this is the right condition? Aren't there MBBs that do not begin sections but belong to a non-primary section?

[TaoPan]

Change condition from MachineBasicBlock is begin section to MachineFunction has BB sections and add comment

[TaoPan]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

2947

Thanks!
I added comment and changed condition to the MachineFunction which has the MachineInstr has BB sections and the MachineBasicBlock which has the MachineInstr is not entry block, how do you think?

[rnk]

Sorry for the delay.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

2952

Instead of just the entry block, can this be MI->getParent()->getSectionID() == MBBSectionId::Default? That would cover all blocks in the default section, right?

[tmsriram]

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

2949

Typo, "section" and "primary". Similar comment rnk@. The condition below only checks whether it is not the entry block but the comment says it is all non-primary basic blocks.

[TaoPan]

Thanks for your review comments!

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

2949

Thanks for pointing out the confuse comment! How about "The codeview line table format requires functions are contiguous, if function has BB sections, all MBBs have their own section, all sections are not contiguous, emit codeview relocation only for entry block MBB in this case."?

2952

I think MBBSectionId can not be used for distinguishing basic blocks in the default section or not. The enum values of MBBSectionId are Default = 0, Exception, Cold. Section ID of all MBBs are MBBSectionId::Default in BB Sections case.
I tried C function like below:
int bazb(...) {

...           // Unique section for function bazb, the MBB is entry block: .text,"xr",one_only,"?bazb@@YAHH@Z"
if (...) {
  ...         // BB section 1: .text,"xr",associative,"?bazb@@YAHH@Z.__part.1"
} else {
  ...         // BB section 2: .text,"xr",associative,"?bazb@@YAHH@Z.__part.2"
}
...           // BB section 3: .text,"xr",associative,"?bazb@@YAHH@Z.__part.3"
if (...) {
  ...         // BB section 4: .text,"xr",associative,"?bazb@@YAHH@Z.__part.4"
} else {
  ...         // BB section 5: .text,"xr",associative,"?bazb@@YAHH@Z.__part.5"
}
...           // BB section 6: .text,"xr",associative,"?bazb@@YAHH@Z.__part.6"

}
I think if function has BB sections, all basic blocks have their own section, all non-EntryBlock sections are relocatable BB Sections. In other words, entry block is unique block in the default section.

[rnk]

lgtm

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

2949

Please fix the typo before committing.

2952

I see. I guess that's me being confused between intra-function hot cold splitting and BB sections. The condition is correct, then.

[TaoPan]

Fix the typo of comment

[TaoPan]

In D101421#2854680, @rnk wrote:

lgtm

Thanks for your big help!

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

2949

I updated the comments.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

 
 void CodeViewDebug::beginInstruction(const MachineInstr *MI) {
   DebugHandlerBase::beginInstruction(MI);
 
   // Ignore DBG_VALUE and DBG_LABEL locations and function prologue.
   if (!Asm || !CurFn || MI->isDebugInstr() ||
       MI->getFlag(MachineInstr::FrameSetup))
     return;
+  // The codeview line table format requires functions are contiguous, if
+  // function has BB sections, all MBBs have their own section, all sections are
+  // not contiguous, emit codeview relocation only for entry block MBB in this
+  // case.
+  if (MI->getParent()->getParent()->hasBBSections() &&
+      !MI->getParent()->isEntryBlock())
+    return;
 
   // If the first instruction of a new MBB has no location, find the first
   // instruction with a location and use that.
   DebugLoc DL = MI->getDebugLoc();
   if (!isUsableDebugLoc(DL) && MI->getParent() != PrevInstBB) {
     for (const auto &NextMI : *MI->getParent()) {
       if (NextMI.isDebugInstr())
         continue;

llvm/test/DebugInfo/COFF/basic-block-sections.ll

+; RUN: llc -O0 %s -function-sections -basic-block-sections=all -unique-basic-block-section-names -mtriple=i686-pc-win32 -filetype=asm -frame-pointer=all -o - | FileCheck --check-prefix=X86 %s
+; RUN: llc -O0 %s -basic-block-sections=all -unique-basic-block-section-names -mtriple=i686-pc-win32 -filetype=obj -frame-pointer=all -o - | llvm-readobj -S --sr --codeview - | FileCheck --check-prefix=OBJ32 %s
+; RUN: llc -O0 %s -function-sections -basic-block-sections=all -unique-basic-block-section-names -mtriple=x86_64-pc-win32 -filetype=asm -frame-pointer=all -o - | FileCheck --check-prefix=X64 %s
+; RUN: llc -O0 %s -basic-block-sections=all -unique-basic-block-section-names -mtriple=x86_64-pc-win32 -filetype=obj -frame-pointer=all -o - | llvm-readobj -S --sr --codeview - | FileCheck --check-prefix=OBJ64 %s
+
+;; Generated with below C++ source:
+;; int volatile x;
+;; static inline void bar() {
+;;   ++x;
+;; }
+;; void baz(bool b) {
+;;   if (b)
+;;     bar();
+;; }
+
+; X86:        .section  .text,"xr",one_only,"?baz@@YAX_N@Z"
+; X86:        [[BEGIN_OF_BAZ:.?Lfunc_begin.*]]:
+; X86:        .cv_file 1 "/media/sdb/llvm/basic-block-sections.cpp"
+; X86:        .cv_loc 0 1 5 0
+; X86:        .cv_loc 0 1 6 0
+; X86-LABEL:  Ltmp0:
+; X86:        .section  .text,"xr",associative,"?baz@@YAX_N@Z.__part.1"
+; X86-LABEL:  "?baz@@YAX_N@Z.__part.1":
+; X86:        .section  .text,"xr",associative,"?baz@@YAX_N@Z.__part.2"
+; X86-LABEL:  "?baz@@YAX_N@Z.__part.2":
+; X86:        retl
+; X86:        .section  .text,"xr",one_only,"?baz@@YAX_N@Z"
+; X86:        [[END_OF_BAZ:.?Lfunc_end.*]]:
+
+; Line table
+; X86:        .cv_linetable 0, "?baz@@YAX_N@Z", [[END_OF_BAZ]]
+; X86-NEXT:   .section .debug$S,"dr"
+; File index to string table offset subsection
+; X86:        .cv_filechecksums
+; String table
+; X86-NEXT:   .cv_stringtable
+
+; OBJ32:      Section {
+; OBJ32:        Name: .debug$S
+; OBJ32:        Characteristics [ (0x42300040)
+; OBJ32:        ]
+; OBJ32:      Section {
+; OBJ32:        Name: .debug$S
+; OBJ32:        Characteristics [ (0x42301040)
+; OBJ32:        ]
+; OBJ32:        Relocations [
+; OBJ32:          0xC IMAGE_REL_I386_DIR32NB ?baz@@YAX_N@Z
+; OBJ32:          0x58 IMAGE_REL_I386_SECREL ?baz@@YAX_N@Z
+; OBJ32:          0x5C IMAGE_REL_I386_SECTION ?baz@@YAX_N@Z
+; OBJ32:          0x9C IMAGE_REL_I386_SECREL ?baz@@YAX_N@Z
+; OBJ32:          0xA0 IMAGE_REL_I386_SECTION ?baz@@YAX_N@Z
+; OBJ32:        ]
+; OBJ32:      CodeViewDebugInfo [
+; OBJ32:        Subsection [
+; OBJ32-NEXT:     SubSectionType: Symbols (0xF1)
+; OBJ32:          {{.*}}Proc{{.*}}Sym {
+; OBJ32:            CodeSize: 0x16
+; OBJ32:            DisplayName: baz
+; OBJ32:            LinkageName: ?baz@@YAX_N@Z
+; OBJ32:          }
+; OBJ32:        FunctionLineTable [
+; OBJ32-NEXT:     LinkageName: ?baz@@YAX_N@Z
+; OBJ32-NEXT:     Flags: 0x0
+; OBJ32-NEXT:     CodeSize: 0x16
+; OBJ32-NEXT:     FilenameSegment [
+; OBJ32-NEXT:       Filename: /media/sdb/llvm/basic-block-sections.cpp
+; OBJ32-NEXT:       +0x0 [
+; OBJ32-NEXT:         LineNumberStart: 5
+; OBJ32-NEXT:         LineNumberEndDelta: 0
+; OBJ32-NEXT:         IsStatement: No
+; OBJ32-NEXT:       ]
+; OBJ32-NEXT:       +0x0 [
+; OBJ32-NEXT:         LineNumberStart: 6
+; OBJ32-NEXT:         LineNumberEndDelta: 0
+; OBJ32-NEXT:         IsStatement: No
+; OBJ32-NEXT:       ]
+; OBJ32-NEXT:     ]
+; OBJ32-NEXT:   ]
+; OBJ32-NEXT: ]
+; X64:        .section  .text,"xr",one_only,"?baz@@YAX_N@Z"
+; X64:        [[BEGIN_OF_BAZ:.?Lfunc_begin.*]]:
+; X64:        .cv_file 1 "/media/sdb/llvm/basic-block-sections.cpp"
+; X64:        .cv_loc 0 1 6 0
+; X64:        .section  .text,"xr",associative,"?baz@@YAX_N@Z.__part.1"
+; X64-LABEL:  "?baz@@YAX_N@Z.__part.1":
+; X64:        .section  .text,"xr",associative,"?baz@@YAX_N@Z.__part.2"
+; X64-LABEL:  "?baz@@YAX_N@Z.__part.2":
+; X64:        retq
+; X64:        .section  .text,"xr",one_only,"?baz@@YAX_N@Z"
+; X64:        [[END_OF_BAZ:.?Lfunc_end.*]]:
+
+; Line table
+; X64:        .cv_linetable 0, "?baz@@YAX_N@Z", [[END_OF_BAZ]]
+; X64-NEXT:   .section .debug$S,"dr"
+; File index to string table offset subsection
+; X64:        .cv_filechecksums
+; String table
+; X64-NEXT:   .cv_stringtable
+
+; OBJ64:      Section {
+; OBJ64:        Name: .debug$S
+; OBJ64:        Characteristics [ (0x42300040)
+; OBJ64:        ]
+; OBJ64:        Relocations [
+; OBJ64-NEXT:     0xA0 IMAGE_REL_AMD64_SECREL ?x@@3HC
+; OBJ64-NEXT:     0xA4 IMAGE_REL_AMD64_SECTION ?x@@3HC
+; OBJ64-NEXT:   ]
+; OBJ64:      Section {
+; OBJ64:        Name: .debug$S
+; OBJ64:        Characteristics [ (0x42301040)
+; OBJ64:        ]
+; OBJ64:        Relocations [
+; OBJ64-NEXT:     0x2C IMAGE_REL_AMD64_SECREL ?baz@@YAX_N@Z
+; OBJ64-NEXT:     0x30 IMAGE_REL_AMD64_SECTION ?baz@@YAX_N@Z
+; OBJ64-NEXT:     0x70 IMAGE_REL_AMD64_SECREL ?baz@@YAX_N@Z
+; OBJ64-NEXT:     0x74 IMAGE_REL_AMD64_SECTION ?baz@@YAX_N@Z
+; OBJ64-NEXT:   ]
+; OBJ64:      CodeViewDebugInfo [
+; OBJ64:        Subsection [
+; OBJ64-NEXT:     SubSectionType: Symbols (0xF1)
+; OBJ64:          {{.*}}Proc{{.*}}Sym {
+; OBJ64:            CodeSize: 0x13
+; OBJ64:            DisplayName: baz
+; OBJ64:            LinkageName: ?baz@@YAX_N@Z
+; OBJ64:          }
+; OBJ64:        FunctionLineTable [
+; OBJ64-NEXT:     LinkageName: ?baz@@YAX_N@Z
+; OBJ64-NEXT:     Flags: 0x0
+; OBJ64-NEXT:     CodeSize: 0x13
+; OBJ64-NEXT:     FilenameSegment [
+; OBJ64-NEXT:       Filename: /media/sdb/llvm/basic-block-sections.cpp
+; OBJ64-NEXT:       +0x0 [
+; OBJ64-NEXT:         LineNumberStart: 6
+; OBJ64-NEXT:         LineNumberEndDelta: 0
+; OBJ64-NEXT:         IsStatement: No
+; OBJ64-NEXT:       ]
+; OBJ64-NEXT:     ]
+; OBJ64-NEXT:   ]
+; OBJ64-NEXT: ]
+
+; ModuleID = 'basic-block-sections.cpp'
+source_filename = "basic-block-sections.cpp"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.0.24245"
+
+@"?x@@3HC" = dso_local global i32 0, align 4, !dbg !0
+
+; Function Attrs: nofree norecurse nounwind uwtable willreturn mustprogress
+define dso_local void @"?baz@@YAX_N@Z"(i1 zeroext %0) local_unnamed_addr #0 !dbg !13 {
+  call void @llvm.dbg.value(metadata i1 %0, metadata !18, metadata !DIExpression(DW_OP_LLVM_convert, 1, DW_ATE_unsigned, DW_OP_LLVM_convert, 8, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !19
+  br i1 %0, label %2, label %5, !dbg !20
+
+2:                                                ; preds = %1
+  %3 = load volatile i32, i32* @"?x@@3HC", align 4, !dbg !21, !tbaa !27
+  %4 = add nsw i32 %3, 1, !dbg !21
+  store volatile i32 %4, i32* @"?x@@3HC", align 4, !dbg !21, !tbaa !27
+  br label %5, !dbg !31
+
+5:                                                ; preds = %2, %1
+  ret void, !dbg !32
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nofree norecurse nounwind uwtable willreturn mustprogress "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { nofree nosync nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!8, !9, !10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "x", linkageName: "?x@@3HC", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 13.0.0 (https://github.com/llvm/llvm-project.git 8e0e1773b6442feaef2f1be11b039984f61aa51c)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "basic-block-sections.cpp", directory: "/media/sdb/llvm", checksumkind: CSK_MD5, checksum: "c5a8f2e2d863d9146247c422b76d1ab1")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !7)
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{i32 2, !"CodeView", i32 1}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 2}
+!11 = !{i32 7, !"uwtable", i32 1}
+!12 = !{!"clang version 13.0.0 (https://github.com/llvm/llvm-project.git 8e0e1773b6442feaef2f1be11b039984f61aa51c)"}
+!13 = distinct !DISubprogram(name: "baz", linkageName: "?baz@@YAX_N@Z", scope: !3, file: !3, line: 5, type: !14, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !17)
+!14 = !DISubroutineType(types: !15)
+!15 = !{null, !16}
+!16 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
+!17 = !{!18}
+!18 = !DILocalVariable(name: "b", arg: 1, scope: !13, file: !3, line: 5, type: !16)
+!19 = !DILocation(line: 0, scope: !13)
+!20 = !DILocation(line: 6, scope: !13)
+!21 = !DILocation(line: 3, scope: !22, inlinedAt: !25)
+!22 = distinct !DISubprogram(name: "bar", linkageName: "?bar@@YAXXZ", scope: !3, file: !3, line: 2, type: !23, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !4)
+!23 = !DISubroutineType(types: !24)
+!24 = !{null}
+!25 = distinct !DILocation(line: 7, scope: !26)
+!26 = distinct !DILexicalBlock(scope: !13, file: !3, line: 6)
+!27 = !{!28, !28, i64 0}
+!28 = !{!"int", !29, i64 0}
+!29 = !{!"omnipotent char", !30, i64 0}
+!30 = !{!"Simple C++ TBAA"}
+!31 = !DILocation(line: 7, scope: !26)
+!32 = !DILocation(line: 8, scope: !13)