Skip to content
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

[DWARFVerifier] Fix verification of empty line tables #81162

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

felipepiovezan
Copy link
Contributor

@felipepiovezan felipepiovezan commented Feb 8, 2024

A line table whose sole entry is an end sequence should not have the entry's file index verified, as that value corresponds to the initial value of the state machine, not to a real file index. In DWARF 5, this is particularly problematic as it uses 0-based indexing, and the state machine specifies a starting index of 1; in other words, you'd need to have two files before such index became legal "by default".

A previous attempt to fix this problem was done 1, but it was too specific in its condition, and did not capture all possible cases where this issue can happen.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 8, 2024

@llvm/pr-subscribers-debuginfo

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

A line table whose sole entry is an entry sequence should not have the entry's file index verified, as that value corresponds to the initial value of the state machine, not to a real file index. In DWARF 5, this is particularly problematic as it uses 0-based indexing, and the state machine specifies a starting index of 1; in other words, you'd need to have two files before such index became legal "by default".

A previous attempt to fix this problem was done 1, but it was too specific in its condition, and did not capture all possible cases where this issue can happen.


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

2 Files Affected:

  • (modified) llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp (+6-7)
  • (added) llvm/test/tools/llvm-dwarfdump/X86/verify_empty_debug_line_sequence.yaml (+55)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 2124ff835c5727..b5235765c77b14 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1025,6 +1025,11 @@ void DWARFVerifier::verifyDebugLineRows() {
       FileIndex++;
     }
 
+    // Nothing to verify in a line table with a single row containing the end
+    // sequence.
+    if (LineTable->Rows.size() == 1 && LineTable->Rows.front().EndSequence)
+      continue;
+
     // Verify rows.
     uint64_t PrevAddress = 0;
     uint32_t RowIndex = 0;
@@ -1048,13 +1053,7 @@ void DWARFVerifier::verifyDebugLineRows() {
             });
       }
 
-      // If the prologue contains no file names and the line table has only one
-      // row, do not verify the file index, this is a line table of an empty
-      // file with an end_sequence, but the DWARF standard sets the file number
-      // to 1 by default, otherwise verify file index.
-      if ((LineTable->Prologue.FileNames.size() ||
-           LineTable->Rows.size() != 1) &&
-          !LineTable->hasFileAtIndex(Row.File)) {
+      if (!LineTable->hasFileAtIndex(Row.File)) {
         ++NumDebugLineErrors;
         ErrorCategory.Report("Invalid file index in debug_line", [&]() {
           error() << ".debug_line["
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_empty_debug_line_sequence.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_empty_debug_line_sequence.yaml
new file mode 100644
index 00000000000000..f1a2748115e75b
--- /dev/null
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_empty_debug_line_sequence.yaml
@@ -0,0 +1,55 @@
+# RUN: yaml2obj %s -o %t.o
+# RUN: llvm-dwarfdump -debug-line -verify %t.o | FileCheck %s
+
+# CHECK: Verifying .debug_line...
+# CHECK: No errors
+
+# In a line table like the one below, with no rows (other than the
+# end_sequence), we should never verify the file index because the state
+# machine starts initializes the file index to 1, which is invalid in DWARF 5
+# due to its 0-based indexing.
+
+# file_names[  0]:
+#            name: "/home/umb/tests_2018/106_rnglists2"
+#       dir_index: 0
+# Address            Line   Column File   ISA Discriminator OpIndex Flags
+# ------------------ ------ ------ ------ --- ------------- ------- -------------
+# 0x0000000000000000      1      0      1   0             0       0  is_stmt end_sequence
+
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+DWARF:
+  debug_abbrev:
+    - ID:              0
+      Table:
+        - Code:            0x1
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_stmt_list
+              Form:            DW_FORM_sec_offset
+  debug_info:
+    - Length:          0xd
+      Version:         5
+      UnitType:        DW_UT_compile
+      AbbrevTableID:   0
+      AbbrOffset:      0x0
+      AddrSize:        8
+      Entries:
+        - AbbrCode:        0x1
+          Values:
+            - Value:           0x0
+Sections:
+  - Name:            .debug_line
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+    Content:         300000000500080025000000010101fb0e0d00010101010000000100000101011f010000000002011f020b010000000000000101
+  - Name:            .debug_line_str
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_MERGE, SHF_STRINGS ]
+    AddressAlign:    0x1
+    Content:         2F686F6D652F756D622F74657374735F323031382F3130365F726E676C697374733200746573742E63707000

Copy link
Contributor

@rastogishubham rastogishubham left a comment

Choose a reason for hiding this comment

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

LGTM!

A line table whose sole entry is an end sequence should not have the entry's
file index verified, as that value corresponds to the initial value of the state
machine, not to a real file index. In DWARF 5, this is particularly problematic
as it uses 0-based indexing, and the state machine specifies a starting index of
1; in other words, you'd need to have _two_ files before such index became legal
"by default".

A previous attempt to fix this problem was done [1], but it was too specific in
its condition, and did not capture all possible cases where this issue can
happen.

[1]: llvm#77004
@felipepiovezan
Copy link
Contributor Author

Fixed a bad typo in the commit message: entry sequence -> end sequence

@felipepiovezan felipepiovezan merged commit 1d4fc38 into llvm:main Feb 9, 2024
4 checks passed
@felipepiovezan felipepiovezan deleted the felipe/empty_line_tables branch February 9, 2024 00:48
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request Feb 9, 2024
A line table whose sole entry is an end sequence should not have the
entry's file index verified, as that value corresponds to the initial
value of the state machine, not to a real file index. In DWARF 5, this
is particularly problematic as it uses 0-based indexing, and the state
machine specifies a starting index of 1; in other words, you'd need to
have _two_ files before such index became legal "by default".

A previous attempt to fix this problem was done [1], but it was too
specific in its condition, and did not capture all possible cases where
this issue can happen.

[1]: llvm#77004

(cherry picked from commit 1d4fc38)
felipepiovezan added a commit to apple/llvm-project that referenced this pull request Feb 9, 2024
…-line-tables

[CherryPick][DWARFVerifier] Fix verification of empty line tables (llvm#81162)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants