From 5c1f1d09c87a1aad95973c8c8ffa2733329b397e Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 29 Jan 2013 01:14:22 +0000 Subject: [PATCH] Abstract the behavior of when to use base-class tail padding. For fun, I added a comedy "actually obey the C++11 POD rules" option which nobody is allowed to use. llvm-svn: 173744 --- clang/include/clang/AST/DeclCXX.h | 2 ++ clang/include/clang/Basic/TargetCXXABI.h | 49 +++++++++++++++++++++++++++ clang/lib/AST/RecordLayoutBuilder.cpp | 57 +++++++++++++++++++++++++++----- 3 files changed, 100 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 3edb583..1a37bd6 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -1046,6 +1046,8 @@ public: /// that is an aggregate that has no non-static non-POD data members, no /// reference data members, no user-defined copy assignment operator and no /// user-defined destructor. + /// + /// Note that this is the C++ TR1 definition of POD. bool isPOD() const { return data().PlainOldData; } /// \brief True if this class is C-like, without C++-specific features, e.g. diff --git a/clang/include/clang/Basic/TargetCXXABI.h b/clang/include/clang/Basic/TargetCXXABI.h index 1a8e62c..3c6677c 100644 --- a/clang/include/clang/Basic/TargetCXXABI.h +++ b/clang/include/clang/Basic/TargetCXXABI.h @@ -183,6 +183,55 @@ public: llvm_unreachable("bad ABI kind"); } + /// When is record layout allowed to allocate objects in the tail + /// padding of a base class? + /// + /// This decision cannot be changed without breaking platform ABI + /// compatibility, and yet it is tied to language guarantees which + /// the committee has so far seen fit to strengthen no less than + /// three separate times: + /// - originally, there were no restrictions at all; + /// - C++98 declared that objects could not be allocated in the + /// tail padding of a POD type; + /// - C++03 extended the definition of POD to include classes + /// containing member pointers; and + /// - C++11 greatly broadened the definition of POD to include + /// all trivial standard-layout classes. + /// Each of these changes technically took several existing + /// platforms and made them permanently non-conformant. + enum TailPaddingUseRules { + /// The tail-padding of a base class is always theoretically + /// available, even if it's POD. This is not strictly conforming + /// in any language mode. + AlwaysUseTailPadding, + + /// Only allocate objects in the tail padding of a base class if + /// the base class is not POD according to the rules of C++ TR1. + /// This is non strictly conforming in C++11 mode. + UseTailPaddingUnlessPOD03, + + /// Only allocate objects in the tail padding of a base class if + /// the base class is not POD according to the rules of C++11. + UseTailPaddingUnlessPOD11 + }; + TailPaddingUseRules getTailPaddingUseRules() const { + switch (getKind()) { + // To preserve binary compatibility, the generic Itanium ABI has + // permanently locked the definition of POD to the rules of C++ TR1, + // and that trickles down to all the derived ABIs. + case GenericItanium: + case GenericARM: + case iOS: + return UseTailPaddingUnlessPOD03; + + // MSVC always allocates fields in the tail-padding of a base class + // subobject, even if they're POD. + case Microsoft: + return AlwaysUseTailPadding; + } + llvm_unreachable("bad ABI kind"); + } + /// Try to parse an ABI name, returning false on error. bool tryParse(llvm::StringRef name); diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 72851df..42c3ba3 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2411,6 +2411,48 @@ RecordLayoutBuilder::Diag(SourceLocation Loc, unsigned DiagID) { return Context.getDiagnostics().Report(Loc, DiagID); } +/// Does the target C++ ABI require us to skip over the tail-padding +/// of the given class (considering it as a base class) when allocating +/// objects? +static bool mustSkipTailPadding(TargetCXXABI ABI, const CXXRecordDecl *RD) { + switch (ABI.getTailPaddingUseRules()) { + case TargetCXXABI::AlwaysUseTailPadding: + return false; + + case TargetCXXABI::UseTailPaddingUnlessPOD03: + // FIXME: To the extent that this is meant to cover the Itanium ABI + // rules, we should implement the restrictions about over-sized + // bitfields: + // + // http://mentorembedded.github.com/cxx-abi/abi.html#POD : + // In general, a type is considered a POD for the purposes of + // layout if it is a POD type (in the sense of ISO C++ + // [basic.types]). However, a POD-struct or POD-union (in the + // sense of ISO C++ [class]) with a bitfield member whose + // declared width is wider than the declared type of the + // bitfield is not a POD for the purpose of layout. Similarly, + // an array type is not a POD for the purpose of layout if the + // element type of the array is not a POD for the purpose of + // layout. + // + // Where references to the ISO C++ are made in this paragraph, + // the Technical Corrigendum 1 version of the standard is + // intended. + return RD->isPOD(); + + case TargetCXXABI::UseTailPaddingUnlessPOD11: + // This is equivalent to RD->getTypeForDecl().isCXX11PODType(), + // but with a lot of abstraction penalty stripped off. This does + // assume that these properties are set correctly even in C++98 + // mode; fortunately, that is true because we want to assign + // consistently semantics to the type-traits intrinsics (or at + // least as many of them as possible). + return RD->isTrivial() && RD->isStandardLayout(); + } + + llvm_unreachable("bad tail-padding use kind"); +} + /// getASTRecordLayout - Get or compute information about the layout of the /// specified record (struct/union/class), which indicates its size and field /// position information. @@ -2455,18 +2497,17 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const { Builder.Layout(RD); } - // FIXME: This is not always correct. See the part about bitfields at - // http://www.codesourcery.com/public/cxx-abi/abi.html#POD for more info. - // FIXME: IsPODForThePurposeOfLayout should be stored in the record layout. - // This does not affect the calculations of MSVC layouts - bool IsPODForThePurposeOfLayout = - (!Builder.isMicrosoftCXXABI() && cast(D)->isPOD()); + // In certain situations, we are allowed to lay out objects in the + // tail-padding of base classes. This is ABI-dependent. + // FIXME: this should be stored in the record layout. + bool skipTailPadding = + mustSkipTailPadding(getTargetInfo().getCXXABI(), cast(D)); // FIXME: This should be done in FinalizeLayout. CharUnits DataSize = - IsPODForThePurposeOfLayout ? Builder.getSize() : Builder.getDataSize(); + skipTailPadding ? Builder.getSize() : Builder.getDataSize(); CharUnits NonVirtualSize = - IsPODForThePurposeOfLayout ? DataSize : Builder.NonVirtualSize; + skipTailPadding ? DataSize : Builder.NonVirtualSize; NewEntry = new (*this) ASTRecordLayout(*this, Builder.getSize(), -- 2.7.4