serge-sans-paille created this revision.
serge-sans-paille added reviewers: aaron.ballman, efriedma, jyknight, jrtc27,
msebor.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Turn it into a single Expr::isFlexibleArrayMemberLike method, as discussed in
https://discourse.llvm.org/t/rfc-harmonize-flexible-array-members-handling
Keep different behavior with respect to macro / template substitution, and
harmonize sharp edges.
This does not impact __builtin_object_size interactions with FAM.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134791
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaChecking.cpp
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ 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<FieldDecl>(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<TypedefTypeLoc>()) {
- const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
- TInfo = TDL->getTypeSourceInfo();
- continue;
- }
- if (ConstantArrayTypeLoc CTL = TL.getAs<ConstantArrayTypeLoc>()) {
- const Expr *SizeExpr = dyn_cast<IntegerLiteral>(CTL.getSizeExpr());
- if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
- return false;
- }
- break;
- }
-
- const RecordDecl *RD = dyn_cast<RecordDecl>(FD->getDeclContext());
- if (!RD)
- return false;
- if (RD->isUnion())
- return false;
- if (const CXXRecordDecl *CRD = dyn_cast<CXXRecordDecl>(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<FieldDecl>(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<DeclRefExpr>(BaseExpr))
- ND = DRE->getDecl();
- else if (const auto *ME = dyn_cast<MemberExpr>(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<ArraySubscriptExpr>(BaseExpr))
- BaseExpr = ASE->getBase()->IgnoreParenCasts();
- if (const auto *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
- ND = DRE->getDecl();
- if (const auto *ME = dyn_cast<MemberExpr>(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<ArraySubscriptExpr>(BaseExpr))
+ BaseExpr = ASE->getBase()->IgnoreParenCasts();
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
+ ND = DRE->getDecl();
+ if (const auto *ME = dyn_cast<MemberExpr>(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<ArraySubscriptExpr>(BaseExpr))
- BaseExpr = ASE->getBase()->IgnoreParenCasts();
- if (const auto *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
- ND = DRE->getDecl();
- if (const auto *ME = dyn_cast<MemberExpr>(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<ArraySubscriptExpr>(BaseExpr))
+ BaseExpr = ASE->getBase()->IgnoreParenCasts();
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
+ ND = DRE->getDecl();
+ if (const auto *ME = dyn_cast<MemberExpr>(BaseExpr))
+ ND = ME->getMemberDecl();
if (ND)
DiagRuntimeBehavior(ND->getBeginLoc(), BaseExpr,
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ 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<ConstantArrayType>(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<IncompleteArrayType>(AT))
- return false;
-
- E = E->IgnoreParens();
-
- // A flexible array member must be the last member in the class.
- if (const auto *ME = dyn_cast<MemberExpr>(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<FieldDecl>(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<FieldDecl *>(FD)));
- return ++FI == FD->getParent()->field_end();
- }
- } else if (const auto *IRE = dyn_cast<ObjCIvarRefExpr>(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<CastExpr>(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<ConstantArrayType>(AT))
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -203,6 +203,78 @@
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<DeclRefExpr>(E))
+ ND = DRE->getDecl();
+ else if (const auto *ME = dyn_cast<MemberExpr>(E))
+ ND = ME->getMemberDecl();
+ else if (const auto *IRE = dyn_cast<ObjCIvarRefExpr>(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<FieldDecl>(ND)) {
+
+ // 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<TypedefTypeLoc>()) {
+ const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
+ TInfo = TDL->getTypeSourceInfo();
+ continue;
+ }
+ if (ConstantArrayTypeLoc CTL = TL.getAs<ConstantArrayTypeLoc>()) {
+ const Expr *SizeExpr = dyn_cast<IntegerLiteral>(CTL.getSizeExpr());
+ if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
+ return false;
+ }
+ break;
+ }
+ }
+
+ if (FD->getParent()->isUnion())
+ return true;
+
+ RecordDecl::field_iterator FI(
+ DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
+ return ++FI == FD->getParent()->field_end();
+ }
+
+ return false;
+}
+
const ValueDecl *
Expr::getAsBuiltinConstantDeclRef(const ASTContext &Context) const {
Expr::EvalResult Eval;
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ 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 substitution of macro or 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
