[PATCH] D71875: [DWARF] Return Error from DWARFDebugArangeSet::extract().

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked 13 inline comments as done.
ikudrin added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:458
+      if (Error E = set.extract(arangesData, &offset)) {
+        WithColor::error() << toString(std::move(E)) << '\n';
+        break;
----------------
clayborg wrote:
> does "toString()" consume the error?
Yes. It uses `handleAllErrors()` internally.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugArangeSet.cpp:34
+Error DWARFDebugArangeSet::extract(DataExtractor data, uint64_t *offset_ptr) {
+  assert(data.isValidOffset(*offset_ptr));
+  ArangeDescriptors.clear();
----------------
clayborg wrote:
> return error instead of crashing the program
This just checks for the precondition. Callers are responsible to call the method with valid arguments. Therefore, if this triggers, it should be considered as an internal error and fixed.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugAranges.cpp:23
 
 void DWARFDebugAranges::extract(DataExtractor DebugArangesData) {
   if (!DebugArangesData.isValidOffset(0))
----------------
clayborg wrote:
> return llvm::Error here?
As this is just a private helper method for `DWARFDebugAranges::generate()`, I opted to print the errors here.


================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:65-66
 
-  while (Set.extract(ArangesData, &Offset)) {
+  while (ArangesData.isValidOffset(Offset) &&
+         !errorToBool(Set.extract(ArangesData, &Offset))) {
     DWARFYAML::ARange Range;
----------------
clayborg wrote:
> Dump the error if Set.extract() fails?
Done, but it pulled a lot of changes. Maybe it worth to move all them into a separate patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71875/new/

https://reviews.llvm.org/D71875