[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 ↗

(On Diff #438772)

(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 ↗

(On Diff #438772)

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 ↗

(On Diff #438772)

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

[BertalanD]

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

3–4 ↗

(On Diff #438772)

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

6

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.s

+# REQUIRES: aarch64
+
+## Check that LLD does not crash if it encounters DWARF sections
+## without __debug_info compile unit DIEs being present.
+
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %s -o %t.o
+# RUN: %lld -arch arm64 %t.o -o /dev/null
+
+.text
+.globl _main
+_main:
+  ret
+
+.section  __DWARF,__debug_abbrev,regular,debug
+  .byte 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: