[PATCH] CodeGen: Resize LifetimeExtendedCleanupHeader to avoid alignment issues

[Justin Bogner]

Richard Smith <richard at metafoo.co.uk> writes:
> LGTM

r241133

> On Tue, Jun 30, 2015 at 4:58 PM, Justin Bogner <mail at justinbogner.com> wrote:
>
>     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...);
>     _______________________________________________
>     cfe-commits mailing list
>     cfe-commits at cs.uiuc.edu
>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits