[PATCH] D34328: [ELF] - Add ability for DWARFContextInMemory to exit early when any error happen.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:357
+      const object::ObjectFile &Obj, const LoadedObjectInfo *L = nullptr,
+      function_ref<bool(Error)> HandleError = defaultErrorHandler);
 
----------------
dblaikie wrote:
> Might be worth introducing an enum ("Halt/Continue") rather than using a bool, so it's more obvious/legible which result means what.
Done.


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:2150-2155
+  // Data contains bytes of object file generated by yaml2obj tool.
+  // YAML contains description of object with two broken compressed
+  // sections.
+  // To produce Data array next commands and following yaml file was used:
+  // yaml2obj test.s -o test.o
+  // xxd -i test.o
----------------
dblaikie wrote:
> I thought one of the ideas of the yaml2obj stuff was to enable the writing of invalid inputs, etc - is that not the case? Should that functionality be added to the yaml2obj somehow rather than having big byte arrays?
What I was trying to achieve here is to produce broken object::ObjectFile somehow to be able to test first constructor of DWARFContextInMemory with new functionality.
Extending yaml2obj to produce StringRef (if I correctly understood the suggestion) would require doing some librarification for it. Like splitting part of yaml2obj into separate library probably. And then test would be able to use such library.

It probably can be usefull (I am not sure how much other users we may have, looks can be usefull for testcases like this), but sounds as a large enough separate change. It is unclear for me if introducing a new library worth that atm.

Fortunately I found alternative way to achieve what I wanted initially in this testcase, without using additional arrays and dependency on yaml2obj. Solution partially consistent with other testcases in this file which aslo use dwarfgen::Generator but for different things.

Alternative solution I was thinking about - is to introduce --error-limit=X option to llvm-dwarfdump. It probably may be usefull itself and would allow to write testcase in a regular manner (separate test file + one/two broken input files and regular tools calls).


https://reviews.llvm.org/D34328