diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 9802370a6056..12bf16372f71 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -175,6 +175,7 @@ def InfiniteRecursion : DiagGroup<"infinite-recursion">; def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">; def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">; def : DiagGroup<"import">; +def IncompatibleMSStruct : DiagGroup<"incompatible-ms-struct">; def IncompatiblePointerTypesDiscardsQualifiers : DiagGroup<"incompatible-pointer-types-discards-qualifiers">; def IncompatiblePointerTypes diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 059a28ab88da..6e1484c1ca82 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -500,8 +500,10 @@ def warn_pragma_pack_pop_identifer_and_alignment : Warning< "specifying both a name and alignment to 'pop' is undefined">; def warn_pragma_pop_failed : Warning<"#pragma %0(pop, ...) failed: %1">, InGroup; -def warn_pragma_ms_struct_failed : Warning<"#pramga ms_struct can not be used with dynamic classes or structures">, - InGroup; +def warn_cxx_ms_struct : + Warning<"ms_struct may not produce MSVC-compatible layouts for classes " + "with base classes or virtual functions">, + DefaultError, InGroup; def warn_pragma_unused_undeclared_var : Warning< "undeclared variable %0 used as an argument for '#pragma unused'">, diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index d27255ecc8de..11835c901cbb 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -1502,6 +1502,10 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { // __attribute__((packed))) always uses the next available bit // offset. + // In an ms_struct struct, the alignment of a fundamental type is + // always equal to its size. This is necessary in order to mimic + // the i386 alignment rules on targets which might not fully align + // all types (e.g. Darwin PPC32, where alignof(long long) == 4). // First, some simple bookkeeping to perform for ms_struct structs. if (IsMsStruct) { diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index b9c7cca326a6..74ab1ee84990 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -4520,11 +4520,18 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) { } } - // Check to see if we're trying to lay out a struct using the ms_struct - // attribute that is dynamic. - if (Record->isMsStruct(Context) && Record->isDynamicClass()) { - Diag(Record->getLocation(), diag::warn_pragma_ms_struct_failed); - Record->dropAttr(); + // ms_struct is a request to use the same ABI rules as MSVC. Check + // whether this class uses any C++ features that are implemented + // completely differently in MSVC, and if so, emit a diagnostic. + // That diagnostic defaults to an error, but we allow projects to + // map it down to a warning (or ignore it). It's a fairly common + // practice among users of the ms_struct pragma to mass-annotate + // headers, sweeping up a bunch of types that the project doesn't + // really rely on MSVC-compatible layout for. We must therefore + // support "ms_struct except for C++ stuff" as a secondary ABI. + if (Record->isMsStruct(Context) && + (Record->isPolymorphic() || Record->getNumBases())) { + Diag(Record->getLocation(), diag::warn_cxx_ms_struct); } // Declare inheriting constructors. We do this eagerly here because: diff --git a/clang/test/SemaCXX/ms_struct.cpp b/clang/test/SemaCXX/ms_struct.cpp index 37fa9a7c687c..2832b5620f3b 100644 --- a/clang/test/SemaCXX/ms_struct.cpp +++ b/clang/test/SemaCXX/ms_struct.cpp @@ -1,5 +1,6 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-apple-darwin9 -std=c++11 %s -// expected-no-diagnostics +// RUN: %clang_cc1 -fsyntax-only -Wno-error=incompatible-ms-struct -verify -triple i686-apple-darwin9 -std=c++11 %s +// RUN: %clang_cc1 -fsyntax-only -Wno-error=incompatible-ms-struct -verify -triple armv7-apple-darwin9 -std=c++11 %s +// RUN: %clang_cc1 -fsyntax-only -DTEST_FOR_ERROR -verify -triple armv7-apple-darwin9 -std=c++11 %s #pragma ms_struct on @@ -9,10 +10,29 @@ struct A { }; struct B : public A { +#ifdef TEST_FOR_ERROR + // expected-error@-2 {{ms_struct may not produce MSVC-compatible layouts for classes with base classes or virtual functions}} +#else + // expected-warning@-4 {{ms_struct may not produce MSVC-compatible layouts for classes with base classes or virtual functions}} +#endif unsigned long c:16; int d; B(); }; static_assert(__builtin_offsetof(B, d) == 12, - "We can't allocate the bitfield into the padding under ms_struct"); \ No newline at end of file + "We can't allocate the bitfield into the padding under ms_struct"); + +// rdar://16178895 +struct C { +#ifdef TEST_FOR_ERROR + // expected-error@-2 {{ms_struct may not produce MSVC-compatible layouts for classes with base classes or virtual functions}} +#else + // expected-warning@-4 {{ms_struct may not produce MSVC-compatible layouts for classes with base classes or virtual functions}} +#endif + virtual void foo(); + long long n; +}; + +static_assert(__builtin_offsetof(C, n) == 8, + "long long field in ms_struct should be 8-byte aligned");