================
@@ -1060,236 +1061,362 @@
CodeGenFunction::evaluateOrEmitBuiltinObjectSize(const Expr *E, unsigned Type,
return ConstantInt::get(ResType, ObjectSize, /*isSigned=*/true);
}
-const FieldDecl *CodeGenFunction::FindFlexibleArrayMemberFieldAndOffset(
- ASTContext &Ctx, const RecordDecl *RD, const FieldDecl *FAMDecl,
- uint64_t &Offset) {
+namespace {
+
+/// StructFieldAccess is a simple visitor class to grab the first MemberExpr
+/// from an Expr. It records any ArraySubscriptExpr we meet along the way.
+class StructFieldAccess
+ : public ConstStmtVisitor<StructFieldAccess, const MemberExpr *> {
+ bool AddrOfSeen = false;
+
+public:
+ const ArraySubscriptExpr *ASE = nullptr;
+
+ const MemberExpr *VisitMemberExpr(const MemberExpr *E) {
+ if (AddrOfSeen && E->getType()->isArrayType())
+ // Avoid forms like '&ptr->array'.
+ return nullptr;
+ return E;
+ }
+
+ const MemberExpr *VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
+ if (ASE)
+ // We don't support multiple subscripts.
+ return nullptr;
+
+ AddrOfSeen = false; // '&ptr->array[idx]' is okay.
+ ASE = E;
+ return Visit(E->getBase());
+ }
+ const MemberExpr *VisitCastExpr(const CastExpr *E) {
+ return Visit(E->getSubExpr());
+ }
+ const MemberExpr *VisitParenExpr(const ParenExpr *E) {
+ return Visit(E->getSubExpr());
+ }
+ const MemberExpr *VisitUnaryAddrOf(const clang::UnaryOperator *E) {
+ AddrOfSeen = true;
+ return Visit(E->getSubExpr());
+ }
+ const MemberExpr *VisitUnaryDeref(const clang::UnaryOperator *E) {
+ AddrOfSeen = false;
+ return Visit(E->getSubExpr());
+ }
+};
+
+} // end anonymous namespace
+
+/// Find a struct's flexible array member. It may be embedded inside multiple
+/// sub-structs, but must still be the last field.
+static const FieldDecl *FindFlexibleArrayMemberField(CodeGenFunction &CGF,
+ ASTContext &Ctx,
+ const RecordDecl *RD) {
const LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
- getLangOpts().getStrictFlexArraysLevel();
- uint32_t FieldNo = 0;
+ CGF.getLangOpts().getStrictFlexArraysLevel();
if (RD->isImplicit())
return nullptr;
for (const FieldDecl *FD : RD->fields()) {
- if ((!FAMDecl || FD == FAMDecl) &&
- Decl::isFlexibleArrayMemberLike(
+ if (Decl::isFlexibleArrayMemberLike(
Ctx, FD, FD->getType(), StrictFlexArraysLevel,
- /*IgnoreTemplateOrMacroSubstitution=*/true)) {
- const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
- Offset += Layout.getFieldOffset(FieldNo);
+ /*IgnoreTemplateOrMacroSubstitution=*/true))
return FD;
+
+ if (auto RT = FD->getType()->getAs<RecordType>())
+ if (const FieldDecl *FD =
+ FindFlexibleArrayMemberField(CGF, Ctx, RT->getAsRecordDecl()))
+ return FD;
+ }
+
+ return nullptr;
+}
+
+/// Calculate the offset of a struct field. It may be embedded inside multiple
+/// sub-structs.
+static bool GetFieldOffset(ASTContext &Ctx, const RecordDecl *RD,
+ const FieldDecl *FD, int64_t &Offset) {
+ if (RD->isImplicit())
+ return false;
+
+ // Keep track of the field number ourselves, because the other methods
+ // (CGRecordLayout::getLLVMFieldNo) aren't always equivalent to how the AST
+ // is laid out.
+ uint32_t FieldNo = 0;
+ const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
+
+ for (const FieldDecl *Field : RD->fields()) {
+ if (Field == FD) {
+ Offset += Layout.getFieldOffset(FieldNo);
+ return true;
}
- QualType Ty = FD->getType();
- if (Ty->isRecordType()) {
- if (const FieldDecl *Field = FindFlexibleArrayMemberFieldAndOffset(
- Ctx, Ty->getAsRecordDecl(), FAMDecl, Offset)) {
- const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
+ if (auto RT = Field->getType()->getAs<RecordType>()) {
+ if (GetFieldOffset(Ctx, RT->getAsRecordDecl(), FD, Offset)) {
Offset += Layout.getFieldOffset(FieldNo);
- return Field;
+ return true;
}
}
if (!RD->isUnion())
++FieldNo;
}
- return nullptr;
+ return false;
}
-static unsigned CountCountedByAttrs(const RecordDecl *RD) {
- unsigned Num = 0;
+static std::optional<int64_t>
+GetFieldOffset(ASTContext &Ctx, const RecordDecl *RD, const FieldDecl *FD) {
+ int64_t Offset = 0;
- for (const FieldDecl *FD : RD->fields()) {
- if (FD->getType()->isCountAttributedType())
- return ++Num;
-
- QualType Ty = FD->getType();
- if (Ty->isRecordType())
- Num += CountCountedByAttrs(Ty->getAsRecordDecl());
- }
+ if (GetFieldOffset(Ctx, RD, FD, Offset))
+ return std::optional<int64_t>(Offset);
- return Num;
+ return std::nullopt;
}
llvm::Value *
-CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
- llvm::IntegerType *ResType) {
- // The code generated here calculates the size of a struct with a flexible
- // array member that uses the counted_by attribute. There are two instances
- // we handle:
- //
- // struct s {
- // unsigned long flags;
- // int count;
- // int array[] __attribute__((counted_by(count)));
- // }
- //
- // 1) bdos of the flexible array itself:
- //
- // __builtin_dynamic_object_size(p->array, 1) ==
- // p->count * sizeof(*p->array)
- //
- // 2) bdos of a pointer into the flexible array:
- //
- // __builtin_dynamic_object_size(&p->array[42], 1) ==
- // (p->count - 42) * sizeof(*p->array)
+CodeGenFunction::emitCountedByMemberSize(const Expr *E, llvm::Value *EmittedE,
+ unsigned Type,
+ llvm::IntegerType *ResType) {
+ ASTContext &Ctx = getContext();
+
+ // Note: If the whole struct is specificed in the __bdos (i.e. Visitor
+ // returns a DeclRefExpr). The calculation of the whole size of the structure
+ // with a flexible array member can be done in two ways:
//
- // 2) bdos of the whole struct, including the flexible array:
+ // 1) sizeof(struct S) + count * sizeof(typeof(fam))
+ // 2) offsetof(struct S, fam) + count * sizeof(typeof(fam))
//
- // __builtin_dynamic_object_size(p, 1) ==
- // max(sizeof(struct s),
- // offsetof(struct s, array) + p->count * sizeof(*p->array))
+ // The first will add additional padding after the end of the array
+ // allocation while the second method is more precise, but not quite expected
+ // from programmers. See
+ // https://lore.kernel.org/lkml/ZvV6X5FPBBW7CO1f@archlinux/ for a discussion
+ // of the topic.
//
- ASTContext &Ctx = getContext();
- const Expr *Base = E->IgnoreParenImpCasts();
- const Expr *Idx = nullptr;
-
- if (const auto *UO = dyn_cast<UnaryOperator>(Base);
- UO && UO->getOpcode() == UO_AddrOf) {
- Expr *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts();
- if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(SubExpr)) {
- Base = ASE->getBase()->IgnoreParenImpCasts();
- Idx = ASE->getIdx()->IgnoreParenImpCasts();
-
- if (const auto *IL = dyn_cast<IntegerLiteral>(Idx)) {
- int64_t Val = IL->getValue().getSExtValue();
- if (Val < 0)
- return getDefaultBuiltinObjectSizeResult(Type, ResType);
-
- if (Val == 0)
- // The index is 0, so we don't need to take it into account.
- Idx = nullptr;
- }
- } else {
- // Potential pointer to another element in the struct.
- Base = SubExpr;
- }
- }
+ // GCC isn't (currently) able to calculate __bdos on a pointer to the whole
+ // structure. Therefore, because of the above issue, we choose to match what
+ // GCC does for consistency's sake.
- // Get the flexible array member Decl.
- const RecordDecl *OuterRD = nullptr;
- const FieldDecl *FAMDecl = nullptr;
- if (const auto *ME = dyn_cast<MemberExpr>(Base)) {
- // Check if \p Base is referencing the FAM itself.
- const ValueDecl *VD = ME->getMemberDecl();
- OuterRD = VD->getDeclContext()->getOuterLexicalRecordContext();
- FAMDecl = dyn_cast<FieldDecl>(VD);
- if (!FAMDecl)
- return nullptr;
- } else if (const auto *DRE = dyn_cast<DeclRefExpr>(Base)) {
- // Check if we're pointing to the whole struct.
- QualType Ty = DRE->getDecl()->getType();
- if (Ty->isPointerType())
- Ty = Ty->getPointeeType();
- OuterRD = Ty->getAsRecordDecl();
-
- // If we have a situation like this:
- //
- // struct union_of_fams {
- // int flags;
- // union {
- // signed char normal_field;
- // struct {
- // int count1;
- // int arr1[] __counted_by(count1);
- // };
- // struct {
- // signed char count2;
- // int arr2[] __counted_by(count2);
- // };
- // };
- // };
- //
- // We don't know which 'count' to use in this scenario:
- //
- // size_t get_size(struct union_of_fams *p) {
- // return __builtin_dynamic_object_size(p, 1);
- // }
- //
- // Instead of calculating a wrong number, we give up.
- if (OuterRD && CountCountedByAttrs(OuterRD) > 1)
- return nullptr;
- }
+ StructFieldAccess Visitor;
+ const MemberExpr *ME = Visitor.Visit(E);
+ if (!ME)
+ return nullptr;
- if (!OuterRD)
+ const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl());
+ if (!FD)
return nullptr;
- // We call FindFlexibleArrayMemberAndOffset even if FAMDecl is non-null to
- // get its offset.
- uint64_t Offset = 0;
- FAMDecl =
- FindFlexibleArrayMemberFieldAndOffset(Ctx, OuterRD, FAMDecl, Offset);
- Offset = Ctx.toCharUnitsFromBits(Offset).getQuantity();
+ const RecordDecl *RD = FD->getDeclContext()->getOuterLexicalRecordContext();
+ const FieldDecl *FlexibleArrayMemberFD = nullptr;
- if (!FAMDecl || !FAMDecl->getType()->isCountAttributedType())
- // No flexible array member found or it doesn't have the "counted_by"
- // attribute.
- return nullptr;
+ if (Decl::isFlexibleArrayMemberLike(
+ Ctx, FD, FD->getType(), getLangOpts().getStrictFlexArraysLevel(),
+ /*IgnoreTemplateOrMacroSubstitution=*/true))
+ FlexibleArrayMemberFD = FD;
+ else
+ FlexibleArrayMemberFD = FindFlexibleArrayMemberField(*this, Ctx, RD);
- const FieldDecl *CountedByFD = FAMDecl->findCountedByField();
- if (!CountedByFD)
- // Can't find the field referenced by the "counted_by" attribute.
+ if (!FlexibleArrayMemberFD ||
+ !FlexibleArrayMemberFD->getType()->isCountAttributedType())
return nullptr;
- if (isa<DeclRefExpr>(Base))
- // The whole struct is specificed in the __bdos. The calculation of the
- // whole size of the structure can be done in two ways:
- //
- // 1) sizeof(struct S) + count * sizeof(typeof(fam))
- // 2) offsetof(struct S, fam) + count * sizeof(typeof(fam))
- //
- // The first will add additional padding after the end of the array,
- // allocation while the second method is more precise, but not quite
- // expected from programmers. See
- // https://lore.kernel.org/lkml/ZvV6X5FPBBW7CO1f@archlinux/ for a
- // discussion of the topic.
- //
- // GCC isn't (currently) able to calculate __bdos on a pointer to the whole
- // structure. Therefore, because of the above issue, we'll choose to match
- // what GCC does for consistency's sake.
+ const FieldDecl *CountFD = FlexibleArrayMemberFD->findCountedByField();
+ if (!CountFD)
+ // Can't find the field referenced by the "counted_by" attribute.
return nullptr;
- // Build a load of the counted_by field.
- bool IsSigned = CountedByFD->getType()->isSignedIntegerType();
- Value *CountedByInst = EmitLoadOfCountedByField(Base, FAMDecl, CountedByFD);
- if (!CountedByInst)
- return getDefaultBuiltinObjectSizeResult(Type, ResType);
-
- CountedByInst = Builder.CreateIntCast(CountedByInst, ResType, IsSigned);
+ const Expr *Idx = nullptr;
+ if (Visitor.ASE) {
+ Idx = Visitor.ASE->getIdx();
- // Build a load of the index and subtract it from the count.
- Value *IdxInst = nullptr;
- if (Idx) {
- if (Idx->HasSideEffects(getContext()))
+ if (Idx->HasSideEffects(Ctx))
// We can't have side-effects.
return getDefaultBuiltinObjectSizeResult(Type, ResType);
+ if (const auto *IL = dyn_cast<IntegerLiteral>(Idx)) {
+ int64_t Val = IL->getValue().getSExtValue();
+ if (Val < 0)
+ return getDefaultBuiltinObjectSizeResult(Type, ResType);
+
+ // The index is 0, so we don't need to take it into account.
+ if (Val == 0)
+ Idx = nullptr;
+ }
+ }
+
+ // Calculate the flexible array member's object size using these formulae
+ // (note: if the calculation is negative, we return 0.):
+ //
+ // struct p;
+ // struct s {
+ // /* ... */
+ // int count;
+ // struct p *array[] __attribute__((counted_by(count)));
+ // };
+ //
+ // 1) 'ptr->array':
+ //
+ // size_t count = (size_t) ptr->count;
+ //
+ // size_t flexible_array_member_base_size = sizeof (*ptr->array);
+ // size_t flexible_array_member_size =
+ // count * flexible_array_member_base_size;
+ //
+ // if (flexible_array_member_size < 0)
+ // return 0;
----------------
nickdesaulniers wrote:
> The result type of __builtin{_dynamic}_object_size is size_t.
Right, but I'm more curious about the type of the `count` field. Can the
`count` be negative? FWICT, this isn't checked (we check the result AFTER
multiplying it by the `flexible_array_member_base_size`, but could that
calculation have overflowed?); the tests indicate it's just zero extended.
(Perhaps we don't need to solve all of these in this PR, but perhaps we should
file issues/TODOs with a list of issues to fix). I'm worried that at runtime
the `count` is somehow set to `-1`, and the array access is allowed because
it's interpreted as a large positive number, which then overflows when
multiplied by `flexible_array_member_base_size` and passes the "is negative"
check, allowing access when access should be denied.
https://github.com/llvm/llvm-project/pull/122198
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
