Skip to content

Commit 8c62bf5

Browse files
[Clang] Disable use of the counted_by attribute for whole struct pointers (#112636)
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 any remaining whitespace that might exist after allocation while the second method is more precise, but not quite expected from programmers. See [1] 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. [1] https://lore.kernel.org/lkml/ZvV6X5FPBBW7CO1f@archlinux/ Co-authored-by: Eli Friedman <[email protected]>
1 parent b060661 commit 8c62bf5

File tree

2 files changed

+110
-194
lines changed

2 files changed

+110
-194
lines changed

clang/lib/CodeGen/CGBuiltin.cpp

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,6 +1013,24 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
10131013
// Can't find the field referenced by the "counted_by" attribute.
10141014
return nullptr;
10151015

1016+
if (isa<DeclRefExpr>(Base))
1017+
// The whole struct is specificed in the __bdos. The calculation of the
1018+
// whole size of the structure can be done in two ways:
1019+
//
1020+
// 1) sizeof(struct S) + count * sizeof(typeof(fam))
1021+
// 2) offsetof(struct S, fam) + count * sizeof(typeof(fam))
1022+
//
1023+
// The first will add additional padding after the end of the array,
1024+
// allocation while the second method is more precise, but not quite
1025+
// expected from programmers. See
1026+
// https://lore.kernel.org/lkml/ZvV6X5FPBBW7CO1f@archlinux/ for a
1027+
// discussion of the topic.
1028+
//
1029+
// GCC isn't (currently) able to calculate __bdos on a pointer to the whole
1030+
// structure. Therefore, because of the above issue, we'll choose to match
1031+
// what GCC does for consistency's sake.
1032+
return nullptr;
1033+
10161034
// Build a load of the counted_by field.
10171035
bool IsSigned = CountedByFD->getType()->isSignedIntegerType();
10181036
Value *CountedByInst = EmitLoadOfCountedByField(Base, FAMDecl, CountedByFD);
@@ -1043,32 +1061,9 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
10431061
CharUnits Size = Ctx.getTypeSizeInChars(ArrayTy->getElementType());
10441062
llvm::Constant *ElemSize =
10451063
llvm::ConstantInt::get(ResType, Size.getQuantity(), IsSigned);
1046-
Value *FAMSize =
1064+
Value *Res =
10471065
Builder.CreateMul(CountedByInst, ElemSize, "", !IsSigned, IsSigned);
1048-
FAMSize = Builder.CreateIntCast(FAMSize, ResType, IsSigned);
1049-
Value *Res = FAMSize;
1050-
1051-
if (isa<DeclRefExpr>(Base)) {
1052-
// The whole struct is specificed in the __bdos.
1053-
const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(OuterRD);
1054-
1055-
// Get the offset of the FAM.
1056-
llvm::Constant *FAMOffset = ConstantInt::get(ResType, Offset, IsSigned);
1057-
Value *OffsetAndFAMSize =
1058-
Builder.CreateAdd(FAMOffset, Res, "", !IsSigned, IsSigned);
1059-
1060-
// Get the full size of the struct.
1061-
llvm::Constant *SizeofStruct =
1062-
ConstantInt::get(ResType, Layout.getSize().getQuantity(), IsSigned);
1063-
1064-
// max(sizeof(struct s),
1065-
// offsetof(struct s, array) + p->count * sizeof(*p->array))
1066-
Res = IsSigned
1067-
? Builder.CreateBinaryIntrinsic(llvm::Intrinsic::smax,
1068-
OffsetAndFAMSize, SizeofStruct)
1069-
: Builder.CreateBinaryIntrinsic(llvm::Intrinsic::umax,
1070-
OffsetAndFAMSize, SizeofStruct);
1071-
}
1066+
Res = Builder.CreateIntCast(Res, ResType, IsSigned);
10721067

10731068
// A negative \p IdxInst or \p CountedByInst means that the index lands
10741069
// outside of the flexible array member. If that's the case, we want to

0 commit comments

Comments
 (0)