diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -523,6 +523,14 @@ /// semantically correspond to a bool. bool isKnownToHaveBooleanValue(bool Semantic = true) const; + /// Check whether this array fits the idiom of a flexible array member, + /// depending on the value of -fstrict-flex-array. + /// When IgnoreTemplateOrMacroSubstitution is set, it doesn't consider sizes + /// resulting from the substitution of a macro or a template as special sizes. + bool isFlexibleArrayMemberLike( + ASTContext &Context, unsigned StrictFlexArraysLevel, + bool IgnoreTemplateOrMacroSubstitution = false) const; + /// isIntegerConstantExpr - Return the value if this expression is a valid /// integer constant expression. If not a valid i-c-e, return None and fill /// in Loc (if specified) with the location of the invalid expression. diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -203,6 +203,77 @@ return false; } +bool Expr::isFlexibleArrayMemberLike( + ASTContext &Context, unsigned StrictFlexArraysLevel, + bool IgnoreTemplateOrMacroSubstitution) const { + + // For compatibility with existing code, we treat arrays of length 0 or + // 1 as flexible array members. + if (const auto *CAT = Context.getAsConstantArrayType(getType())) { + llvm::APInt Size = CAT->getSize(); + + // GCC extension, only allowed to represent a FAM. + if (Size == 0) + return true; + + // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing + // arrays to be treated as flexible-array-members, we still emit diagnostics + // as if they are not. Pending further discussion... + if (StrictFlexArraysLevel >= 2 || Size.uge(2)) + return false; + + } else if (!Context.getAsIncompleteArrayType(getType())) + return false; + + const Expr *E = IgnoreParens(); + + const NamedDecl *ND = nullptr; + if (const auto *DRE = dyn_cast(E)) + ND = DRE->getDecl(); + else if (const auto *ME = dyn_cast(E)) + ND = ME->getMemberDecl(); + else if (const auto *IRE = dyn_cast(E)) + return IRE->getDecl()->getNextIvar() == nullptr; + + if (!ND) + return false; + + // A flexible array member must be the last member in the class. + // FIXME: If the base type of the member expr is not FD->getParent(), + // this should not be treated as a flexible array member access. + if (const auto *FD = dyn_cast(ND)) { + if (FD->getParent()->isUnion()) + return true; + + // Don't consider sizes resulting from macro expansions or template argument + // substitution to form C89 tail-padded arrays. + if (IgnoreTemplateOrMacroSubstitution) { + TypeSourceInfo *TInfo = FD->getTypeSourceInfo(); + while (TInfo) { + TypeLoc TL = TInfo->getTypeLoc(); + // Look through typedefs. + if (TypedefTypeLoc TTL = TL.getAsAdjusted()) { + const TypedefNameDecl *TDL = TTL.getTypedefNameDecl(); + TInfo = TDL->getTypeSourceInfo(); + continue; + } + if (ConstantArrayTypeLoc CTL = TL.getAs()) { + const Expr *SizeExpr = dyn_cast(CTL.getSizeExpr()); + if (!SizeExpr || SizeExpr->getExprLoc().isMacroID()) + return false; + } + break; + } + } + + RecordDecl::field_iterator FI( + DeclContext::decl_iterator(const_cast(FD))); + return ++FI == FD->getParent()->field_end(); + } + + return false; +} + const ValueDecl * Expr::getAsBuiltinConstantDeclRef(const ASTContext &Context) const { Expr::EvalResult Eval; diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -875,50 +875,6 @@ } } -/// Determine whether this expression refers to a flexible array member in a -/// struct. We disable array bounds checks for such members. -static bool isFlexibleArrayMemberExpr(const Expr *E, - unsigned StrictFlexArraysLevel) { - // For compatibility with existing code, we treat arrays of length 0 or - // 1 as flexible array members. - // FIXME: This is inconsistent with the warning code in SemaChecking. Unify - // the two mechanisms. - const ArrayType *AT = E->getType()->castAsArrayTypeUnsafe(); - if (const auto *CAT = dyn_cast(AT)) { - // FIXME: Sema doesn't treat [1] as a flexible array member if the bound - // was produced by macro expansion. - if (StrictFlexArraysLevel >= 2 && CAT->getSize().ugt(0)) - return false; - // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing - // arrays to be treated as flexible-array-members, we still emit ubsan - // checks as if they are not. - if (CAT->getSize().ugt(1)) - return false; - } else if (!isa(AT)) - return false; - - E = E->IgnoreParens(); - - // A flexible array member must be the last member in the class. - if (const auto *ME = dyn_cast(E)) { - // FIXME: If the base type of the member expr is not FD->getParent(), - // this should not be treated as a flexible array member access. - if (const auto *FD = dyn_cast(ME->getMemberDecl())) { - // FIXME: Sema doesn't treat a T[1] union member as a flexible array - // member, only a T[0] or T[] member gets that treatment. - if (FD->getParent()->isUnion()) - return true; - RecordDecl::field_iterator FI( - DeclContext::decl_iterator(const_cast(FD))); - return ++FI == FD->getParent()->field_end(); - } - } else if (const auto *IRE = dyn_cast(E)) { - return IRE->getDecl()->getNextIvar() == nullptr; - } - - return false; -} - llvm::Value *CodeGenFunction::LoadPassedObjectSize(const Expr *E, QualType EltTy) { ASTContext &C = getContext(); @@ -974,7 +930,8 @@ if (const auto *CE = dyn_cast(Base)) { if (CE->getCastKind() == CK_ArrayToPointerDecay && - !isFlexibleArrayMemberExpr(CE->getSubExpr(), StrictFlexArraysLevel)) { + !CE->getSubExpr()->isFlexibleArrayMemberLike(CGF.getContext(), + StrictFlexArraysLevel)) { IndexedType = CE->getSubExpr()->getType(); const ArrayType *AT = IndexedType->castAsArrayTypeUnsafe(); if (const auto *CAT = dyn_cast(AT)) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -15898,72 +15898,6 @@ << TRange << Op->getSourceRange(); } -/// Check whether this array fits the idiom of a flexible array member, -/// depending on the value of -fstrict-flex-array. -/// -/// We avoid emitting out-of-bounds access warnings for such arrays. -static bool isFlexibleArrayMemberExpr(Sema &S, const Expr *E, - const NamedDecl *ND, - unsigned StrictFlexArraysLevel) { - - if (!ND) - return false; - - const ConstantArrayType *ArrayTy = - S.Context.getAsConstantArrayType(E->getType()); - llvm::APInt Size = ArrayTy->getSize(); - - if (Size == 0) - return true; - - // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing - // arrays to be treated as flexible-array-members, we still emit diagnostics - // as if they are not. Pending further discussion... - if (StrictFlexArraysLevel >= 2 || Size.uge(2)) - return false; - - const FieldDecl *FD = dyn_cast(ND); - if (!FD) - return false; - - // Don't consider sizes resulting from macro expansions or template argument - // substitution to form C89 tail-padded arrays. - - TypeSourceInfo *TInfo = FD->getTypeSourceInfo(); - while (TInfo) { - TypeLoc TL = TInfo->getTypeLoc(); - // Look through typedefs. - if (TypedefTypeLoc TTL = TL.getAsAdjusted()) { - const TypedefNameDecl *TDL = TTL.getTypedefNameDecl(); - TInfo = TDL->getTypeSourceInfo(); - continue; - } - if (ConstantArrayTypeLoc CTL = TL.getAs()) { - const Expr *SizeExpr = dyn_cast(CTL.getSizeExpr()); - if (!SizeExpr || SizeExpr->getExprLoc().isMacroID()) - return false; - } - break; - } - - const RecordDecl *RD = dyn_cast(FD->getDeclContext()); - if (!RD) - return false; - if (RD->isUnion()) - return false; - if (const CXXRecordDecl *CRD = dyn_cast(RD)) { - if (!CRD->isStandardLayout()) - return false; - } - - // See if this is the last field decl in the record. - const Decl *D = FD; - while ((D = D->getNextDeclInContext())) - if (isa(D)) - return false; - return true; -} - void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, const ArraySubscriptExpr *ASE, bool AllowOnePastEnd, bool IndexNegated) { @@ -15983,17 +15917,12 @@ unsigned StrictFlexArraysLevel = getLangOpts().StrictFlexArrays; - const NamedDecl *ND = nullptr; - if (const auto *DRE = dyn_cast(BaseExpr)) - ND = DRE->getDecl(); - else if (const auto *ME = dyn_cast(BaseExpr)) - ND = ME->getMemberDecl(); - const Type *BaseType = ArrayTy == nullptr ? nullptr : ArrayTy->getElementType().getTypePtr(); bool IsUnboundedArray = - BaseType == nullptr || - isFlexibleArrayMemberExpr(*this, BaseExpr, ND, StrictFlexArraysLevel); + BaseType == nullptr || BaseExpr->isFlexibleArrayMemberLike( + Context, StrictFlexArraysLevel, + /*IgnoreTemplateOrMacroSubstitution=*/true); if (EffectiveType->isDependentType() || (!IsUnboundedArray && BaseType->isDependentType())) return; @@ -16061,15 +15990,14 @@ << (unsigned)MaxElems.getLimitedValue(~0U) << IndexExpr->getSourceRange()); - if (!ND) { - // Try harder to find a NamedDecl to point at in the note. - while (const auto *ASE = dyn_cast(BaseExpr)) - BaseExpr = ASE->getBase()->IgnoreParenCasts(); - if (const auto *DRE = dyn_cast(BaseExpr)) - ND = DRE->getDecl(); - if (const auto *ME = dyn_cast(BaseExpr)) - ND = ME->getMemberDecl(); - } + const NamedDecl *ND = nullptr; + // Try harder to find a NamedDecl to point at in the note. + while (const auto *ASE = dyn_cast(BaseExpr)) + BaseExpr = ASE->getBase()->IgnoreParenCasts(); + if (const auto *DRE = dyn_cast(BaseExpr)) + ND = DRE->getDecl(); + if (const auto *ME = dyn_cast(BaseExpr)) + ND = ME->getMemberDecl(); if (ND) DiagRuntimeBehavior(ND->getBeginLoc(), BaseExpr, @@ -16152,15 +16080,14 @@ << IndexExpr->getSourceRange()); } - if (!ND) { - // Try harder to find a NamedDecl to point at in the note. - while (const auto *ASE = dyn_cast(BaseExpr)) - BaseExpr = ASE->getBase()->IgnoreParenCasts(); - if (const auto *DRE = dyn_cast(BaseExpr)) - ND = DRE->getDecl(); - if (const auto *ME = dyn_cast(BaseExpr)) - ND = ME->getMemberDecl(); - } + const NamedDecl *ND = nullptr; + // Try harder to find a NamedDecl to point at in the note. + while (const auto *ASE = dyn_cast(BaseExpr)) + BaseExpr = ASE->getBase()->IgnoreParenCasts(); + if (const auto *DRE = dyn_cast(BaseExpr)) + ND = DRE->getDecl(); + if (const auto *ME = dyn_cast(BaseExpr)) + ND = ME->getMemberDecl(); if (ND) DiagRuntimeBehavior(ND->getBeginLoc(), BaseExpr, diff --git a/clang/test/SemaObjC/flexible-array-bounds.m b/clang/test/SemaObjC/flexible-array-bounds.m new file mode 100644 --- /dev/null +++ b/clang/test/SemaObjC/flexible-array-bounds.m @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics +// +// RUN: %clang_cc1 -fsyntax-only -fstrict-flex-arrays=2 -verify=warn %s + +@interface Flexible { +@public + char flexible[]; +} +@end + +@interface Flexible0 { +@public + char flexible[0]; +} +@end + +@interface Flexible1 { +@public + char flexible[1]; +} +@end + +char readit(Flexible *p) { return p->flexible[2]; } +char readit0(Flexible0 *p) { return p->flexible[2]; } +char readit1(Flexible1 *p) { return p->flexible[2]; } // warn-warning {{array index 2 is past the end of the array (which contains 1 element)}} +