Skip to content

[MC] Add .loc_label instruction #99710

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.

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

Merged
merged 6 commits into from
Sep 20, 2024
Merged

[MC] Add .loc_label instruction #99710

merged 6 commits into from
Sep 20, 2024

Conversation

alx32
Copy link
Contributor

@alx32 alx32 commented Jul 19, 2024

As discussed in the RFC we need a way to create labels in the assembler-generated line section in order to support the future addition of the DW_AT_LLVM_stmt_sequence attribute.

We have a similar precedent for such behavior with the .cfi_label instruction - so we add the .loc_label THE_LABEL_NAME instruction which:

  • Terminates the current line sequence in the line section
  • Creates a new label with the specified label name in the .debug_line section

@alx32 alx32 marked this pull request as ready for review July 19, 2024 22:42
@llvmbot llvmbot added debuginfo mc Machine (object) code labels Jul 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 19, 2024

@llvm/pr-subscribers-mc

Author: None (alx32)

Changes

As discussed in the RFC we need a way to create labels in the assembler-generated line section in order to support the future addition of the DW_AT_LLVM_stmt_sequence attribute.

We have a similar precedent for such behavior with the .cfi_label instruction - so we add the .loc_label THE_LABEL_NAME instruction which:

  • Terminates the current line sequence in the line section
  • Creates a new label with the specified label name in the .debug_line section

Full diff: https://github.com/llvm/llvm-project/pull/99710.diff

7 Files Affected:

  • (modified) llvm/include/llvm/MC/MCDwarf.h (+8-2)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (+3)
  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (+8)
  • (modified) llvm/lib/MC/MCDwarf.cpp (+17-1)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+19-1)
  • (modified) llvm/lib/MC/MCStreamer.cpp (+19)
  • (added) llvm/test/MC/ELF/debug-loc-label.s (+74)
diff --git a/llvm/include/llvm/MC/MCDwarf.h b/llvm/include/llvm/MC/MCDwarf.h
index d0e45ab59a92e..7ac9c2e023617 100644
--- a/llvm/include/llvm/MC/MCDwarf.h
+++ b/llvm/include/llvm/MC/MCDwarf.h
@@ -194,11 +194,17 @@ class MCDwarfLineEntry : public MCDwarfLoc {
 
 public:
   // Constructor to create an MCDwarfLineEntry given a symbol and the dwarf loc.
-  MCDwarfLineEntry(MCSymbol *label, const MCDwarfLoc loc)
-      : MCDwarfLoc(loc), Label(label) {}
+  MCDwarfLineEntry(MCSymbol *label, const MCDwarfLoc loc,
+                   MCSymbol *lineStreamLabel = nullptr)
+      : MCDwarfLoc(loc), Label(label), LineStreamLabel(lineStreamLabel) {}
 
   MCSymbol *getLabel() const { return Label; }
 
+  // This is the label that is to be emmited into the line stream. If this is
+  // non-null and we need to emit a label, also make sure to restart the current
+  // line sequence.
+  MCSymbol *LineStreamLabel;
+
   // This indicates the line entry is synthesized for an end entry.
   bool IsEndEntry = false;
 
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 78aa12062102c..5fe4534d11d8e 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -912,6 +912,9 @@ class MCStreamer {
                                      unsigned Isa, unsigned Discriminator,
                                      StringRef FileName);
 
+  /// This implements the '.loc_label Name' directive.
+  virtual void emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name);
+
   /// Associate a filename with a specified logical file number, and also
   /// specify that file's checksum information.  This implements the '.cv_file 4
   /// "foo.c"' assembler directive. Returns true on success.
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 45c32f13e759b..89a4aaf47fcd7 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -285,6 +285,8 @@ class MCAsmStreamer final : public MCStreamer {
                              unsigned Flags, unsigned Isa,
                              unsigned Discriminator,
                              StringRef FileName) override;
+  virtual void emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name) override;
+
   MCSymbol *getDwarfLineTableSymbol(unsigned CUID) override;
 
   bool emitCVFileDirective(unsigned FileNo, StringRef Filename,
@@ -1751,6 +1753,12 @@ void MCAsmStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,
                                           Discriminator, FileName);
 }
 
