[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: [PATCH] CodeGen: Resize LifetimeExtendedCleanupHeader to avoid alignment issues
From: Justin Bogner <mail () justinbogner ! com>
Date: 2015-06-30 23:58:41
Message-ID: m2wpyku4su.fsf () chronotis ! apple ! com
[Download RAW message or body]
ping
Justin Bogner <mail@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@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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic