[DebugInfo] - Fix attempts of reading past the end of .eh_frame in DWARFDebugFrame::parse().

[grimar]

Summary

I've noticed an issue with "Data.getRelocatedValue(...)" call.

it might silently ignore an error when a content is truncated.
That leads to an infinite loop in the code (e.g. llvm-readobj hangs).

After fixing the issue I've found that actually we always tried
to read past the end of a section, even when a content was valid.
It happened because the terminator CIE (a CIE with the length == 0)
was never handled. At first I've tried just to stop adding the terminator
entry (and return), but it does not seem to be correct, because tools like
llvm-objdump might want to print something for such entries
(see comments in the code and test cases).

This patch fixes issues mentioned, provides new test cases for
both llvm-readobj and lib/DebugInfo and adds FIXMEs to existent
test cases related.

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

381

do -> to

what is "something special"?

[MaskRay]

Fix multiple issues in DWARFDebugFrame::parse().

Better to call out the exact problems: truncated contents issues.

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

381

Do you mean readelf -wf/objdump -Wf output ZERO terminator?

This code here is correct. According to D46566: glibc unwind-dw2-fde.c classify_object_over_fdes has (I just checked, it still has) an oob problem if there is no ZERO terminator.

388

Minor nit: push_back (does not require a special constructor)

llvm/test/DebugInfo/X86/eh-frame-truncated.s

5

-unknown-linux can be dropped (generic ELF behavior)

[grimar]

In D80299#2050391, @MaskRay wrote:

Fix multiple issues in DWARFDebugFrame::parse().

Better to call out the exact problems: truncated contents issues.

It is not only about truncation. CIE terminator entries were not handled and we tried to
read past the end of section buffer. Though no visible issue/crash happened because the error
was never reported.

Upd: I've adjusted the patch name a bit.

[grimar]

• Addressed review comments.

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

381

what is "something special"?

Do you mean readelf -wf/objdump -Wf output ZERO terminator?

Yeah. I've updated the comment to clarify.

388

Right, thanks!

[MaskRay]

LGTM. Probably worth giving other reviewers a change to comment (May 25 is a US holiday)

llvm/test/DebugInfo/X86/eh-frame-truncated.s

2

the content

the content of .eh_frame
or
the content of the .eh_frame section

[MaskRay]

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

382

llvm-objdump --dwarf=frames

Giving a bit more context here helps the reader.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM, with nit (somehow forgot to submit this when previously reviewing it, sorry for the delay).

llvm/test/tools/llvm-readobj/ELF/unwind.test

247

contains a truncated -> contains truncated

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

