[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4484
+  if (!TooManyActiveBits) {
+    AlignVal = Alignment.getZExtValue();
+    // C++11 [dcl.align]p2:
----------------
erichkeane wrote:
> So looking more closely, THIS is the problem right here.  I think we probably should just be storing `MaximumAlignment` and `AlignVal` as an llvm::APInt and just do the comparison on `APInt`, rather than try to be tricky with the bits here.
> 
> Basically, don't extract the 'Alignment' from the `APInt` until after the test for >.
You're correct that this is the cause of the assertion, which happens when `Alignment` can't fit in a `uint64_t`.

It took me a minute to see what you meant, but I agree, we can test whether `Alignment < MaximumAlignment` and rely on the fact that `MaximumAlignment` will be representable in a 64-bit integer. I think I led @shafik astray with a comment I made on the issue.


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

https://reviews.llvm.org/D152335