Crash on alignments >= 2**16 (PR26444)

Sema::AddAlignment() defines MaxValidAlignment like this:

  // Alignment calculations can wrap around if it's greater than 2**28.
  unsigned MaxValidAlignment =
      Context.getTargetInfo().getTriple().isOSBinFormatCOFF() ? 8192
                                                              : 268435456;

But AggValueSlot stores Alignment as an unsigned short:

class AggValueSlot {
  unsigned short Alignment;

The max value for an unsigned short is 2**16 -1, but you are passing 2**16, which AddeAlignment says is okay.  However, it ends up getting stored as 0 in an unsigned short.

The fix is to make these sizes consistent, but I'm not sure which should be changed  (though I'm guessing Alignment should be unsigned instead of unsigned short).

From the LLVM perspective, the maximum alignment is specified as:

In Value.h:
00520   /// \brief The maximum alignment for instructions.
00521   ///
00522   /// This is the greatest alignment value supported by load, store, and alloca
00523   /// instructions, and global values.
[00524](   static const unsigned [MaxAlignmentExponent]( = 29;
[00525](   static const unsigned [MaximumAlignment]( = 1u << [MaxAlignmentExponent](;

This is checked in the Verifier via:
Assert(GV.getAlignment() <= Value::MaximumAlignment,
“huge alignment values are unsupported”, &GV);
(and similiar checks for Load, Store, Alloca, but not the various atomic instructions)

Hi Philip:

Clang does check to make sure max alignment is <= 29, so that is consistent, however, AggValueSlot stores it in an unsigned short. On my system, OSX, that means values >= 2**16 are stored as 0, which causes and assert/crash.

I submitted a patch over the weekend to change it to an unsigned,, but it hasn’t been reviewed yet.

thanks for taking a look…