[PATCH] D83560: [DebugInfo] Added support for DW_OP_implicit_value in llvm

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

> For cases other than those gracefully handled(`float, double, long double) for DW_OP_implicit_value I'm considering a hard error sort of assert/llvm_unreachable or 2nd option could be just bail out early.

We should only use llvm_unreachable when it's actually unreachable. If it is possible to create IR input that would cause us to go down this path, we should use some other error reporting mechanism instead. I think I'd be happy with emitting a warning to llvm::dbgs(), or an assembler comment (if that is available here, not sure). We could also emit something like `DW_OP_nop` or `DW_OP_lit0 DW_OP_div`, or `DW_OP_user_hi`. Or we could flag the DwarfExpression object as having an error and enforce the owner to check this before the expression is emitted, but I think that's too much to ask just for this patch :-)



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:233
+  }
+  if (NumBytes == 10 /*long double*/) {
+    // long double IEEE representation uses 80 bits(10 bytes).
----------------
SouraVX wrote:
> aprantl wrote:
> > Why would we special-case Float80 instead of just emitting everything that we didn't emit in the above block here unconditionally?
> Initially thought of implementing that way, but due padding issues this cannot be done.
> For instance, for `long double` bit width will be 80 bits(10 bytes) and emit as
> ```
> emitUnsigned(NumBytes);
> for(...)
> emitData();
> ```
> This will ends up in expression of `NumBytes(10)` in this case, however `DW_AT_byte_size` for `long double` type is `16` bytes -- this conflict resulting in following error in `GDB` 
> ```
> (gdb) p ld
> access outside bounds of object referenced via synthetic pointer.
> ```
> For addressing padding issues and another distant issue(some other Floating point constant with different IEEE representation should not be emitted erronously.)
Thanks for the explanation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83560