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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

Besides my above comment, I'm very much looking forward to this commit. It will also help a lot with implementing dwarfdump -verify functionality.



================
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());
----------------
probinson wrote:
> dblaikie wrote:
> > Should this be an assertion instead?
> I dithered on that.  An assertion would tell you "something bad happened somewhere" with no details, because there's no way I know of to report a runtime value in an assertion. So you don't know what FORM is causing the problem.  Doing it this way was more valuable when figuring out the test failures that I had to fix.
> But, it's true if we detect a problem here, that indicates an internal inconsistency rather than a problem with our external input, so on that basis an assertion would be more the thing.
This is something I'm still getting confused about. Doesn't `llvm_unreachable()` give the compiler the license to optimize away everything in the path leading up to the unreachable? Would `report_fatal_error()` be more appropriate here?


https://reviews.llvm.org/D30785