[clang] 3460a5d - [clang] Unify Sema and CodeGen implementation of isFlexibleArrayMemberExpr
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 4 11:42:46 PDT 2022
Author: serge-sans-paille
Date: 2022-10-04T20:42:36+02:00
New Revision: 3460a5d7957237a8dbe8ce523edb3e83baa2f4c7
URL: https://github.com/llvm/llvm-project/commit/3460a5d7957237a8dbe8ce523edb3e83baa2f4c7
DIFF: https://github.com/llvm/llvm-project/commit/3460a5d7957237a8dbe8ce523edb3e83baa2f4c7.diff
LOG: [clang] Unify Sema and CodeGen implementation of isFlexibleArrayMemberExpr
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: ObjC interface now behave as C struct wrt. FAM and
-fstrict-flex-arrays.
This does not impact __builtin_object_size interactions with FAM.
Differential Revision: https://reviews.llvm.org/D134791
Added:
clang/test/SemaObjC/flexible-array-bounds.m
Modified:
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaChecking.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index d8efe663ce8ee..7c90df25ccddd 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -523,6 +523,14 @@ class Expr : public ValueStmt {
/// 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
index 5fd4129301810..410c8e4d649ec 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -203,6 +203,77 @@ bool Expr::isKnownToHaveBooleanValue(bool Semantic) const {
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)) {
+ 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<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;
+ }
+ }
+
+ 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;
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 80a05c984f601..164ee81bb1879 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -875,50 +875,6 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
}
}
-/// 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 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction &CGF,
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))
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 95f86cb6a48e2..e73da2db179bb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15898,72 +15898,6 @@ void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
<< 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 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
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 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
<< (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 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
<< 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,
diff --git a/clang/test/SemaObjC/flexible-array-bounds.m b/clang/test/SemaObjC/flexible-array-bounds.m
new file mode 100644
index 0000000000000..78202dc8c6c9f
--- /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
+
+ at interface Flexible {
+ at public
+ char flexible[];
+}
+ at end
+
+ at interface Flexible0 {
+ at public
+ char flexible[0];
+}
+ at end
+
+ at interface Flexible1 {
+ at public
+ char flexible[1];
+}
+ at 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)}}
+
More information about the cfe-commits
mailing list