[PATCH] D30785: [DWARF] Versioning for DWARF constants; verify FORMs

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DIE.cpp:82-89
+    if (!dwarf::isValidFormForVersion(AttrData.getForm(),
+                                      AP->getDwarfVersion())) {
+      std::string msg;
+      raw_string_ostream Msg(msg);
+      Msg << "Invalid form " << format("0x%x", AttrData.getForm())
+          << " for DWARF version " << AP->getDwarfVersion();
+      report_fatal_error(Msg.str());
----------------
Should this be an assertion instead?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:918-919
     addFlag(Buffer, dwarf::DW_AT_declaration);
-    return addDIETypeSignature(Buffer, dwarf::DW_AT_signature, Identifier);
+    if (getDwarfVersion() >= 4)
+      addDIETypeSignature(Buffer, dwarf::DW_AT_signature, Identifier);
+    return;
----------------
This is probably incorrect (short of the broader discussion I started about how references to type units should be implemented - I should ping that thread).

If the user requests type units, either it should fail if asking for a DWARF version that doesn't support type units - or ignore the conflict and produce the signature anyway?


https://reviews.llvm.org/D30785