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