[libcxx-commits] [PATCH] D72543: [libcxxabi] Insert padding in __cxa_exception struct for compatibility

[Shoaib Meenai via Phabricator via libcxx-commits]

smeenai added a comment.

In D72543#1825454 <https://reviews.llvm.org/D72543#1825454>, @rjmccall wrote:

> In D72543#1825422 <https://reviews.llvm.org/D72543#1825422>, @smeenai wrote:
>
> > In D72543#1825178 <https://reviews.llvm.org/D72543#1825178>, @rjmccall wrote:
> >
> > > In D72543#1825150 <https://reviews.llvm.org/D72543#1825150>, @smeenai wrote:
> > >
> > > > From spelunking through history, it looks like D33030 <https://reviews.llvm.org/D33030> tried to solve the same problem, and then rL319123 <https://reviews.llvm.org/rL319123> undid that solution and came up with a different one to solve the same underlying issue. rL319123 <https://reviews.llvm.org/rL319123> is still in place; does it need to be adjusted for the change in this diff?
> > >
> > >
> > > It looks like those patches were attempts to ensure adequate alignment of the exception object given the absence of an alignment attribute on the `_Unwind_Exception` type itself.  rL319123 <https://reviews.llvm.org/rL319123> became inadequate after `_Unwind_Exception` became an aligned type because that change still affected the layout of the C++ exception types.  What we're fixing here is essentially the same problem that was introduced by D33030 <https://reviews.llvm.org/D33030>.
> >
> >
> > Makes sense. Should rL319123 <https://reviews.llvm.org/rL319123> be reverted then? The extra space that's being allocated by that change doesn't do what it's meant to do, so it seems wasteful to keep allocating it.
>
>
> If `__cxa_exception` is already sufficiently aligned, I believe that patch does not allocate any extra space.


You're right; I understand the case rL319123 <https://reviews.llvm.org/rL319123> was intended for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72543/new/

https://reviews.llvm.org/D72543