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

Fix file index verifier when there is no file name in the prologue. #77004

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

rastogishubham
Copy link
Contributor

If there is no file name in the prologue of a line table, the verifier will try to verify the file index, which will be set to 1 by default. This will cause the DWARF verifier to throw an error even if there is no error.

rdar://114476503
rdar://114343624

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 4, 2024

@llvm/pr-subscribers-debuginfo

Author: Shubham Sandeep Rastogi (rastogishubham)

Changes

If there is no file name in the prologue of a line table, the verifier will try to verify the file index, which will be set to 1 by default. This will cause the DWARF verifier to throw an error even if there is no error.

rdar://114476503
rdar://114343624


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

2 Files Affected:

  • (modified) llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp (+7-2)
  • (added) llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml (+356)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 43ed60d7f9775d..18377815c78b3e 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -940,8 +940,13 @@ void DWARFVerifier::verifyDebugLineRows() {
         OS << '\n';
       }
 
-      // Verify file index.
-      if (!LineTable->hasFileAtIndex(Row.File)) {
+      // 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() == 0 &&
+            LineTable->Rows.size() == 1) &&
+          !LineTable->hasFileAtIndex(Row.File)) {
         ++NumDebugLineErrors;
         error() << ".debug_line["
                 << format("0x%08" PRIx64,
diff --git a/llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml b/llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml
new file mode 100644
index 00000000000000..e43bb880821623
--- /dev/null
+++ b/llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml
@@ -0,0 +1,356 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: yaml2obj %s -o %t/test.o
+# RUN: llvm-dwarfdump --debug-line --verify %t/test.o 2>&1 | FileCheck %s
+
+# CHECK-NOT: error: .debug_line[0x{{[0-9a-f]+}}][0] has invalid file index 1 (valid values are [1,0]):
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x100000C
+  cpusubtype:      0x0
+  filetype:        0x1
+  ncmds:           4
+  sizeofcmds:      1000
+  flags:           0x2000
+  reserved:        0x0
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         872
+    segname:         ''
+    vmaddr:          0
+    vmsize:          635
+    fileoff:         1032
+    filesize:        635
+    maxprot:         7
+    initprot:        7
+    nsects:          10
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0
+        size:            4
+        offset:          0x408
+        align:           2
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         C0035FD6
+      - sectname:        __debug_abbrev
+        segname:         __DWARF
+        addr:            0x4
+        size:            50
+        offset:          0x40C
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+      - sectname:        __debug_info
+        segname:         __DWARF
+        addr:            0x36
+        size:            76
+        offset:          0x43E
+        align:           0
+        reloff:          0x688
+        nreloc:          2
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        relocations:
+          - address:         0x33
+            symbolnum:       1
+            pcrel:           false
+            length:          3
+            extern:          false
+            type:            0
+            scattered:       false
+            value:           0
+          - address:         0x26
+            symbolnum:       1
+            pcrel:           false
+            length:          3
+            extern:          false
+            type:            0
+            scattered:       false
+            value:           0
+      - sectname:        __debug_str
+        segname:         __DWARF
+        addr:            0x82
+        size:            196
+        offset:          0x48A
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+      - sectname:        __apple_names
+        segname:         __DWARF
+        addr:            0x146
+        size:            88
+        offset:          0x54E
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         485341480100000002000000020000000C000000000000000100000001000600FFFFFFFF000000008973880BEB28616A3800000048000000B8000000010000003200000000000000BC000000010000003200000000000000
+      - sectname:        __apple_objc
+        segname:         __DWARF
+        addr:            0x19E
+        size:            36
+        offset:          0x5A6
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         485341480100000001000000000000000C000000000000000100000001000600FFFFFFFF
+      - sectname:        __apple_namespac
+        segname:         __DWARF
+        addr:            0x1C2
+        size:            36
+        offset:          0x5CA
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         485341480100000001000000000000000C000000000000000100000001000600FFFFFFFF
+      - sectname:        __apple_types
+        segname:         __DWARF
+        addr:            0x1E6
+        size:            44
+        offset:          0x5EE
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         48534148010000000100000000000000140000000000000003000000010006000300050004000B00FFFFFFFF
+      - sectname:        __compact_unwind
+        segname:         __LD
+        addr:            0x218
+        size:            32
+        offset:          0x620
+        align:           3
+        reloff:          0x698
+        nreloc:          1
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         '0000000000000000040000000000000200000000000000000000000000000000'
+        relocations:
+          - address:         0x0
+            symbolnum:       1
+            pcrel:           false
+            length:          3
+            extern:          false
+            type:            0
+            scattered:       false
+            value:           0
+      - sectname:        __debug_line
+        segname:         __DWARF
+        addr:            0x238
+        size:            67
+        offset:          0x640
+        align:           0
+        reloff:          0x6A0
+        nreloc:          1
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        relocations:
+          - address:         0x35
+            symbolnum:       1
+            pcrel:           false
+            length:          3
+            extern:          false
+            type:            0
+            scattered:       false
+            value:           0
+  - cmd:             LC_BUILD_VERSION
+    cmdsize:         24
+    platform:        1
+    minos:           917504
+    sdk:             918528
+    ntools:          0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          1704
+    nsyms:           3
+    stroff:          1752
+    strsize:         24
+  - cmd:             LC_DYSYMTAB
+    cmdsize:         80
+    ilocalsym:       0
+    nlocalsym:       2
+    iextdefsym:      2
+    nextdefsym:      1
+    iundefsym:       3
+    nundefsym:       0
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
+    nmodtab:         0
+    extrefsymoff:    0
+    nextrefsyms:     0
+    indirectsymoff:  0
+    nindirectsyms:   0
+    extreloff:       0
+    nextrel:         0
+    locreloff:       0
+    nlocrel:         0
+LinkEditData:
+  NameList:
+    - n_strx:          16
+      n_type:          0xE
+      n_sect:          1
+      n_desc:          0
+      n_value:         0
+    - n_strx:          10
+      n_type:          0xE
+      n_sect:          9
+      n_desc:          0
+      n_value:         536
+    - n_strx:          1
+      n_type:          0xF
+      n_sect:          1
+      n_desc:          0
+      n_value:         0
+  StringTable:
+    - ''
+    - __Z3foov
+    - ltmp1
+    - ltmp0
+    - ''
+    - ''
+DWARF:
+  debug_str:
+    - 'Apple clang version 16.0.0 (clang-1600.0.9.14)'
+    - '/tmp/test.cpp'
+    - '/Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk'
+    - MacOSX14.4.sdk
+    - '/Users/shubham/Development/llvm-project/build_ninja'
+    - foo
+    - _Z3foov
+  debug_abbrev:
+    - ID:              0
+      Table:
+        - Code:            0x1
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_yes
+          Attributes:
+            - Attribute:       DW_AT_producer
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_data2
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_LLVM_sysroot
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_APPLE_sdk
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_stmt_list
+              Form:            DW_FORM_sec_offset
+            - Attribute:       DW_AT_comp_dir
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_low_pc
+              Form:            DW_FORM_addr
+            - Attribute:       DW_AT_high_pc
+              Form:            DW_FORM_data4
+        - Code:            0x2
+          Tag:             DW_TAG_subprogram
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_low_pc
+              Form:            DW_FORM_addr
+            - Attribute:       DW_AT_high_pc
+              Form:            DW_FORM_data4
+            - Attribute:       DW_AT_APPLE_omit_frame_ptr
+              Form:            DW_FORM_flag_present
+            - Attribute:       DW_AT_frame_base
+              Form:            DW_FORM_exprloc
+            - Attribute:       DW_AT_linkage_name
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_strp
+            - Attribute:       DW_AT_decl_file
+              Form:            DW_FORM_data1
+            - Attribute:       DW_AT_decl_line
+              Form:            DW_FORM_data1
+            - Attribute:       DW_AT_external
+              Form:            DW_FORM_flag_present
+  debug_info:
+    - Length:          0x48
+      Version:         4
+      AbbrevTableID:   0
+      AbbrOffset:      0x0
+      AddrSize:        8
+      Entries:
+        - AbbrCode:        0x1
+          Values:
+            - Value:           0x0
+            - Value:           0x4
+            - Value:           0x2F
+            - Value:           0x3D
+            - Value:           0x75
+            - Value:           0x0
+            - Value:           0x84
+            - Value:           0x0
+            - Value:           0x4
+        - AbbrCode:        0x2
+          Values:
+            - Value:           0x0
+            - Value:           0x4
+            - Value:           0x1
+            - Value:           0x1
+              BlockData:       [ 0x6F ]
+            - Value:           0xBC
+            - Value:           0xB8
+            - Value:           0x1
+            - Value:           0x1
+            - Value:           0x1
+        - AbbrCode:        0x0
+  debug_line:
+    - Length:          38
+      Version:         4
+      PrologueLength:  29
+      MinInstLength:   1
+      MaxOpsPerInst:   1
+      DefaultIsStmt:   1
+      LineBase:        251
+      LineRange:       14
+      OpcodeBase:      13
+      StandardOpcodeLengths: [ 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 ]
+      IncludeDirs:
+        - '/tmp'
+      Files:
+        - Name:            ''
+          DirIdx:          10
+          ModTime:         11
+          Length:          12
+      Opcodes:
+        - Opcode:          DW_LNS_extended_op
+          ExtLen:          1
+          SubOpcode:       DW_LNE_end_sequence
+          Data:            0
+...

Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments inside

// 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() == 0 &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!(LineTable->Prologue.FileNames.size() == 0 &&
if ((LineTable->Prologue.FileNames.size() || LineTable->Rows.size() != 1) && !LineTable->hasFileAtIndex(Row.File)) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice De-Morgan's law application, I wrote it the way I did because it was more readable to me, but either is fine

@@ -0,0 +1,356 @@
# RUN: rm -rf %t && mkdir -p %t
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to create a directory:

Suggested change
# RUN: rm -rf %t && mkdir -p %t
# RUN: yaml2obj %s -o %t.o
# RUN: llvm-dwarfdump --debug-line --verify %t.o 2>&1 | FileCheck %s

If there is no file name in the prologue of a line table, the verifier
will try to verify the file index, which will be set to 1 by default.
This will cause the DWARF verifier to throw an error even if there is no
error.
@rastogishubham rastogishubham merged commit 93d2e49 into llvm:main Jan 5, 2024
3 of 4 checks passed
rastogishubham added a commit to rastogishubham/llvm-project that referenced this pull request Jan 5, 2024
Accidentally didn't commit the review feedback before merging the PR
rastogishubham added a commit that referenced this pull request Jan 5, 2024
Accidentally didn't commit the review feedback before merging the PR
rastogishubham added a commit to rastogishubham/llvm-project that referenced this pull request Jan 6, 2024
…lvm#77004)

If there is no file name in the prologue of a line table, the verifier
will try to verify the file index, which will be set to 1 by default.
This will cause the DWARF verifier to throw an error even if there is no
error.

rdar://114476503
rdar://114343624
(cherry picked from commit 93d2e49)
rastogishubham added a commit to rastogishubham/llvm-project that referenced this pull request Jan 6, 2024
Accidentally didn't commit the review feedback before merging the PR

(cherry picked from commit f22dc88)
rastogishubham added a commit to apple/llvm-project that referenced this pull request Jan 10, 2024
Cherry-pick PR llvm#77004 from llvm.org/main to stable/20230725
qiaojbao pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Jan 26, 2024
…2c2d62d39

Local branch amd-gfx 7792c2d Merged main:c49965b97e159b7ff92a5fca3e144b2d2574a84d into amd-gfx:75d03f4f8f7e
Remote branch main 93d2e49 Fix file index verifier when there is no file name in the prologue. (llvm#77004)
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…lvm#77004)

If there is no file name in the prologue of a line table, the verifier
will try to verify the file index, which will be set to 1 by default.
This will cause the DWARF verifier to throw an error even if there is no
error.

rdar://114476503
rdar://114343624
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Accidentally didn't commit the review feedback before merging the PR
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request Feb 8, 2024
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.

[1]: llvm#77004
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request 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.

[1]: llvm#77004
felipepiovezan added a commit 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]: #77004
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)
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