Author: ahatanak Date: Thu Feb 7 12:21:46 2019 New Revision: 353459 URL: http://llvm.org/viewvc/llvm-project?rev=353459&view=rev Log: [Sema][ObjC] Disallow non-trivial C struct fields in unions.
This patch fixes a bug where clang doesn’t reject union fields of non-trivial C struct types. For example: ``` // This struct is non-trivial under ARC. struct S0 { id x; }; union U0 { struct S0 s0; // clang should reject this. struct S0 s1; // clang should reject this. }; void test(union U0 a) { // Previously, both 'a.s0.x' and 'a.s1.x' were released in this // function. } ``` rdar://problem/46677858 Differential Revision: https://reviews.llvm.org/D55659 Modified: cfe/trunk/include/clang/AST/Type.h cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaObjC/arc-decls.m Modified: cfe/trunk/include/clang/AST/Type.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=353459&r1=353458&r2=353459&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Type.h (original) +++ cfe/trunk/include/clang/AST/Type.h Thu Feb 7 12:21:46 2019 @@ -1121,6 +1121,12 @@ public: }; /// Check if this is a non-trivial type that would cause a C struct + /// transitively containing this type to be non-trivial. This function can be + /// used to determine whether a field of this type can be declared inside a C + /// union. + bool isNonTrivialPrimitiveCType(const ASTContext &Ctx) const; + + /// Check if this is a non-trivial type that would cause a C struct /// transitively containing this type to be non-trivial to copy and return the /// kind. PrimitiveCopyKind isNonTrivialToPrimitiveCopy() const; Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=353459&r1=353458&r2=353459&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Feb 7 12:21:46 2019 @@ -609,6 +609,8 @@ def warn_cstruct_memaccess : Warning< InGroup<NonTrivialMemaccess>; def note_nontrivial_field : Note< "field is non-trivial to %select{copy|default-initialize}0">; +def err_nontrivial_primitive_type_in_union : Error< + "non-trivial C types are disallowed in union">; def warn_dyn_class_memaccess : Warning< "%select{destination for|source of|first operand of|second operand of}0 this " "%1 call is a pointer to %select{|class containing a }2dynamic class %3; " Modified: cfe/trunk/lib/AST/Type.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=353459&r1=353458&r2=353459&view=diff ============================================================================== --- cfe/trunk/lib/AST/Type.cpp (original) +++ cfe/trunk/lib/AST/Type.cpp Thu Feb 7 12:21:46 2019 @@ -22,6 +22,7 @@ #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/AST/NestedNameSpecifier.h" +#include "clang/AST/NonTrivialTypeVisitor.h" #include "clang/AST/PrettyPrinter.h" #include "clang/AST/TemplateBase.h" #include "clang/AST/TemplateName.h" @@ -2244,6 +2245,62 @@ bool QualType::isNonWeakInMRRWithObjCWea getObjCLifetime() != Qualifiers::OCL_Weak; } +namespace { +// Helper class that determines whether this is a type that is non-trivial to +// primitive copy or move, or is a struct type that has a field of such type. +template <bool IsMove> +struct IsNonTrivialCopyMoveVisitor + : CopiedTypeVisitor<IsNonTrivialCopyMoveVisitor<IsMove>, IsMove, bool> { + using Super = + CopiedTypeVisitor<IsNonTrivialCopyMoveVisitor<IsMove>, IsMove, bool>; + IsNonTrivialCopyMoveVisitor(const ASTContext &C) : Ctx(C) {} + void preVisit(QualType::PrimitiveCopyKind PCK, QualType QT) {} + + bool visitWithKind(QualType::PrimitiveCopyKind PCK, QualType QT) { + if (const auto *AT = this->Ctx.getAsArrayType(QT)) + return this->asDerived().visit(QualType(AT, 0)); + return Super::visitWithKind(PCK, QT); + } + + bool visitARCStrong(QualType QT) { return true; } + bool visitARCWeak(QualType QT) { return true; } + bool visitTrivial(QualType QT) { return false; } + // Volatile fields are considered trivial. + bool visitVolatileTrivial(QualType QT) { return false; } + + bool visitStruct(QualType QT) { + const RecordDecl *RD = QT->castAs<RecordType>()->getDecl(); + // We don't want to apply the C restriction in C++ because C++ + // (1) can apply the restriction at a finer grain by banning copying or + // destroying the union, and + // (2) allows users to override these restrictions by declaring explicit + // constructors/etc, which we're not proposing to add to C. + if (isa<CXXRecordDecl>(RD)) + return false; + for (const FieldDecl *FD : RD->fields()) + if (this->asDerived().visit(FD->getType())) + return true; + return false; + } + + const ASTContext &Ctx; +}; + +} // namespace + +bool QualType::isNonTrivialPrimitiveCType(const ASTContext &Ctx) const { + if (isNonTrivialToPrimitiveDefaultInitialize()) + return true; + DestructionKind DK = isDestructedType(); + if (DK != DK_none && DK != DK_cxx_destructor) + return true; + if (IsNonTrivialCopyMoveVisitor<false>(Ctx).visit(*this)) + return true; + if (IsNonTrivialCopyMoveVisitor<true>(Ctx).visit(*this)) + return true; + return false; +} + QualType::PrimitiveDefaultInitializeKind QualType::isNonTrivialToPrimitiveDefaultInitialize() const { if (const auto *RT = Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=353459&r1=353458&r2=353459&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Feb 7 12:21:46 2019 @@ -15916,6 +15916,10 @@ void Sema::ActOnFields(Scope *S, SourceL Record->setHasObjectMember(true); if (Record && FDTTy->getDecl()->hasVolatileMember()) Record->setHasVolatileMember(true); + if (Record && Record->isUnion() && + FD->getType().isNonTrivialPrimitiveCType(Context)) + Diag(FD->getLocation(), + diag::err_nontrivial_primitive_type_in_union); } else if (FDTy->isObjCObjectType()) { /// A field cannot be an Objective-c object Diag(FD->getLocation(), diag::err_statically_allocated_object) Modified: cfe/trunk/test/SemaObjC/arc-decls.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-decls.m?rev=353459&r1=353458&r2=353459&view=diff ============================================================================== --- cfe/trunk/test/SemaObjC/arc-decls.m (original) +++ cfe/trunk/test/SemaObjC/arc-decls.m Thu Feb 7 12:21:46 2019 @@ -3,13 +3,29 @@ // rdar://8843524 struct A { - id x; + id x[4]; + id y; }; union u { id u; // expected-error {{ARC forbids Objective-C objects in union}} }; +union u_nontrivial_c { + struct A a; // expected-error {{non-trivial C types are disallowed in union}} +}; + +// Volatile fields are fine. +struct C { + volatile int x[4]; + volatile int y; +}; + +union u_trivial_c { + volatile int b; + struct C c; +}; + @interface I { struct A a; struct B { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits