[PATCH] D80797: [DebugInfo] Check for errors when reading data for extended opcode

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDataExtractor.h:74
+                               Error *Err = nullptr) const {
+    return getRelocatedValue(getAddressSize(), Off, SecIx, Err);
   }
----------------
aprantl wrote:
> I assume this never turns an error into a success?
Nope. The idea is that if the error is already set, then the function will be a no-op, and the error will remain in the erroneous (and unchecked) state.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:877
       default:
-        if (OS)
+        if (!Err && OS)
           *OS << format("Unrecognized extended op 0x%02.02" PRIx8, SubOpcode)
----------------
aprantl wrote:
> This seems oddly stricter than before. Is the `!Err` condition necessary?
I'm guessing this is because  in the erroneous state the `SubOpcode` and `Len` won't contain real data, but only zeroes, and so a better course of action is to print the error about reaching the end-of-file (contribution) message.

Though if that's the case, then it does bring up the question of why is this the only statement guarded by `!Err` -- this argument should apply to other print statements  too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80797