[libcxxabi] Align unwindHeader on a double-word boundary

[ahatanak]

Summary

r276215 made a change to annotate _Unwind_Exception with "attribute((aligned))" so that exception objects are double-word aligned. This fix hasn't been incorporated to unwind.h on Darwin yet and, since it is an ABI breaking change, I'm not sure it will ever happen in the future.

Instead of annotating struct _Unwind_Exception with the attribute, this patch annotates field unwindHeader of __cxa_exception. This fixes the test case (exception_object_alignment.sh.cpp) that used to crash because of the under-aligned object.

rdar://problem/25364625

[mehdi_amini]

Something still isn't clear to me: the issue IIUC is not with the unwindHeader itself, but with the misaligned thrown object that is allocated right after the __cxa_exception itself. Isn't it?

Then are you aligning the unwindHeader field and by side-effect the size of the __cxa_exception structure becomes aligned and then the thrown object will be as well?

[ahatanak]

In D33030#751187, @mehdi_amini wrote:

Something still isn't clear to me: the issue IIUC is not with the unwindHeader itself, but with the misaligned thrown object that is allocated right after the __cxa_exception itself. Isn't it?

Then are you aligning the unwindHeader field and by side-effect the size of the __cxa_exception structure becomes aligned and then the thrown object will be as well?

Yes, that's correct.

If field unwindHeader is annotated with the aligned attribute, both the field and struct cxa_exception are aligned. As a result, the exception object that follows cxa_exception is aligned too.

If we annotate struct cxa_exception with the attribute instead of annotating the field, the test case segfaults. It seems that this approach doesn't work because several places in cxa_exception.cpp assume the exception object immediately follows field unwindHeader (there are should be no paddings at the end of cxa_exception).

[mehdi_amini]

In D33030#751723, @ahatanak wrote:

If field unwindHeader is annotated with the aligned attribute, both the field and struct cxa_exception are aligned. As a result, the exception object that follows cxa_exception is aligned too.

If we annotate struct cxa_exception with the attribute instead of annotating the field, the test case segfaults. It seems that this approach doesn't work because several places in cxa_exception.cpp assume the exception object immediately follows field unwindHeader (there are should be no paddings at the end of cxa_exception).

OK, then LGTM but add all these informations as a comment for unwindHeader field.

[sberg]

In LibreOffice we unfortunately have some dirty code that synthesizes exception throwing, calling __cxa_allocate_exception, __cxa_throw (passing in our own exception destructor function), and then, when the exception destructor function gets called, assumes we can map from the passed pointer to the __cxa_exception header preceding it (to get at its exceptionType member). That code was thus broken by this change. I have a dirty hack around that (checking whether the __cxa_exception exceptionDestructor member has the expected value; if not, adjust: Hack to dynamically adapt to __cxa_exceptiom in LLVM 5.0 libcxxabi) and two questions:

• Is there any estimation when this change will show up in macOS's libcxxabi? That would give me an idea which LibreOffice versions I would need to backport my fix to.

• Does anybody happen to have a good idea how to do my hacky fix in a cleaner way?

libcxxabi/trunk/src/cxa_exception.hpp

 
 #if !defined(__LP64__) && !defined(_LIBCXXABI_ARM_EHABI)
     // This is a new field to support C++ 0x exception_ptr.
     // For binary compatibility it is placed where the compiler
     // previously adding padded to 64-bit align unwindHeader.
     size_t referenceCount;
 #endif
 
-    _Unwind_Exception unwindHeader;
+    // This field is annotated with attribute aligned so that the exception
+    // object following the field is sufficiently aligned and there is no
+    // gap between the field and the exception object. r276215 made a change to
+    // annotate _Unwind_Exception with __attribute__((aligned)), but we cannot
+    // incorporate the fix on Darwin since it is an ABI-breaking change, which
+    // is why we need the attribute on this field.
+    _Unwind_Exception unwindHeader __attribute__((aligned));
 };
 
 // http://sourcery.mentor.com/archives/cxx-abi-dev/msg01924.html
 // The layout of this structure MUST match the layout of __cxa_exception, with
 // primaryException instead of referenceCount.
 struct _LIBCXXABI_HIDDEN __cxa_dependent_exception {
 #if defined(__LP64__) || defined(_LIBCXXABI_ARM_EHABI)
     void* primaryException;
#else
     void * catchTemp;
     void *adjustedPtr;
 #endif
 
 #if !defined(__LP64__) && !defined(_LIBCXXABI_ARM_EHABI)
     void* primaryException;
 #endif
 
-    _Unwind_Exception unwindHeader;
+    // See the comment in __cxa_exception as to why this field has attribute
+    // aligned.
+    _Unwind_Exception unwindHeader __attribute__((aligned));
 };
 
 struct _LIBCXXABI_HIDDEN __cxa_eh_globals {
     __cxa_exception *   caughtExceptions;
     unsigned int        uncaughtExceptions;
 #if defined(_LIBCXXABI_ARM_EHABI)
     __cxa_exception* propagatingExceptions;
 #endif

libcxxabi/trunk/test/exception_object_alignment.sh.cpp

+//===---------------- exception_object_alignment.sh.cpp -------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: libcxxabi-no-exceptions
+
+// RUN: %cxx %flags %compile_flags -O1 %s -o %t.exe
+// RUN: %exec %t.exe
+
+// This test used to segfault on Darwin because field unwindHeader of struct
+// __cxa_exception was not 16B aligned.
+
+namespace {
+
+struct S {
+  int a;
+  int __attribute__((aligned(16))) b;
+};
+
+class base1 {
+protected:
+  virtual ~base1() throw() {}
+};
+
+class derived: public base1 {
+public:
+  derived() : member() {}
+private:
+  S member;
+};
+
+}
+
+int main() {
+  try {
+    throw derived();
+  }
+  catch(...) {
+  }
+  return 0;
+}