[PATCH] D92728: [NFC][MSan] Round up OffsetPtr in PoisonMembers

Mon Dec 7 14:52:00 PST 2020

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

LGTM

================
Comment at: clang/lib/CodeGen/CGClass.cpp:1742
+          Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) +
+                                      Context.getCharWidth() - 1);
+      llvm::ConstantInt *OffsetSizePtr =
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > morehouse wrote:
> > > How does this interact with bit fields?
> > I guess existing code work because is bit fields are trivial and the one aligned to the char is going to be layoutStartOffset
> > I assume if we call this function for layoutStartOffset pointing to the bitfield in the middle of char it will poison entire byte which is incorrect.
> > With the patch it will not poison such chars: now it will be either all bits or nothing which I believe better.
> toCharUnitsFromBits already down aligns by definition: it's just a bitsoffset/charwidth
> 
> regarding assert: I am not 100% that this is not possible, maybe some corner-case in language allows that.
> if so, it's better not to poison partial byte at all than poison it completely.
> 
> I assume if we call this function for layoutStartOffset pointing to the bitfield in the middle of char it will poison entire byte which is incorrect.
Right. We never do this, of course, because the previous non-trivial field can not possibly end in the middle of a char.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92728