[PATCH] D72154: [DebugInfo] Make debug line address size mismatch non-fatal to parsing

[James Henderson via Phabricator via llvm-commits]

jhenderson marked an inline comment as done.
jhenderson added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:597
         // from the size of the operand.
-        if (DebugLineData.getAddressSize() == 0)
-          DebugLineData.setAddressSize(Len - 1);
-        else if (DebugLineData.getAddressSize() != Len - 1) {
-          return createStringError(errc::invalid_argument,
-                             "mismatching address size at offset 0x%8.8" PRIx64
-                             " expected 0x%2.2" PRIx8 " found 0x%2.2" PRIx64,
-                             ExtOffset, DebugLineData.getAddressSize(),
-                             Len - 1);
+        {
+          uint8_t ExtractorAddressSize = DebugLineData.getAddressSize();
----------------
ikudrin wrote:
> I find these lines a bit hard to follow, without prior understanding of the whole idea. I would suggest reorganize them like this:
> ```
> uint8_t ExtractorAddressSize = DebugLineData.getAddressSize();
> if (ExtractorAddressSize != Len - 1) {
>   RecoverableErrorCallback(...);
> }
> // Assuming that the line table is correct, temporarily override the address size.
> DebugLineData.setAddressSize(Len - 1);
> State.Row.Address.Address = ...
> // Restore the address size if the extractor already had it.
> if (ExtractorAddressSize != 0)
>   DebugLineData.setAddressSize(ExtractorAddressSize);
> ```
> What do you think?
Yes, I think that should work, with one minor change to the error check - a value of zero from the extractor shouldn't cause an error. I'll try that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72154