[PATCH] CodeGen: Resize LifetimeExtendedCleanupHeader to avoid alignment issues

[Justin Bogner]

ping

Justin Bogner <mail at justinbogner.com> writes:
> The LifetimeExtendedCleanupHeader is carefully fit into 32 bytes,
> meaning that cleanups on the LifetimeExtendedCleanupStack are *always*
> allocated at a misaligned address and cause undefined behaviour.
>
> There are two ways to solve this - add padding after the header when
> we allocated our cleanups, or just simplify the header and let it use
> 64 bits in the first place. I've opted for the latter, and added a
> static assert to avoid the issue in the future.
>
> This does waste some space in the 32-bit case. It wouldn't be too hard
> to make the size of this struct dependent on the host machine if
> that's an issue. WDYT?
>
> commit 1f1d41a60f5106e9e89fbc917bfc0826b5caad02
> Author: Justin Bogner <mail at justinbogner.com>
> Date:   Mon Jun 22 16:02:17 2015 -0700
>
>     CodeGen: Resize LifetimeExtendedCleanupHeader to avoid alignment issues
>     
>     The LifetimeExtendedCleanupHeader is carefully fit into 32 bytes,
>     meaning that cleanups on the LifetimeExtendedCleanupStack are *always*
>     allocated at a misaligned address and cause undefined behaviour.
>     
>     There are two ways to solve this - add padding after the header when
>     we allocated our cleanups, or just simplify the header and let it use
>     64 bits in the first place. I've opted for the latter, and added a
>     static assert to avoid the issue in the future.
>
> diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
> index 45475d1..0891b5d 100644
> --- a/lib/CodeGen/CodeGenFunction.h
> +++ b/lib/CodeGen/CodeGenFunction.h
> @@ -283,12 +283,12 @@ public:
>    /// Header for data within LifetimeExtendedCleanupStack.
>    struct LifetimeExtendedCleanupHeader {
>      /// The size of the following cleanup object.
> -    unsigned Size : 29;
> +    unsigned Size;
>      /// The kind of cleanup to push: a value from the CleanupKind enumeration.
> -    unsigned Kind : 3;
> +    CleanupKind Kind;
>  
> -    size_t getSize() const { return size_t(Size); }
> -    CleanupKind getKind() const { return static_cast<CleanupKind>(Kind); }
> +    size_t getSize() const { return Size; }
> +    CleanupKind getKind() const { return Kind; }
>    };
>  
>    /// i32s containing the indexes of the cleanup destinations.
> @@ -388,6 +388,8 @@ public:
>      LifetimeExtendedCleanupStack.resize(
>          LifetimeExtendedCleanupStack.size() + sizeof(Header) + Header.Size);
>  
> +    static_assert(sizeof(Header) % llvm::AlignOf<T>::Alignment == 0,
> +                  "Cleanup will be allocated on misaligned address");
>      char *Buffer = &LifetimeExtendedCleanupStack[OldSize];
>      new (Buffer) LifetimeExtendedCleanupHeader(Header);
>      new (Buffer + sizeof(Header)) T(A...);