r202427 - Diagnose attempts to apply ms_struct to records with base classes
John McCall
rjmccall at apple.com
Thu Feb 27 12:30:49 PST 2014
Author: rjmccall
Date: Thu Feb 27 14:30:49 2014
New Revision: 202427
URL: http://llvm.org/viewvc/llvm-project?rev=202427&view=rev
Log:
Diagnose attempts to apply ms_struct to records with base classes
or virtual functions, but permit that error to be downgraded to
a warning (with -Wno-error=incompatible-ms-struct), and officially
support this kind of dual, ABI-mixing layout.
The basic problem here is that projects which use ms_struct are often
not very circumspect about what types they annotate; for example,
some projects enable the pragma in a prefix header and then only
selectively disable it around system header inclusions. They may
only care about binary compatibility with MSVC for a subset of
those structs, but that doesn't mean they have no binary
compatibility concerns at all for the rest; thus we are essentially
forced into supporting this hybrid ABI. But it's reasonable for
us to at least point out the places where we're not making
any guarantees.
The original diagnostic was for dynamic classes, i.e. those with
virtual functions or virtual bases; I've extended it to include
all classes with bases, because we are not actually making any
attempt to duplicate MSVC's base subobject layout in ms_struct
(and it is indeed quite different from Itanium, even for
non-virtual bases).
rdar://16178895
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/SemaCXX/ms_struct.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=202427&r1=202426&r2=202427&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Feb 27 14:30:49 2014
@@ -175,6 +175,7 @@ def InfiniteRecursion : DiagGroup<"infin
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
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=202427&r1=202426&r2=202427&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Feb 27 14:30:49 2014
@@ -500,8 +500,10 @@ def warn_pragma_pack_pop_identifer_and_a
"specifying both a name and alignment to 'pop' is undefined">;
def warn_pragma_pop_failed : Warning<"#pragma %0(pop, ...) failed: %1">,
InGroup<IgnoredPragmas>;
-def warn_pragma_ms_struct_failed : Warning<"#pramga ms_struct can not be used with dynamic classes or structures">,
- InGroup<IgnoredPragmas>;
+def warn_cxx_ms_struct :
+ Warning<"ms_struct may not produce MSVC-compatible layouts for classes "
+ "with base classes or virtual functions">,
+ DefaultError, InGroup<IncompatibleMSStruct>;
def warn_pragma_unused_undeclared_var : Warning<
"undeclared variable %0 used as an argument for '#pragma unused'">,
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=202427&r1=202426&r2=202427&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Feb 27 14:30:49 2014
@@ -1502,6 +1502,10 @@ void RecordLayoutBuilder::LayoutBitField
// __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) {
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=202427&r1=202426&r2=202427&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Feb 27 14:30:49 2014
@@ -4520,11 +4520,18 @@ void Sema::CheckCompletedCXXClass(CXXRec
}
}
- // 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<MsStructAttr>();
+ // 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:
Modified: cfe/trunk/test/SemaCXX/ms_struct.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms_struct.cpp?rev=202427&r1=202426&r2=202427&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/ms_struct.cpp (original)
+++ cfe/trunk/test/SemaCXX/ms_struct.cpp Thu Feb 27 14:30:49 2014
@@ -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 at -2 {{ms_struct may not produce MSVC-compatible layouts for classes with base classes or virtual functions}}
+#else
+ // expected-warning at -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 at -2 {{ms_struct may not produce MSVC-compatible layouts for classes with base classes or virtual functions}}
+#else
+ // expected-warning at -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");
More information about the cfe-commits
mailing list