[ADT] [lld-macho] Check for end iterator deref in filter_iterator_base

[BertalanD]

Summary

If ld64.lld was supplied an object file that had a __debug_abbrev or
__debug_str section, but didn't have any compile unit DIEs in
__debug_info, it would dereference an iterator pointing to the empty
array of DIEs. This underlying issue started causing segmentation faults
when parsing for __debug_info was addded in D128184. That commit was
reverted, and this one fixes the invalid dereference to allow relanding
it.

This commit adds an assertion to filter_iterator_base's dereference
operators to catch bugs like this one.

Ran check-llvm, check-clang and check-lld

[thakis]

Nice!

[mstorsjo]

lld/test/MachO/dwarf-no-compile-unit.yaml

1

(driveby comment) I don't think you'd need the aarch64 target enabled, just for running yaml2obj on an aarch64 object file (unless the linker requires the aarch64 target for linking)?

[int3]

lld/MachO/InputFiles.cpp

1016

parens unnecessary

lld/test/MachO/dwarf-no-compile-unit.yaml

3–4

how was this generated?

llvm/include/llvm/ADT/STLExtras.h

447–456

this seems like it should be a separate commit

447–456

(but if thakis thinks it's fine then I guess it is)

[int3]

lld/test/MachO/dwarf-no-compile-unit.yaml

3–4

(and why can't we generate it directly from llvm-mc?)

[BertalanD]

lld/test/MachO/dwarf-no-compile-unit.yaml

3–4

I guess we can. I'll update the patch in a sec.

[thakis]

llvm/include/llvm/ADT/STLExtras.h

447–456

(bertaland suggested making it a separate commit originally, but I thought it'd be nice to have assert and fix in one change (?))

[BertalanD]

Call llvm-mc directly and remove unnecessary parentheses.

[BertalanD]

oops, no

missing a REQUIRES line

[int3]

much shorter test case, nice :)

lld/test/MachO/dwarf-no-compile-unit.s

5 ↗

(On Diff #438791)

could just be -o /dev/null

llvm/include/llvm/ADT/STLExtras.h

447–456

ah gotcha

[mstorsjo]

In D128294#3599885, @BertalanD wrote:

oops, no

missing a REQUIRES line

Sorry about that - indeed, if switching back to llvm-mc, you'll need the REQUIRES.

[BertalanD]

Third time's the charm (hopefully)

added missing REQUIRES and switched to -o /dev/null

Thank you for all the suggestions.

lld/MachO/InputFiles.cpp

auto *ctx = make<DWARFContext>(
       });
 
   // TODO: Since object files can contain a lot of DWARF info, we should verify
   // that we are parsing just the info we need
   const DWARFContext::compile_unit_range &units = ctx->compile_units();
   // FIXME: There can be more than one compile unit per object file. See
   // PR48637.
   auto it = units.begin();
-  compileUnit = it->get();
+  compileUnit = (it != units.end()) ? it->get() : nullptr;
 }
 
 ArrayRef<data_in_code_entry> ObjFile::getDataInCode() const {
   const auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
   const load_command *cmd = findCommand(buf, LC_DATA_IN_CODE);
   if (!cmd)
     return {};
   const auto *c = reinterpret_cast<const linkedit_data_command *>(cmd);

lld/test/MachO/dwarf-no-compile-unit.yaml

+# REQUIRES: aarch64
+
+## Check that LLD does not crash if it encounters DWARF sections
+## without __debug_info compile units DIEs being present.
+
+# RUN: yaml2obj %s -o %t.o
+# RUN: %lld -dylib -arch arm64 -o %t.dylib %t.o
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x100000C
+  cpusubtype:      0x0
+  filetype:        0x1
+  ncmds:           4
+  sizeofcmds:      360
+  flags:           0x0
+  reserved:        0x0
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         232
+    segname:         ''
+    vmaddr:          0
+    vmsize:          1
+    fileoff:         392
+    filesize:        1
+    maxprot:         7
+    initprot:        7
+    nsects:          2
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0
+        size:            0
+        offset:          0x188
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x80000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         ''
+      - sectname:        __debug_abbrev
+        segname:         __DWARF
+        addr:            0x0
+        size:            1
+        offset:          0x188
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x2000000
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+  - cmd:             LC_BUILD_VERSION
+    cmdsize:         24
+    platform:        1
+    minos:           786432
+    sdk:             0
+    ntools:          0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          400
+    nsyms:           1
+    stroff:          416
+    strsize:         8
+  - cmd:             LC_DYSYMTAB
+    cmdsize:         80
+    ilocalsym:       0
+    nlocalsym:       1
+    iextdefsym:      1
+    nextdefsym:      0
+    iundefsym:       1
+    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:          1
+      n_type:          0xE
+      n_sect:          1
+      n_desc:          0
+      n_value:         0
+  StringTable:
+    - ''
+    - ltmp0
+    - ''
+DWARF:
+  debug_abbrev:
+    - ID:              0
+...

llvm/include/llvm/ADT/STLExtras.h

 public:
   using BaseT::operator++;
 
   filter_iterator_base &operator++() {
     BaseT::operator++();
     findNextValid();
     return *this;
   }
+
+  decltype(auto) operator*() const {
+    assert(BaseT::wrapped() != End && "Cannot dereference end iterator!");
+    return BaseT::operator*();
+  }
+
+  decltype(auto) operator->() const {
+    assert(BaseT::wrapped() != End && "Cannot dereference end iterator!");
+    return BaseT::operator->();
+  }
 };
 
 /// Specialization of filter_iterator_base for forward iteration only.
 template <typename WrappedIteratorT, typename PredicateT,
           typename IterTag = std::forward_iterator_tag>
 class filter_iterator_impl
     : public filter_iterator_base<WrappedIteratorT, PredicateT, IterTag> {
 public: