[dsymutil] Don't resolve DIE reference to NULL DIE.

[JDevlieghere]

Summary

This patch prevents dsymutil from resolving a reference to a NULL DIE
when a bogus reference happens to be coincidentally referencing a NULL
DIE. Now this is detected as an invalid reference and a warning is
printed.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=33873

[friss]

test/tools/dsymutil/X86/null-die.s

10 ↗

(On Diff #115993)

Do you need the final binary file? My guess is that you only need the .o file and a debug map which can be YAML. You can use 'llvm-dsymutil -dump-debug-map' on null_die to extract it.

tools/dsymutil/DwarfLinker.cpp

1498–1503

I would add a comment here. There is no valid reason for the DIE to be NULL. Something along the lines of "In a file with broken references, an attribute might point to a NULL DIE".

[dblaikie]

test/tools/dsymutil/X86/null-die.s

4–10 ↗

(On Diff #115993)

This seems like a rather large test case for this small issue?

[JDevlieghere]

• Address review comments from Fred and David.
• Convert test to YAML with instruction on how to reproduce the binary.

[JDevlieghere]

• Remove relative path in test.

[dblaikie]

test/tools/dsymutil/X86/null-die.test

10–19

This DWARF could be a bit simpler by skipping argc and argv and having foo be void() rather than int(int), have foo call some other function & compile without optimizations:

void f1() {}
__attribute__((always_inline)) void f2() {
  f1();
}
int main() {
  f2();
}

(marginally simpler if 'int main' were just 'void f3' - though then I guess you'd need a stub object (which wouldn't need debug info anyway) to link in main, not sure, which might be more hassle than necessary - if you did that you could also put f1 out of line in that object file so there would be no DWARF for f1 either, again simplifying the test case a bit)

Also potentially simpler if this is in C++ and f2 is marked 'inline' as well, that way there's no out of line definition of f2.

[JDevlieghere]

Thanks David, I appreciate your feedback!

[friss]

I think the test can go in the generic test directory. If you're not generating any output, it shouldn't need the X86 backend.

Otherwise, LGTM

test/tools/dsymutil/X86/null-die.test

+#RUN: llvm-dsymutil -f -oso-prepend-path=%p/../Inputs/ -y %s -no-output 2>&1 \
+#RUN:   |  FileCheck %s
+
+# CHECK: warning: could not find referenced DIE
+
+# We've modified the DW_AT_abstract_origin offset to reference a NULL DIE.
+#
+# Source:
+#
+#   int foo(int i) __attribute__((always_inline));
+#   int foo(int i)
+#   {
+#     return i + 10;
+#   }
+#
+#   int main(int argc, char** argv)
+#   {
+#     return foo(argc);
+#   }
+#
+# Compile with:
+#
+#   $ clang -S -O3 -g test.c -o null_die.s
+#
+#   Manually patch the DW_AT_abstract_origin to point to a NULL DIE.
+#
+#   $ llvm-mc -triple x86_64-apple-darwin -filetype=obj -o null_die.o null_die.s
+#   $ ld -arch x86_64 -macosx_version_min 10.13.0 -lSystem null_die.o -o null_die
+
+---
+triple:          'x86_64-apple-darwin'
+binary-path:     ../llvm-dsymutil-crash/test/tools/dsymutil/Inputs/null_die
+objects:
+  - filename:        /null_die.o
+    timestamp:       1505906803
+    symbols:
+      - { sym: _foo, objAddr: 0x0000000000000000, binAddr: 0x0000000100000F30, size: 0x00000010 }
+      - { sym: _main, objAddr: 0x0000000000000010, binAddr: 0x0000000100000F40, size: 0x00000009 }
+...

tools/dsymutil/DwarfLinker.cpp

 static DWARFDie resolveDIEReference(
     const DwarfLinker &Linker, std::vector<std::unique_ptr<CompileUnit>> &Units,
     const DWARFFormValue &RefValue, const DWARFUnit &Unit,
     const DWARFDie &DIE, CompileUnit *&RefCU) {
   assert(RefValue.isFormClass(DWARFFormValue::FC_Reference));
   uint64_t RefOffset = *RefValue.getAsReference();
 
   if ((RefCU = getUnitForOffset(Units, RefOffset)))
-    if (const auto RefDie = RefCU->getOrigUnit().getDIEForOffset(RefOffset))
+    if (const auto RefDie = RefCU->getOrigUnit().getDIEForOffset(RefOffset)) {
+      // In a file with broken references, an attribute might point to a NULL
+      // DIE.
+      if(!RefDie.isNULL())
         return RefDie;
+    }
 
   Linker.reportWarning("could not find referenced DIE", &DIE);
   return DWARFDie();
 }
 
 /// \returns whether the passed \a Attr type might contain a DIE
 /// reference suitable for ODR uniquing.
 static bool isODRAttribute(uint16_t Attr) {