Assume `__cxa_allocate_exception` returns an under-aligned memory on Darwin if the version of libc++abi isn't new enough to include the fix in r319123

[ahatanak]

Summary

This patch resurrects r264998, which was committed to work around a bug in libc++abi that was causing _cxa_allocate_exception to return a memory that wasn't double-word aligned.

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160328/154332.html

I reverted r264998 after fixing the bug in libc++abi in r319123 (see the link below), but didn't really realize that doing so could break projects using older versions of libc++abi that don't have the fix committed in r319123.

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20171127/210878.html

In addition, this patch makes clang issue a warning if the type of the thrown object requires an alignment that is larger than the minimum alignment the libc++abi runtime guarantees.

rdar://problem/49864414

[ldionne]

include/clang/Basic/AlignedExceptionObject.h

15 ↗

(On Diff #198601)

The guards look wrong.

31 ↗

(On Diff #198601)

Would it make more sense to return the alignment directly here?

[rjmccall]

include/clang/Basic/AlignedExceptionObject.h

31 ↗

(On Diff #198601)

I agree: this should be a function returning the minimum expected exception alignment on the current target. Also, why does this have its own header instead of being, say, a function on TargetInfo?

[rjmccall]

include/clang/Basic/AlignedExceptionObject.h

31 ↗

(On Diff #198601)

Or ASTContext or something similar.

[ahatanak]

Move the code that was in AlignedExceptionObject.h to getExnObjectAlignment and remove the header.

[ahatanak]

include/clang/Basic/AlignedExceptionObject.h

31 ↗

(On Diff #198601)

I moved the code in alignedExceptionObjectMinVersion to getExnObjectAlignment and deleted the header.

[rjmccall]

include/clang/Basic/DiagnosticGroups.td

411

Why is this name so clipped?

include/clang/Basic/DiagnosticSemaKinds.td

6584

Hyphens seem wrong here, and you're missing some words, and it shouldn't be capitalized. How about:

"required alignment of type %0 (%1 bytes) is larger than the supported alignment of C++ exception objects on this target (%2 bytes)"

include/clang/Basic/TargetInfo.h

643

CharUnits?

lib/Sema/SemaExprCXX.cpp

946

Why is this a Darwin-specific check? Seems like it ought to be an Itanium-specific check, but it's equally applicable on Linux to highly-aligned types.

[ahatanak]

Address review comments.

[ahatanak]

include/clang/Basic/TargetInfo.h

643

I added a function to ASTContext that returns CharUnits.

lib/Sema/SemaExprCXX.cpp

946

Except when the C++ ABI is Microsoft, this is always diagnosed.

[rjmccall]

lib/Sema/SemaExprCXX.cpp

946

Thanks. Please don't refer to libc++abi in this comment, though, since this is a general Itanium issue and not specific to any particular implementation. How about something like:

Under the Itanium C++ ABI, memory for the exception object is allocated by the runtime with no ability for the compiler to request additional alignment.  Warn if the exception type requires alignment beyond the minimum guaranteed by the target C++ runtime.

[ahatanak]

Update the comment in CheckCXXThrowOperand.

[rjmccall]

Thanks, LGTM.

include/clang/AST/ASTContext.h

public:
   /// alignment.
   ///
   /// If \p ForAlignof, references are treated like their underlying type
   /// and  large arrays don't get any special treatment. If not \p ForAlignof
   /// it computes the value expected by CodeGen: references are treated like
   /// pointers and large arrays get extra alignment.
   CharUnits getDeclAlign(const Decl *D, bool ForAlignof = false) const;
 
+  /// Return the alignment (in bytes) of the thrown exception object. This is
+  /// only meaningful for targets that allocate C++ exceptions in a system
+  /// runtime, such as those using the Itanium C++ ABI.
+  CharUnits getExnObjectAlignment() const {
+    return toCharUnitsFromBits(Target->getExnObjectAlignment());
+  }
+
   /// Get or compute information about the layout of the specified
   /// record (struct/union/class) \p D, which indicates its size and field
   /// position information.
   const ASTRecordLayout &getASTRecordLayout(const RecordDecl *D) const;
 
   /// Get or compute information about the layout of the specified
   /// Objective-C interface.
   const ASTRecordLayout &getASTObjCInterfaceLayout(const ObjCInterfaceDecl *D)

include/clang/Basic/DiagnosticGroups.td

 def ObjCInvalidIBOutletProperty : DiagGroup<"invalid-iboutlet">;
 def ObjCRootClass : DiagGroup<"objc-root-class">;
 def ObjCPointerIntrospectPerformSelector : DiagGroup<"deprecated-objc-pointer-introspection-performSelector">;
 def ObjCPointerIntrospect : DiagGroup<"deprecated-objc-pointer-introspection", [ObjCPointerIntrospectPerformSelector]>;
 def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">;
 def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">;
 def ObjCBoxing : DiagGroup<"objc-boxing">;
 def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">;
+def UnderalignedExceptionObject : DiagGroup<"underaligned-exception-object">;
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
 def Packed : DiagGroup<"packed">;
 def Padded : DiagGroup<"padded">;
 
 def PessimizingMove : DiagGroup<"pessimizing-move">;
 def ReturnStdMoveInCXX11 : DiagGroup<"return-std-move-in-c++11">;
 def ReturnStdMove : DiagGroup<"return-std-move">;

include/clang/Basic/DiagnosticSemaKinds.td

 def err_conditional_vector_element_size : Error<
   "vector condition type %0 and result type %1 do not have elements of the "
   "same size">;
 
 def err_throw_incomplete : Error<
   "cannot throw object of incomplete type %0">;
 def err_throw_incomplete_ptr : Error<
   "cannot throw pointer to object of incomplete type %0">;
+def warn_throw_underaligned_obj : Warning<
+  "underaligned exception object thrown">,
+  InGroup<UnderalignedExceptionObject>;
+def note_throw_underaligned_obj : Note<
+  "required alignment of type %0 (%1 bytes) is larger than the supported "
+  "alignment of C++ exception objects on this target (%2 bytes)">;
 def err_return_in_constructor_handler : Error<
   "return in the catch of a function try block of a constructor is illegal">;
 def warn_cdtor_function_try_handler_mem_expr : Warning<
   "cannot refer to a non-static member from the handler of a "
   "%select{constructor|destructor}0 function try block">, InGroup<Exceptions>;
 
 let CategoryName = "Lambda Issue" in {
   def err_capture_more_than_once : Error<

include/clang/Basic/TargetInfo.h

public:
 
   /// Return the maximum vector alignment supported for the given target.
   unsigned getMaxVectorAlign() const { return MaxVectorAlign; }
   /// 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. This is
+  /// only meaningful for targets that allocate C++ exceptions in a system
+  /// runtime, such as those using the Itanium C++ ABI.
+  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();
+  }
+
   /// 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); }
 

lib/Basic/Targets/OSTargets.h

const char *getStaticInitSectionSpecifier() const override {
     return "__TEXT,__StaticInit,regular,pure_instructions";
   }
 
   /// 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 {
+    // Older versions of libc++abi guarantee an alignment of only 8-bytes for
+    // exception objects because of a bug in __cxa_exception that was
+    // eventually fixed in r319123.
+    llvm::VersionTuple MinVersion;
+    const llvm::Triple &T = this->getTriple();
+
+    // Compute the earliest OS versions that have the fix to libc++abi.
+    switch (T.getOS()) {
+    case llvm::Triple::Darwin:
+    case llvm::Triple::MacOSX: // Earliest supporting version is 10.14.
+      MinVersion = llvm::VersionTuple(10U, 14U);
+      break;
+    case llvm::Triple::IOS:
+    case llvm::Triple::TvOS: // Earliest supporting version is 12.0.0.
+      MinVersion = llvm::VersionTuple(12U);
+      break;
+    case llvm::Triple::WatchOS: // Earliest supporting version is 5.0.0.
+      MinVersion = llvm::VersionTuple(5U);
+      break;
+    default:
+      llvm_unreachable("Unexpected OS");
+    }
+
+    unsigned Major, Minor, Micro;
+    T.getOSVersion(Major, Minor, Micro);
+    if (llvm::VersionTuple(Major, Minor, Micro) < MinVersion)
+      return 64;
+    return OSTargetInfo<Target>::getExnObjectAlignment();
+  }
+
   TargetInfo::IntType getLeastIntTypeByWidth(unsigned BitWidth,
                                              bool IsSigned) const final {
     // Darwin uses `long long` for `int_least64_t` and `int_fast64_t`.
     return BitWidth == 64
                ? (IsSigned ? TargetInfo::SignedLongLong
                            : TargetInfo::UnsignedLongLong)
                : TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned);
   }

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);
-  }
-
   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 *
   emitTerminateForUnexpectedException(CodeGenFunction &CGF,
                                       llvm::Value *Exn) override;
void ItaniumCXXABI::emitThrow(CodeGenFunction &CGF, const CXXThrowExpr *E) {
   // Now allocate the exception object.
   llvm::Type *SizeTy = CGF.ConvertType(getContext().getSizeType());
   uint64_t TypeSize = getContext().getTypeSizeInChars(ThrowType).getQuantity();
 
   llvm::FunctionCallee AllocExceptionFn = getAllocateExceptionFn(CGM);
   llvm::CallInst *ExceptionPtr = CGF.EmitNounwindRuntimeCall(
       AllocExceptionFn, llvm::ConstantInt::get(SizeTy, TypeSize), "exception");
 
-  CharUnits ExnAlign = getAlignmentOfExnObject();
+  CharUnits ExnAlign = CGF.getContext().getExnObjectAlignment();
   CGF.EmitAnyExprToExn(E->getSubExpr(), Address(ExceptionPtr, ExnAlign));
 
   // Now throw the exception.
   llvm::Constant *TypeInfo = CGM.GetAddrOfRTTIDescriptor(ThrowType,
                                                          /*ForEH=*/true);
 
   // The address of the destructor.  If the exception type has a
   // trivial destructor (or isn't a record), we just pass null.

lib/Sema/SemaExprCXX.cpp

for (CXXRecordDecl *Subobject : UnambiguousPublicSubobjects) {
       // we must rebuild them here.
       for (unsigned I = 1, E = CD->getNumParams(); I != E; ++I) {
         if (CheckCXXDefaultArgExpr(ThrowLoc, CD, CD->getParamDecl(I)))
           return true;
       }
     }
   }
 
+  // Under the Itanium C++ ABI, memory for the exception object is allocated by
+  // the runtime with no ability for the compiler to request additional
+  // alignment. Warn if the exception type requires alignment beyond the minimum
+  // guaranteed by the target C++ runtime.
+  if (Context.getTargetInfo().getCXXABI().isItaniumFamily()) {
+    CharUnits TypeAlign = Context.getTypeAlignInChars(Ty);
+    CharUnits ExnObjAlign = Context.getExnObjectAlignment();
+    if (ExnObjAlign < TypeAlign) {
+      Diag(ThrowLoc, diag::warn_throw_underaligned_obj);
+      Diag(ThrowLoc, diag::note_throw_underaligned_obj)
+          << Ty << (unsigned)TypeAlign.getQuantity()
+          << (unsigned)ExnObjAlign.getQuantity();
+    }
+  }
+
   return false;
 }
 
 static QualType adjustCVQualifiersForCXXThisWithinLambda(
     ArrayRef<FunctionScopeInfo *> FunctionScopes, QualType ThisTy,
     DeclContext *CurSemaContext, ASTContext &ASTCtx) {
 
   QualType ClassType = ThisTy->getPointeeType();

test/CodeGenCXX/eh.cpp

-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -triple x86_64-apple-darwin -std=c++11 -emit-llvm %s -o - \
-// RUN:   | FileCheck %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -triple x86_64-apple-macosx10.13.99 -std=c++11 -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=UNALIGNED %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -triple x86_64-apple-macosx10.14 -std=c++11 -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=ALIGNED %s
 
 struct test1_D {
   double d;
 } d1;
 
 void test1() {
   throw d1;
 }
 
 // CHECK-LABEL:     define void @_Z5test1v()
 // CHECK:       [[EXNOBJ:%.*]] = call i8* @__cxa_allocate_exception(i64 8)
 // CHECK-NEXT:  [[EXN:%.*]] = bitcast i8* [[EXNOBJ]] to [[DSTAR:%[^*]*\*]]
 // CHECK-NEXT:  [[EXN2:%.*]] = bitcast [[DSTAR]] [[EXN]] to i8*
-// CHECK-NEXT:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[EXN2]], i8* align 8 bitcast ([[DSTAR]] @d1 to i8*), i64 8, i1 false)
+// UNALIGNED-NEXT:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 [[EXN2]], i8* align 8 bitcast ([[DSTAR]] @d1 to i8*), i64 8, i1 false)
+// ALIGNED-NEXT:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 [[EXN2]], i8* align 8 bitcast ([[DSTAR]] @d1 to i8*), i64 8, i1 false)
 // CHECK-NEXT:  call void @__cxa_throw(i8* [[EXNOBJ]], i8* bitcast ({ i8*, i8* }* @_ZTI7test1_D to i8*), i8* null) [[NR:#[0-9]+]]
 // CHECK-NEXT:  unreachable
 
 
 struct test2_D {
   test2_D(const test2_D&o);
   test2_D();
   virtual void bar() { }

 
 int foo() {
   throw DerivedException();
   // The alignment passed to memset is 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* align 16 [[T2]], i8 0, i64 16, i1 false)
+  // UNALIGNED-NEXT: call void @llvm.memset.p0i8.i64(i8* align 8 [[T2]], i8 0, i64 16, i1 false)
+  // ALIGNED-NEXT: call void @llvm.memset.p0i8.i64(i8* align 16 [[T2]], i8 0, i64 16, i1 false)
 }
 }
 
 // CHECK: attributes [[NUW]] = { nounwind }
 // CHECK: attributes [[NR]] = { noreturn }

test/SemaCXX/warn-overaligned-type-thrown.cpp

+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.99 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple arm64-apple-ios10 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple arm64-apple-tvos10 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple arm64-apple-watchos4 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DUNDERALIGNED %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple arm64-apple-ios12 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple arm64-apple-tvos12 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple arm64-apple-watchos5 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple arm-linux-gnueabi -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnueabi -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple mipsel-linux-gnu -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple mips64el-linux-gnu -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple wasm64-unknown-unknown -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14 -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -Wno-underaligned-exception-object -DNODIAG %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -verify -fsyntax-only -std=c++11 -fcxx-exceptions -fexceptions -DNODIAG %s
+
+struct S0 {
+  S0();
+  int m;
+};
+
+struct Overaligned1 {
+  Overaligned1();
+  int __attribute__((aligned(16))) m;
+};
+
+struct __attribute__((aligned(16))) Overaligned2 {
+  Overaligned2();
+  int m;
+};
+
+struct Overaligned3 {
+  Overaligned3();
+  int __attribute__((aligned(64))) m;
+};
+
+void test0() {
+  throw S0();
+}
+
+void test1() {
+  throw Overaligned1();
+}
+
+void test2() {
+  throw Overaligned2();
+}
+
+void test3() {
+  throw Overaligned3();
+}
+
+#if defined(NODIAG)
+// expected-no-diagnostics
+#elif defined(UNDERALIGNED)
+// expected-warning@-14 {{underaligned exception object thrown}}
+// expected-note@-15 {{(16 bytes) is larger than the supported alignment of C++ exception objects on this target (8 bytes)}}
+// expected-warning@-12 {{underaligned exception object thrown}}
+// expected-note@-13 {{(16 bytes) is larger than the supported alignment of C++ exception objects on this target (8 bytes)}}
+// expected-warning@-10 {{underaligned exception object thrown}}
+// expected-note@-11 {{(64 bytes) is larger than the supported alignment of C++ exception objects on this target (8 bytes)}}
+#else
+// expected-warning@-13 {{underaligned exception object thrown}}
+// expected-note@-14 {{(64 bytes) is larger than the supported alignment of C++ exception objects on this target (16 bytes)}}
+#endif