[CodeGenCXX] Fix ItaniumCXXABI::getAlignmentOfExnObject to return 8-byte alignment on Darwin

[ahatanak]

Summary

r246985 made changes to give a higher alignment for exception objects on the grounds that Itanium says _Unwind_Exception should be "double-word" aligned and the structure is normally declared with attribute((aligned)) guaranteeing 16-byte alignment. It turns out that libc++abi doesn't declare the structure with attribute((aligned)) and therefore only guarantees 8-byte alignment on 32-bit and 64-bit platforms. This caused a crash in some cases when the backend emitted SIMD store instructions that requires 16-byte alignment (such as movaps).

This patch makes ItaniumCXXABI::getAlignmentOfExnObject return an 8-byte alignment on Darwin to fix the crash.

[rjmccall]

lib/CodeGen/ItaniumCXXABI.cpp

174 ↗

(On Diff #51675)

Just add a ExnObjectAlignment field to TargetInfo, please. We already have a DarwinTargetInfo mix-in that can set this to something lower.

Otherwise, this patch LGTM, although as mentioned elsewhere we should figure out whether other OSes also limit this more strictly.

[ahatanak]

Thank you for the review, John.

I've updated the patch as per your request. I defined virtual function getExnObjectAlignment instead of adding a ExnObjectAlignment field to TargetInfo since I found that getDefaultAlignForAttributeAligned() can't be called in TargetInfo's constructor to initialize ExnObjectAlignment.

[rjmccall]

cfe/trunk/include/clang/Basic/TargetInfo.h

426

This should just be an ordinary comment, not a /// comment.

Please also add this to the documentation comment:

This is only meaningful for targets that allocate C++ exceptions in a system runtime, such as those using the Itanium C++ ABI.

[rjmccall]

Otherwise, LGTM.

[ahatanak]

cfe/trunk/include/clang/Basic/TargetInfo.h

426

Fixed in r265035.

cfe/trunk/include/clang/Basic/TargetInfo.h

public:
 
   /// \brief Return the maximum vector alignment supported for the given target.
   unsigned getMaxVectorAlign() const { return MaxVectorAlign; }
   /// \brief Return default simd alignment for the given target. Generally, this
   /// value is type-specific, but this alignment can be used for most of the
   /// types for the given target.
   unsigned getSimdDefaultAlign() const { return SimdDefaultAlign; }
 
+  /// Return the alignment (in bits) of the thrown exception object.
+  virtual unsigned getExnObjectAlignment() const {
+    /// Itanium says that an _Unwind_Exception has to be "double-word"
+    /// aligned (and thus the end of it is also so-aligned), meaning 16
+    /// bytes.  Of course, that was written for the actual Itanium,
+    /// which is a 64-bit platform.  Classically, the ABI doesn't really
+    /// specify the alignment on other platforms, but in practice
+    /// libUnwind declares the struct with __attribute__((aligned)), so
+    /// we assume that alignment here.  (It's generally 16 bytes, but
+    /// some targets overwrite it.)
+    return getDefaultAlignForAttributeAligned();
+  }
+
   /// \brief Return the size of intmax_t and uintmax_t for this target, in bits.
   unsigned getIntMaxTWidth() const {
     return getTypeWidth(IntMaxType);
   }
 
   // Return the size of unwind_word for this target.
   virtual unsigned getUnwindWordWidth() const { return getPointerWidth(0); }
 

cfe/trunk/lib/Basic/Targets.cpp

public:
   }
 
   /// Darwin does not support protected visibility.  Darwin's "default"
   /// is very similar to ELF's "protected";  Darwin requires a "weak"
   /// attribute on declarations that can be dynamically replaced.
   bool hasProtectedVisibility() const override {
     return false;
   }
+
+  unsigned getExnObjectAlignment() const override {
+    // The alignment of an exception object is 8-bytes for darwin since
+    // libc++abi doesn't declare _Unwind_Exception with __attribute__((aligned))
+    // and therefore doesn't guarantee 16-byte alignment.
+    return  64;
+  }
 };
 
 
 // DragonFlyBSD Target
 template<typename Target>
 class DragonFlyBSDTargetInfo : public OSTargetInfo<Target> {
 protected:
   void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,

cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp

public:
   llvm::Value *EmitMemberPointerIsNotNull(CodeGenFunction &CGF,
                                          llvm::Value *Addr,
                                          const MemberPointerType *MPT) override;
 
   void emitVirtualObjectDelete(CodeGenFunction &CGF, const CXXDeleteExpr *DE,
                                Address Ptr, QualType ElementType,
                                const CXXDestructorDecl *Dtor) override;
 
-  /// Itanium says that an _Unwind_Exception has to be "double-word"
-  /// aligned (and thus the end of it is also so-aligned), meaning 16
-  /// bytes.  Of course, that was written for the actual Itanium,
-  /// which is a 64-bit platform.  Classically, the ABI doesn't really
-  /// specify the alignment on other platforms, but in practice
-  /// libUnwind declares the struct with __attribute__((aligned)), so
-  /// we assume that alignment here.  (It's generally 16 bytes, but
-  /// some targets overwrite it.)
   CharUnits getAlignmentOfExnObject() {
-    auto align = CGM.getContext().getTargetDefaultAlignForAttributeAligned();
-    return CGM.getContext().toCharUnitsFromBits(align);
+    unsigned Align = CGM.getContext().getTargetInfo().getExnObjectAlignment();
+    return CGM.getContext().toCharUnitsFromBits(Align);
   }
 
   void emitRethrow(CodeGenFunction &CGF, bool isNoReturn) override;
   void emitThrow(CodeGenFunction &CGF, const CXXThrowExpr *E) override;
 
   void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C) override;
 
   llvm::CallInst *

cfe/trunk/test/CodeGenCXX/eh.cpp

void bar() {
     // CHECK:      [[T0:%.*]] = load i1, i1* [[EXN_ACTIVE]]
     // CHECK-NEXT: br i1 [[T0]]
     // CHECK:      [[T1:%.*]] = load i8*, i8** [[EXN_SAVE]]
     // CHECK-NEXT: call void @__cxa_free_exception(i8* [[T1]])
     // CHECK-NEXT: br label
   }
 }
 
+namespace test17 {
+class BaseException {
+private:
+  int a[4];
+public:
+  BaseException() {};
+};
+
+class DerivedException: public BaseException {
+};
+
+int foo() {
+  throw DerivedException();
+  // The alignment passed to memset is 8, not 16, on Darwin.
+
+  // CHECK: [[T0:%.*]] = call i8* @__cxa_allocate_exception(i64 16)
+  // CHECK-NEXT: [[T1:%.*]] = bitcast i8* [[T0]] to %"class.test17::DerivedException"*
+  // CHECK-NEXT: [[T2:%.*]] = bitcast %"class.test17::DerivedException"* [[T1]] to i8*
+  // CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[T2]], i8 0, i64 16, i32 8, i1 false)
+}
+}
+
 // CHECK: attributes [[NUW]] = { nounwind }
 // CHECK: attributes [[NR]] = { noreturn }