[PATCH] D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue

[Adrian Prantl via Phabricator via llvm-commits]

aprantl accepted this revision.
aprantl added a comment.

Still works for me, but please add a comment explaining why getStoreSize is used here and what that means.



================
Comment at: include/llvm/IR/IntrinsicInst.h:98
+    /// that is described.
+    Optional<uint64_t> getFragmentSize() const;
+
----------------
bjope wrote:
> aprantl wrote:
> > `getFragmentSizeInBits()` ?
> > 
> > I'm still curious whether it makes sense to support the case where this returns None, or if we could add a Verifier check that is effectively `assert_di(getFragmentSize())`. Doesn't need to be in this commit though.
> I don't know if we can do that check in the verifier. I guess it is bad if we can't determine the size of the variable (as we would lose debug info).
> 
> All I know is that there is a comment about "broken types" in the implementation of DIVariable::getSizeInBits():
> ```
> Optional<uint64_t> DIVariable::getSizeInBits() const {
>   // This is used by the Verifier so be mindful of broken types.
>   ...
> ```
> I have no idea if those "broken types" only exist in handwritten / poorly stripped IR, or if they are common in real use cases.
That "broken types" comment refers to the need for the Verifier to be robust against malformed debug metadata. The Verifier should not crash in the presence of malformed input, and it also shouldn't accept malformed input. Given the many embedded uses of LLVM both of these situations could be conceivably security vulnerabilities.


Repository:
  rL LLVM

https://reviews.llvm.org/D48024