Error DWARFDebugFrame::parse(DWARFDataExtractor Data) {
   DenseMap<uint64_t, CIE *> CIEs;
 
   while (Data.isValidOffset(Offset)) {
     uint64_t StartOffset = Offset;
 
     uint64_t Length;
     DwarfFormat Format;
     std::tie(Length, Format) = Data.getInitialLength(&Offset);
-    uint64_t Id;
+    bool IsDWARF64 = Format == DWARF64;
+
+    // If the Length is 0, then this CIE is a terminator. We add it because some
+    // dumper tools might need it to print something special for such entries
+    // (e.g. llvm-objdump --dwarf=frames prints "ZERO terminator").
+    if (Length == 0) {
+      auto Cie = std::make_unique<CIE>(
+          IsDWARF64, StartOffset, 0, 0, SmallString<8>(), 0, 0, 0, 0, 0,
+          SmallString<8>(), 0, 0, None, None, Arch);
+      CIEs[StartOffset] = Cie.get();
+      Entries.push_back(std::move(Cie));
+      break;
+    }
 
     // At this point, Offset points to the next field after Length.
     // Length is the structure size excluding itself. Compute an offset one
     // past the end of the structure (needed to know how many instructions to
     // read).
     uint64_t StartStructureOffset = Offset;
     uint64_t EndStructureOffset = Offset + Length;
 
     // The Id field's size depends on the DWARF format
-    bool IsDWARF64 = Format == DWARF64;
-    Id = Data.getRelocatedValue((IsDWARF64 && !IsEH) ? 8 : 4, &Offset);
+    Error Err = Error::success();
+    uint64_t Id = Data.getRelocatedValue((IsDWARF64 && !IsEH) ? 8 : 4, &Offset,
+                                         /*SectionIndex=*/nullptr, &Err);
+    if (Err)
+      return Err;
+
     if (Id == getCIEId(IsDWARF64, IsEH)) {
       uint8_t Version = Data.getU8(&Offset);
       const char *Augmentation = Data.getCStr(&Offset);
       StringRef AugmentationString(Augmentation ? Augmentation : "");
       uint8_t AddressSize = Version < 4 ? Data.getAddressSize() :
                                           Data.getU8(&Offset);
       Data.setAddressSize(AddressSize);
       uint8_t SegmentDescriptorSize = Version < 4 ? 0 : Data.getU8(&Offset);

llvm/test/DebugInfo/X86/eh-frame-truncated.s

+## Check we report a proper error when the content
+## of the .eh_frame section is truncated.
+
+# RUN: llvm-mc -triple x86_64 %s -filetype=obj -o %t
+# RUN: not llvm-dwarfdump -debug-frame %t 2>&1 | FileCheck %s
+
+# CHECK: error: unexpected end of data at offset 0x4
+
+.section .eh_frame,"a",@unwind
+.long 0xFF ## Length

llvm/test/tools/llvm-objdump/eh_frame-mipsel.test

 
 # CHECK: 0000001c 00000018 00000020 FDE cie=00000000 pc=00400890...004008dc
 # CHECK:   DW_CFA_advance_loc: 4
 # CHECK:   DW_CFA_def_cfa_offset: +24
 # CHECK:   DW_CFA_advance_loc: 4
 # CHECK:   DW_CFA_offset: reg31 -4
 # CHECK:   DW_CFA_nop:
 
+## FIXME: GNU objdump prints "00000038 ZERO terminator" instead.
 # CHECK: 00000038 00000000 00000000 CIE
 # CHECK:   Version:               0
 # CHECK:   Augmentation:          ""
 # CHECK:   Code alignment factor: 0
 # CHECK:   Data alignment factor: 0
 # CHECK:   Return address column: 0

llvm/test/tools/llvm-objdump/eh_frame_zero_cie.test

 # RUN: llvm-objdump --dwarf=frames %p/Inputs/eh_frame_zero_cie.o 2>/dev/null | FileCheck %s
 
 # CHECK: .eh_frame contents:
 
+## FIXME: GNU objdump prints "00000000 ZERO terminator" instead.
 # CHECK: 00000000 00000000 00000000 CIE
 # CHECK:   Version:               0
 # CHECK:   Augmentation:          ""
 # CHECK:   Code alignment factor: 0
 # CHECK:   Data alignment factor: 0
 # CHECK:   Return address column: 0

llvm/test/tools/llvm-readobj/ELF/unwind.test

 ## .section .eh_frame,"a",@unwind
 ## .long .Lend - .LCIEptr  # Length
 ## .LCIEptr:
 ##   .long 0xffffffff # CIE pointer
 ##   .quad 0x1111abcd # Initial location
 ##   .quad 0x00010000 # Address range
 ## .Lend:
     Content: 14000000FFFFFFFFCDAB1111000000000000010000000000
+
+## Check we report a error when the .eh_frame section contains truncated data.
+# RUN: yaml2obj --docnum=3 %s -o %t3.exe
+# RUN: not llvm-readobj --unwind %t3.exe 2>&1 | FileCheck %s -DFILE=%t3.exe --check-prefix=TRUNCATED-ERR
+
+# TRUNCATED-ERR:      .eh_frame section at offset 0x34 address 0x0:
+# TRUNCATED-ERR-NEXT: error: '[[FILE]]': unexpected end of data at offset 0x4
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_386
+Sections:
+  - Name:    .eh_frame
+    Type:    SHT_PROGBITS
+## Length is set to 0xFF, though the actual section length is 4.
+    Content: "FF000000"