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

[Shoaib Meenai via Phabricator via libcxx-commits]

smeenai added a subscriber: hans.
smeenai added a comment.

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.

>> Additionally, is this something that should be picked to 10.0 after it's committed?
> 
> I think this should be put in 10.0, yes.

CC @hans



================
Comment at: libcxxabi/src/cxa_exception.cpp:59
+#else
+static_assert(offsetof(__cxa_exception, adjustedPtr) +
+                      sizeof(_Unwind_Exception) + sizeof(void*) ==
----------------
There's no `adjustedPtr` field in the `_LIBCXXABI_ARM_EHABI` case ... the field right before the `unwindHeader` in that case is `int propagationCount`.


================
Comment at: libcxxabi/src/cxa_exception.cpp:63
+              "adjustedPtr has wrong negative offset");
+static_assert(offsetof(__cxa_dependent_exception, adjustedPtr) +
+                      sizeof(_Unwind_Exception) + sizeof(void*) ==
----------------
Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72543