+void MCAsmStreamer::emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name) {
+  MCStreamer::emitDwarfLocLabelDirective(Loc, Name);
+  OS << "\t.loc_label " << Name;
+  EmitEOL();
+}
+
 MCSymbol *MCAsmStreamer::getDwarfLineTableSymbol(unsigned CUID) {
   // Always use the zeroth line table, since asm syntax only supports one line
   // table for now.
diff --git a/llvm/lib/MC/MCDwarf.cpp b/llvm/lib/MC/MCDwarf.cpp
index efafd555c5c5c..4355b7b078e89 100644
--- a/llvm/lib/MC/MCDwarf.cpp
+++ b/llvm/lib/MC/MCDwarf.cpp
@@ -172,6 +172,7 @@ void MCDwarfLineTable::emitOne(
     const MCLineSection::MCDwarfLineEntryCollection &LineEntries) {
 
   unsigned FileNum, LastLine, Column, Flags, Isa, Discriminator;
+  bool IsAtStartSeq;
   MCSymbol *LastLabel;
   auto init = [&]() {
     FileNum = 1;
@@ -181,6 +182,7 @@ void MCDwarfLineTable::emitOne(
     Isa = 0;
     Discriminator = 0;
     LastLabel = nullptr;
+    IsAtStartSeq = true;
   };
   init();
 
@@ -189,11 +191,24 @@ void MCDwarfLineTable::emitOne(
   for (const MCDwarfLineEntry &LineEntry : LineEntries) {
     MCSymbol *Label = LineEntry.getLabel();
     const MCAsmInfo *asmInfo = MCOS->getContext().getAsmInfo();
+
+    if (LineEntry.LineStreamLabel) {
+      if (!IsAtStartSeq) {
+        MCOS->emitDwarfLineEndEntry(Section, LastLabel);
+        init();
+      }
+      MCOS->emitLabel(LineEntry.LineStreamLabel);
+
+      IsAtStartSeq = true;
+      continue;
+    }
+
     if (LineEntry.IsEndEntry) {
       MCOS->emitDwarfAdvanceLineAddr(INT64_MAX, LastLabel, Label,
                                      asmInfo->getCodePointerSize());
       init();
       EndEntryEmitted = true;
+      IsAtStartSeq = true;
       continue;
     }
 
@@ -243,6 +258,7 @@ void MCDwarfLineTable::emitOne(
     Discriminator = 0;
     LastLine = LineEntry.getLine();
     LastLabel = Label;
+    IsAtStartSeq = false;
   }
 
   // Generate DWARF line end entry.
@@ -250,7 +266,7 @@ void MCDwarfLineTable::emitOne(
   // table using ranges whenever CU or section changes. However, the MC path
   // does not track ranges nor terminate the line table. In that case,
   // conservatively use the section end symbol to end the line table.
-  if (!EndEntryEmitted)
+  if (!EndEntryEmitted && !IsAtStartSeq)
     MCOS->emitDwarfLineEndEntry(Section, LastLabel);
 }
 
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index d05712bca73cd..2db7881342ca4 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -485,6 +485,7 @@ class AsmParser : public MCAsmParser {
     DK_FILE,
     DK_LINE,
     DK_LOC,
+    DK_LOC_LABEL,
     DK_STABS,
     DK_CV_FILE,
     DK_CV_FUNC_ID,
@@ -580,10 +581,11 @@ class AsmParser : public MCAsmParser {
   // ".align{,32}", ".p2align{,w,l}"
   bool parseDirectiveAlign(bool IsPow2, unsigned ValueSize);
 
-  // ".file", ".line", ".loc", ".stabs"
+  // ".file", ".line", ".loc", ".loc_label", ".stabs"
   bool parseDirectiveFile(SMLoc DirectiveLoc);
   bool parseDirectiveLine();
   bool parseDirectiveLoc();
+  bool parseDirectiveLocLabel(SMLoc DirectiveLoc);
   bool parseDirectiveStabs();
 
   // ".cv_file", ".cv_func_id", ".cv_inline_site_id", ".cv_loc", ".cv_linetable",
@@ -2156,6 +2158,8 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info,
       return parseDirectiveLine();
     case DK_LOC:
       return parseDirectiveLoc();
+    case DK_LOC_LABEL:
+      return parseDirectiveLocLabel(IDLoc);
     case DK_STABS:
       return parseDirectiveStabs();
     case DK_CV_FILE:
@@ -3737,6 +3741,19 @@ bool AsmParser::parseDirectiveLoc() {
   return false;
 }
 
+/// parseDirectiveLoc
+/// ::= .loc_label label
+bool AsmParser::parseDirectiveLocLabel(SMLoc DirectiveLoc) {
+  StringRef Name;
+  DirectiveLoc = Lexer.getLoc();
+  if (parseIdentifier(Name))
+    return TokError("expected identifier");
+  if (parseEOL())
+    return true;
+  getStreamer().emitDwarfLocLabelDirective(DirectiveLoc, Name);
+  return false;
+}
+
 /// parseDirectiveStabs
 /// ::= .stabs string, number, number, number
 bool AsmParser::parseDirectiveStabs() {
@@ -5549,6 +5566,7 @@ void AsmParser::initializeDirectiveKindMap() {
   DirectiveKindMap[".file"] = DK_FILE;
   DirectiveKindMap[".line"] = DK_LINE;
   DirectiveKindMap[".loc"] = DK_LOC;
+  DirectiveKindMap[".loc_label"] = DK_LOC_LABEL;
   DirectiveKindMap[".stabs"] = DK_STABS;
   DirectiveKindMap[".cv_file"] = DK_CV_FILE;
   DirectiveKindMap[".cv_func_id"] = DK_CV_FUNC_ID;
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index 1594bd3231abe..d5f323129c4dd 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -267,6 +267,25 @@ void MCStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,
                                   Discriminator);
 }
 
+void MCStreamer::emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name) {
+  auto &ctx = getContext();
+
+  auto *LineStreamLabel = ctx.getOrCreateSymbol(Name);
+
+  auto *LineSym = ctx.createTempSymbol();
+  const MCDwarfLoc &DwarfLoc = ctx.getCurrentDwarfLoc();
+
+  // Create a 'fake' line entry by having LineStreamLabel be non-null. This
+  // won't actually emit any line information, it will reset the line table
+  // sequence and emit a label at the start of the new line table sequence.
+  MCDwarfLineEntry LineEntry(LineSym, DwarfLoc, LineStreamLabel);
+
+  // Add the line entry to this section's entries.
+  ctx.getMCDwarfLineTable(ctx.getDwarfCompileUnitID())
+      .getMCLineSections()
+      .addLineEntry(LineEntry, getCurrentSectionOnly());
+}
+
 MCSymbol *MCStreamer::getDwarfLineTableSymbol(unsigned CUID) {
   MCDwarfLineTable &Table = getContext().getMCDwarfLineTable(CUID);
   if (!Table.getLabel()) {
diff --git a/llvm/test/MC/ELF/debug-loc-label.s b/llvm/test/MC/ELF/debug-loc-label.s
new file mode 100644
index 0000000000000..dabf470d417cc
--- /dev/null
+++ b/llvm/test/MC/ELF/debug-loc-label.s
@@ -0,0 +1,74 @@
+// Verify that the .loc_label instruction resets the line sequence and generates
+// the requested label at the correct position in the line stream
+
+// RUN: llvm-mc -filetype obj -triple x86_64-linux-elf %s -o %t.o
+// RUN: llvm-objdump -d %t.o | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: llvm-dwarfdump -v --debug-line %t.o | FileCheck %s --check-prefix=CHECK-LINE-TABLE
+
+# CHECK-ASM:        <foo>:
+# CHECK-ASM-NEXT:     movq    %rdx, 0x33
+# CHECK-ASM-NEXT:     movq    %rax, 0x3b
+# CHECK-ASM-NEXT:     movq    %rbx, 0x4e
+# CHECK-ASM-NEXT:     movq    %rcx, 0x60
+# CHECK-ASM-NEXT:     retq
+
+# CHECK-LINE-TABLE:                  Address            Line   Column File   ISA Discriminator OpIndex Flags
+# CHECK-LINE-TABLE-NEXT:             ------------------ ------ ------ ------ --- ------------- ------- -------------
+# CHECK-LINE-TABLE-NEXT: 0x00000028: 05 DW_LNS_set_column (1)
+# CHECK-LINE-TABLE-NEXT: 0x0000002a: 00 DW_LNE_set_address (0x0000000000000000)
+# CHECK-LINE-TABLE-NEXT: 0x00000035: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000000      1      1      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x00000036: 02 DW_LNS_advance_pc (addr += 33, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x00000038: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      1      1   0             0       0  is_stmt end_sequence
+# CHECK-LINE-TABLE-NEXT: 0x0000003b: 05 DW_LNS_set_column (2)
+# CHECK-LINE-TABLE-NEXT: 0x0000003d: 00 DW_LNE_set_address (0x0000000000000008)
+# CHECK-LINE-TABLE-NEXT: 0x00000048: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000008      1      2      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x00000049: 02 DW_LNS_advance_pc (addr += 25, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x0000004b: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      2      1   0             0       0  is_stmt end_sequence
+# CHECK-LINE-TABLE-NEXT: 0x0000004e: 05 DW_LNS_set_column (3)
+# CHECK-LINE-TABLE-NEXT: 0x00000050: 00 DW_LNE_set_address (0x0000000000000010)
+# CHECK-LINE-TABLE-NEXT: 0x0000005b: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000010      1      3      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x0000005c: 08 DW_LNS_const_add_pc (addr += 0x0000000000000011, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x0000005d: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      3      1   0             0       0  is_stmt end_sequence
+# CHECK-LINE-TABLE-NEXT: 0x00000060: 05 DW_LNS_set_column (4)
+# CHECK-LINE-TABLE-NEXT: 0x00000062: 00 DW_LNE_set_address (0x0000000000000018)
+# CHECK-LINE-TABLE-NEXT: 0x0000006d: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000018      1      4      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x0000006e: 05 DW_LNS_set_column (5)
+# CHECK-LINE-TABLE-NEXT: 0x00000070: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000018      1      5      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x00000071: 02 DW_LNS_advance_pc (addr += 9, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x00000073: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      5      1   0             0       0  is_stmt end_sequence
+	.text
+	.file	"test.c"
+	.globl	foo
+	.align	16, 0x90
+	.type	foo,@function
+foo:
+.Lfunc_begin0:
+	.file	1 "test.c"
+	.cfi_startproc
+	.loc	1 1 1
+	mov     %rdx, 0x33
+	.loc_label my_label_02
+	.loc	1 1 2
+  movq    %rax, my_label_02-.Lline_table_start0
+	.loc	1 1 3
+	.loc_label my_label_03
+	.loc_label my_label_03.1
+  movq    %rbx, my_label_03-.Lline_table_start0
+	.loc	1 1 4
+	.loc_label my_label_05
+	.loc	1 1 5
+  movq    %rcx, my_label_05-.Lline_table_start0
+	ret
+	.cfi_endproc
+
+	.section	.debug_line,"",@progbits
+.Lline_table_start0:

@llvmbot
Copy link
Member

llvmbot commented Jul 19, 2024

@llvm/pr-subscribers-debuginfo

Author: None (alx32)

Changes

As discussed in the RFC we need a way to create labels in the assembler-generated line section in order to support the future addition of the DW_AT_LLVM_stmt_sequence attribute.

We have a similar precedent for such behavior with the .cfi_label instruction - so we add the .loc_label THE_LABEL_NAME instruction which:

  • Terminates the current line sequence in the line section
  • Creates a new label with the specified label name in the .debug_line section

Full diff: https://github.com/llvm/llvm-project/pull/99710.diff

7 Files Affected:

  • (modified) llvm/include/llvm/MC/MCDwarf.h (+8-2)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (+3)
  • (modified) llvm/lib/MC/MCAsmStreamer.cpp (+8)
  • (modified) llvm/lib/MC/MCDwarf.cpp (+17-1)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+19-1)
  • (modified) llvm/lib/MC/MCStreamer.cpp (+19)
  • (added) llvm/test/MC/ELF/debug-loc-label.s (+74)
diff --git a/llvm/include/llvm/MC/MCDwarf.h b/llvm/include/llvm/MC/MCDwarf.h
index d0e45ab59a92e..7ac9c2e023617 100644
--- a/llvm/include/llvm/MC/MCDwarf.h
+++ b/llvm/include/llvm/MC/MCDwarf.h
@@ -194,11 +194,17 @@ class MCDwarfLineEntry : public MCDwarfLoc {
 
 public:
   // Constructor to create an MCDwarfLineEntry given a symbol and the dwarf loc.
-  MCDwarfLineEntry(MCSymbol *label, const MCDwarfLoc loc)
-      : MCDwarfLoc(loc), Label(label) {}
+  MCDwarfLineEntry(MCSymbol *label, const MCDwarfLoc loc,
+                   MCSymbol *lineStreamLabel = nullptr)
+      : MCDwarfLoc(loc), Label(label), LineStreamLabel(lineStreamLabel) {}
 
   MCSymbol *getLabel() const { return Label; }
 
+  // This is the label that is to be emmited into the line stream. If this is
+  // non-null and we need to emit a label, also make sure to restart the current
+  // line sequence.
+  MCSymbol *LineStreamLabel;
+
   // This indicates the line entry is synthesized for an end entry.
   bool IsEndEntry = false;
 
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 78aa12062102c..5fe4534d11d8e 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -912,6 +912,9 @@ class MCStreamer {
                                      unsigned Isa, unsigned Discriminator,
                                      StringRef FileName);
 
+  /// This implements the '.loc_label Name' directive.
+  virtual void emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name);
+
   /// Associate a filename with a specified logical file number, and also
   /// specify that file's checksum information.  This implements the '.cv_file 4
   /// "foo.c"' assembler directive. Returns true on success.
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 45c32f13e759b..89a4aaf47fcd7 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -285,6 +285,8 @@ class MCAsmStreamer final : public MCStreamer {
                              unsigned Flags, unsigned Isa,
                              unsigned Discriminator,
                              StringRef FileName) override;
+  virtual void emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name) override;
+
   MCSymbol *getDwarfLineTableSymbol(unsigned CUID) override;
 
   bool emitCVFileDirective(unsigned FileNo, StringRef Filename,
@@ -1751,6 +1753,12 @@ void MCAsmStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,
                                           Discriminator, FileName);
 }
 
+void MCAsmStreamer::emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name) {
+  MCStreamer::emitDwarfLocLabelDirective(Loc, Name);
+  OS << "\t.loc_label " << Name;
+  EmitEOL();
+}
+
 MCSymbol *MCAsmStreamer::getDwarfLineTableSymbol(unsigned CUID) {
   // Always use the zeroth line table, since asm syntax only supports one line
   // table for now.
diff --git a/llvm/lib/MC/MCDwarf.cpp b/llvm/lib/MC/MCDwarf.cpp
index efafd555c5c5c..4355b7b078e89 100644
--- a/llvm/lib/MC/MCDwarf.cpp
+++ b/llvm/lib/MC/MCDwarf.cpp
@@ -172,6 +172,7 @@ void MCDwarfLineTable::emitOne(
     const MCLineSection::MCDwarfLineEntryCollection &LineEntries) {
 
   unsigned FileNum, LastLine, Column, Flags, Isa, Discriminator;
+  bool IsAtStartSeq;
   MCSymbol *LastLabel;
   auto init = [&]() {
     FileNum = 1;
@@ -181,6 +182,7 @@ void MCDwarfLineTable::emitOne(
     Isa = 0;
     Discriminator = 0;
     LastLabel = nullptr;
+    IsAtStartSeq = true;
   };
   init();
 
@@ -189,11 +191,24 @@ void MCDwarfLineTable::emitOne(
   for (const MCDwarfLineEntry &LineEntry : LineEntries) {
     MCSymbol *Label = LineEntry.getLabel();
     const MCAsmInfo *asmInfo = MCOS->getContext().getAsmInfo();
+
+    if (LineEntry.LineStreamLabel) {
+      if (!IsAtStartSeq) {
+        MCOS->emitDwarfLineEndEntry(Section, LastLabel);
+        init();
+      }
+      MCOS->emitLabel(LineEntry.LineStreamLabel);
+
+      IsAtStartSeq = true;
+      continue;
+    }
+
     if (LineEntry.IsEndEntry) {
       MCOS->emitDwarfAdvanceLineAddr(INT64_MAX, LastLabel, Label,
                                      asmInfo->getCodePointerSize());
       init();
       EndEntryEmitted = true;
+      IsAtStartSeq = true;
       continue;
     }
 
@@ -243,6 +258,7 @@ void MCDwarfLineTable::emitOne(
     Discriminator = 0;
     LastLine = LineEntry.getLine();
     LastLabel = Label;
+    IsAtStartSeq = false;
   }
 
   // Generate DWARF line end entry.
@@ -250,7 +266,7 @@ void MCDwarfLineTable::emitOne(
   // table using ranges whenever CU or section changes. However, the MC path
   // does not track ranges nor terminate the line table. In that case,
   // conservatively use the section end symbol to end the line table.
-  if (!EndEntryEmitted)
+  if (!EndEntryEmitted && !IsAtStartSeq)
     MCOS->emitDwarfLineEndEntry(Section, LastLabel);
 }
 
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index d05712bca73cd..2db7881342ca4 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -485,6 +485,7 @@ class AsmParser : public MCAsmParser {
     DK_FILE,
     DK_LINE,
     DK_LOC,
+    DK_LOC_LABEL,
     DK_STABS,
     DK_CV_FILE,
     DK_CV_FUNC_ID,
@@ -580,10 +581,11 @@ class AsmParser : public MCAsmParser {
   // ".align{,32}", ".p2align{,w,l}"
   bool parseDirectiveAlign(bool IsPow2, unsigned ValueSize);
 
-  // ".file", ".line", ".loc", ".stabs"
+  // ".file", ".line", ".loc", ".loc_label", ".stabs"
   bool parseDirectiveFile(SMLoc DirectiveLoc);
   bool parseDirectiveLine();
   bool parseDirectiveLoc();
+  bool parseDirectiveLocLabel(SMLoc DirectiveLoc);
   bool parseDirectiveStabs();
 
   // ".cv_file", ".cv_func_id", ".cv_inline_site_id", ".cv_loc", ".cv_linetable",
@@ -2156,6 +2158,8 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info,
       return parseDirectiveLine();
     case DK_LOC:
       return parseDirectiveLoc();
+    case DK_LOC_LABEL:
+      return parseDirectiveLocLabel(IDLoc);
     case DK_STABS:
       return parseDirectiveStabs();
     case DK_CV_FILE:
@@ -3737,6 +3741,19 @@ bool AsmParser::parseDirectiveLoc() {
   return false;
 }
 
+/// parseDirectiveLoc
+/// ::= .loc_label label
+bool AsmParser::parseDirectiveLocLabel(SMLoc DirectiveLoc) {
+  StringRef Name;
+  DirectiveLoc = Lexer.getLoc();
+  if (parseIdentifier(Name))
+    return TokError("expected identifier");
+  if (parseEOL())
+    return true;
+  getStreamer().emitDwarfLocLabelDirective(DirectiveLoc, Name);
+  return false;
+}
+
 /// parseDirectiveStabs
 /// ::= .stabs string, number, number, number
 bool AsmParser::parseDirectiveStabs() {
@@ -5549,6 +5566,7 @@ void AsmParser::initializeDirectiveKindMap() {
   DirectiveKindMap[".file"] = DK_FILE;
   DirectiveKindMap[".line"] = DK_LINE;
   DirectiveKindMap[".loc"] = DK_LOC;
+  DirectiveKindMap[".loc_label"] = DK_LOC_LABEL;
   DirectiveKindMap[".stabs"] = DK_STABS;
   DirectiveKindMap[".cv_file"] = DK_CV_FILE;
   DirectiveKindMap[".cv_func_id"] = DK_CV_FUNC_ID;
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index 1594bd3231abe..d5f323129c4dd 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -267,6 +267,25 @@ void MCStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,
                                   Discriminator);
 }
 
+void MCStreamer::emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name) {
+  auto &ctx = getContext();
+
+  auto *LineStreamLabel = ctx.getOrCreateSymbol(Name);
+
+  auto *LineSym = ctx.createTempSymbol();
+  const MCDwarfLoc &DwarfLoc = ctx.getCurrentDwarfLoc();
+
+  // Create a 'fake' line entry by having LineStreamLabel be non-null. This
+  // won't actually emit any line information, it will reset the line table
+  // sequence and emit a label at the start of the new line table sequence.
+  MCDwarfLineEntry LineEntry(LineSym, DwarfLoc, LineStreamLabel);
+
+  // Add the line entry to this section's entries.
+  ctx.getMCDwarfLineTable(ctx.getDwarfCompileUnitID())
+      .getMCLineSections()
+      .addLineEntry(LineEntry, getCurrentSectionOnly());
+}
+
 MCSymbol *MCStreamer::getDwarfLineTableSymbol(unsigned CUID) {
   MCDwarfLineTable &Table = getContext().getMCDwarfLineTable(CUID);
   if (!Table.getLabel()) {
diff --git a/llvm/test/MC/ELF/debug-loc-label.s b/llvm/test/MC/ELF/debug-loc-label.s
new file mode 100644
index 0000000000000..dabf470d417cc
--- /dev/null
+++ b/llvm/test/MC/ELF/debug-loc-label.s
@@ -0,0 +1,74 @@
+// Verify that the .loc_label instruction resets the line sequence and generates
+// the requested label at the correct position in the line stream
+
+// RUN: llvm-mc -filetype obj -triple x86_64-linux-elf %s -o %t.o
+// RUN: llvm-objdump -d %t.o | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: llvm-dwarfdump -v --debug-line %t.o | FileCheck %s --check-prefix=CHECK-LINE-TABLE
+
+# CHECK-ASM:        <foo>:
+# CHECK-ASM-NEXT:     movq    %rdx, 0x33
+# CHECK-ASM-NEXT:     movq    %rax, 0x3b
+# CHECK-ASM-NEXT:     movq    %rbx, 0x4e
+# CHECK-ASM-NEXT:     movq    %rcx, 0x60
+# CHECK-ASM-NEXT:     retq
+
+# CHECK-LINE-TABLE:                  Address            Line   Column File   ISA Discriminator OpIndex Flags
+# CHECK-LINE-TABLE-NEXT:             ------------------ ------ ------ ------ --- ------------- ------- -------------
+# CHECK-LINE-TABLE-NEXT: 0x00000028: 05 DW_LNS_set_column (1)
+# CHECK-LINE-TABLE-NEXT: 0x0000002a: 00 DW_LNE_set_address (0x0000000000000000)
+# CHECK-LINE-TABLE-NEXT: 0x00000035: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000000      1      1      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x00000036: 02 DW_LNS_advance_pc (addr += 33, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x00000038: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      1      1   0             0       0  is_stmt end_sequence
+# CHECK-LINE-TABLE-NEXT: 0x0000003b: 05 DW_LNS_set_column (2)
+# CHECK-LINE-TABLE-NEXT: 0x0000003d: 00 DW_LNE_set_address (0x0000000000000008)
+# CHECK-LINE-TABLE-NEXT: 0x00000048: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000008      1      2      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x00000049: 02 DW_LNS_advance_pc (addr += 25, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x0000004b: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      2      1   0             0       0  is_stmt end_sequence
+# CHECK-LINE-TABLE-NEXT: 0x0000004e: 05 DW_LNS_set_column (3)
+# CHECK-LINE-TABLE-NEXT: 0x00000050: 00 DW_LNE_set_address (0x0000000000000010)
+# CHECK-LINE-TABLE-NEXT: 0x0000005b: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000010      1      3      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x0000005c: 08 DW_LNS_const_add_pc (addr += 0x0000000000000011, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x0000005d: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      3      1   0             0       0  is_stmt end_sequence
+# CHECK-LINE-TABLE-NEXT: 0x00000060: 05 DW_LNS_set_column (4)
+# CHECK-LINE-TABLE-NEXT: 0x00000062: 00 DW_LNE_set_address (0x0000000000000018)
+# CHECK-LINE-TABLE-NEXT: 0x0000006d: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000018      1      4      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x0000006e: 05 DW_LNS_set_column (5)
+# CHECK-LINE-TABLE-NEXT: 0x00000070: 01 DW_LNS_copy
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000018      1      5      1   0             0       0  is_stmt
+# CHECK-LINE-TABLE-NEXT: 0x00000071: 02 DW_LNS_advance_pc (addr += 9, op-index += 0)
+# CHECK-LINE-TABLE-NEXT: 0x00000073: 00 DW_LNE_end_sequence
+# CHECK-LINE-TABLE-NEXT:             0x0000000000000021      1      5      1   0             0       0  is_stmt end_sequence
+	.text
+	.file	"test.c"
+	.globl	foo
+	.align	16, 0x90
+	.type	foo,@function
+foo:
+.Lfunc_begin0:
+	.file	1 "test.c"
+	.cfi_startproc
+	.loc	1 1 1
+	mov     %rdx, 0x33
+	.loc_label my_label_02
+	.loc	1 1 2
+  movq    %rax, my_label_02-.Lline_table_start0
+	.loc	1 1 3
+	.loc_label my_label_03
+	.loc_label my_label_03.1
+  movq    %rbx, my_label_03-.Lline_table_start0
+	.loc	1 1 4
+	.loc_label my_label_05
+	.loc	1 1 5
+  movq    %rcx, my_label_05-.Lline_table_start0
+	ret
+	.cfi_endproc
+
+	.section	.debug_line,"",@progbits
+.Lline_table_start0:

Copy link
Collaborator

@pogo59 pogo59 left a comment

Choose a reason for hiding this comment

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

Thanks for handling the repeated directive case!

Looks like the test does not check that the line-table labels are emitted in the right places. I'm not 100% sure how to do that, but probably dumping .symtab would show them?

@alx32
Copy link
Contributor Author

alx32 commented Jul 22, 2024

the test does not check that the line-table labels are emitted in the right places

I wanted to test them as they would be used in the future - via relative offsets to the line table start. So for example for my_label_02 which should point to column 2: (.loc 1 1 2) we have in the test:

  movq    %rax, my_label_02-.Lline_table_start0

And then the tests checks that this gets assembled to:

# CHECK-ASM-NEXT:     movq    %rax, 0x3b

And the test also checks that offset 0x3b in the line table is indeed column 2:

# CHECK-LINE-TABLE-NEXT: 0x0000003b: 05 DW_LNS_set_column (2)

I can add additional tests using llvm-objdump --syms but I think they would be checking the same thing again. Or should we add this test to make sure the syms end up in the symtab ?

Note, in the future the offsets would be used with DW_AT_LLVM_stmt_sequence but as this doesn't exist yet I used them with movq instructions instead.

@dwblaikie
Copy link
Collaborator

You could emit the label difference into some other attribute in assembly rather than in an instruction (guess that's not necessarily better - it'll necessarily be a bogus attribute/value combination I guess) - or I guess just into its own section and dump the bytes of the section raw...

Guess you could use an unknown/bogus attribute number, I think it'd still dump OK & we'd just print a bogus attribute number in the attribute name somehow... - mixed feelings about what the best way to do that is.

Copy link
Contributor Author

@alx32 alx32 left a comment

Choose a reason for hiding this comment

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

Moved the offsets to their own sections in the test - I think this is a better version than parameters to 'movq' instructions.

@alx32 alx32 requested review from MaskRay and pogo59 August 7, 2024 21:35
@alx32
Copy link
Contributor Author

alx32 commented Aug 9, 2024

The latest update should address all issues - would you have another look please, @pogo59, @MaskRay, @dwblaikie. Thanks!

@alx32
Copy link
Contributor Author

alx32 commented Aug 14, 2024

Would be great if we could get this reviewed soon in order to proceed with adding the same functionality to clang.
cc: @pogo59, @MaskRay, @dwblaikie.

@alx32 alx32 requested a review from kyulee-com August 16, 2024 00:55
@@ -1767,6 +1770,12 @@ void MCAsmStreamer::emitDwarfLocDirective(unsigned FileNo, unsigned Line,
Discriminator, FileName);
}

void MCAsmStreamer::emitDwarfLocLabelDirective(SMLoc Loc, StringRef Name) {
MCStreamer::emitDwarfLocLabelDirective(Loc, Name);
OS << "\t.loc_label " << Name;
Copy link
Member

Choose a reason for hiding this comment

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

".loc_label\t"

}

// Generate DWARF line end entry.
// We do not need this for DwarfDebug that explicitly terminates the line
// table using ranges whenever CU or section changes. However, the MC path
// does not track ranges nor terminate the line table. In that case,
// conservatively use the section end symbol to end the line table.
if (!EndEntryEmitted)
if (!EndEntryEmitted && !IsAtStartSeq)
Copy link
Member

Choose a reason for hiding this comment

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

Missing test coverage: the test doesn't fail if I remove && !IsAtStartSeq

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added coverage.

@MaskRay
Copy link
Member

MaskRay commented Aug 19, 2024

You could emit the label difference into some other attribute in assembly rather than in an instruction (guess that's not necessarily better - it'll necessarily be a bogus attribute/value combination I guess) - or I guess just into its own section and dump the bytes of the section raw...

Guess you could use an unknown/bogus attribute number, I think it'd still dump OK & we'd just print a bogus attribute number in the attribute name somehow... - mixed feelings about what the best way to do that is.

It's usually better to inspect the symbol table with llvm-readelf -sX to know other properties (e.g. binding/type, even if they are the default, STB_LOCAL, STT_NOTYPE). A label difference could lead to both relocations and a content change.

@alx32
Copy link
Contributor Author

alx32 commented Aug 28, 2024

@MaskRay I think current version addresses all the comments - could you have another look ?

@alx32
Copy link
Contributor Author

alx32 commented Sep 3, 2024

@MaskRay - Could you have another look - trying to finish this to proceed with the clang side of this.

@alx32 alx32 requested review from aaupov and ayermolo September 9, 2024 16:11
@kyulee-com kyulee-com requested a review from MatzeB September 18, 2024 16:53
@alx32
Copy link
Contributor Author

alx32 commented Sep 19, 2024

@pogo59, @dwblaikie - Could you have a look ?

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

This looks good from the MC perspective, but you need some stamps from debug-info folks.

// Verify that the .loc_label instruction resets the line sequence and generates
// the requested label at the correct position in the line stream

// RUN: llvm-mc -filetype obj -triple x86_64-linux-elf %s -o %t.o
Copy link
Member

Choose a reason for hiding this comment

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

Just -triple x86_64 to make it clear that this is generic ELF behavior, not specific to Linux.

linux-elf is kinda incorrect triple. When the os is linux, "elf" should not be used.

Copy link
Collaborator

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

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

Looks good to me too

@alx32 alx32 merged commit 28646d0 into llvm:main Sep 20, 2024
8 checks passed
alx32 added a commit that referenced this pull request Nov 14, 2024
…nces (#110192)

**Summary**

This patch introduces a new compiler option `-mllvm
-emit-func-debug-line-table-offsets` that enables the emission of
per-function line table offsets and end sequences in DWARF debug
information. This enhancement allows tools and debuggers to accurately
attribute line number information to their corresponding functions, even
in scenarios where functions are merged or share the same address space
due to optimizations like Identical Code Folding (ICF) in the linker.

**Background**
RFC: [New DWARF Attribute for Symbolication of Merged
Functions](https://discourse.llvm.org/t/rfc-new-dwarf-attribute-for-symbolication-of-merged-functions/79434)

Previous similar PR:
[#93137](#93137) – This PR was
very similar to the current one but at the time, the assembler had no
support for emitting labels within the line table. That support was
added in PR [#99710](#99710) -
and in this PR we use some of the support added in the assembler PR.

In the current implementation, Clang generates line information in the
`debug_line` section without directly associating line entries with
their originating `DW_TAG_subprogram` DIEs. This can lead to issues when
post-compilation optimizations merge functions, resulting in overlapping
address ranges and ambiguous line information.

For example, when functions are merged by ICF in LLD, multiple functions
may end up sharing the same address range. Without explicit linkage
between functions and their line entries, tools cannot accurately
attribute line information to the correct function, adversely affecting
debugging and call stack resolution.


**Implementation Details**
To address the above issue, the patch makes the following key changes:

**`DW_AT_LLVM_stmt_sequence` Attribute**: Introduces a new LLVM-specific
attribute `DW_AT_LLVM_stmt_sequence` to each `DW_TAG_subprogram` DIE.
This attribute holds a label pointing to the offset in the line table
where the function's line entries begin.

**End-of-Sequence Markers**: Emits an explicit DW_LNE_end_sequence after
each function's line entries in the line table. This marks the end of
the line information for that function, ensuring that line entries are
correctly delimited.

**Assembler and Streamer Modifications**: Modifies the MCStreamer and
related classes to support emitting the necessary labels and tracking
the current function's line entries. A new flag
GenerateFuncLineTableOffsets is added to control this behavior.

**Compiler Option**: Introduces the `-mllvm
-emit-func-debug-line-table-offsets` option to enable this
functionality, allowing users to opt-in as needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debuginfo mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants