intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-20 10:58:11 +08:00
Code Issues Packages Projects Releases Wiki Activity

[clang] Unify Sema and CodeGen implementation of isFlexibleArrayMemberExpr

Browse Source 
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
This commit is contained in:
serge-sans-paille
2022-09-28 08:36:41 +02:00
parent 76ccd1db73
commit 3460a5d795
5 changed files with 127 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
clang/include/clang/AST/Expr.h
View File

@@ -523,6 +523,14 @@ public:
/// 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.

71
clang/lib/AST/Expr.cpp
View File

@@ -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;

47
clang/lib/CodeGen/CGExpr.cpp
View File

@@ -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))

111
clang/lib/Sema/SemaChecking.cpp
View File

@@ -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,

27
clang/test/SemaObjC/flexible-array-bounds.m Normal file
View File

@@ -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)}}