[PATCH] D83958: [DebugInfo] Fix a misleading usage of DWARF forms with DIEExpr. NFC.

Thu Jul 16 20:54:49 PDT 2020

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D83958#2157396 <https://reviews.llvm.org/D83958#2157396>, @ikudrin wrote:

> The change is NFC (NFCI?).

Yeah, always intent is the best we can do, really.

> The form is ignored in `DIEExpr::emitValue()` and only the result of `SizeOf()` is used, which returned the expected value because it used `AP->getPointerSize()` for unhandled forms. All the construction is quite fragile and worked only thanks to the fortune.

Oh, form isn't ignored exactly - but it's used via SizeOf so the bug is symmetric: if SizeOf got the size wrong, at least it'd get it wrong both in the layout code that needed to know the size ahead of time, and in the bytes being emitted - so you wouldn't end up with the output being weirdly the wrong length compared to other lengths/offsets/etc used in the DWARF.

Makes sense. Thanks for the patch/explanation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83958