Skip to content

Commit

Permalink
[Clang] Disable use of the counted_by attribute for whole struct poin…
Browse files Browse the repository at this point in the history
…ters (llvm#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 <efriedma@quicinc.com>
  • Loading branch information
bwendling and efriedma-quic committed Oct 17, 2024
1 parent 7ba7d8e commit 5ef6cde
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 191 deletions.
45 changes: 20 additions & 25 deletions clang/lib/CodeGen/CGBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,24 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
// Can't find the field referenced by the "counted_by" attribute.
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.
return nullptr;

// Build a load of the counted_by field.
bool IsSigned = CountedByFD->getType()->isSignedIntegerType();
Value *CountedByInst = EmitCountedByFieldExpr(Base, FAMDecl, CountedByFD);
Expand Down Expand Up @@ -1031,32 +1049,9 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
CharUnits Size = Ctx.getTypeSizeInChars(ArrayTy->getElementType());
llvm::Constant *ElemSize =
llvm::ConstantInt::get(ResType, Size.getQuantity(), IsSigned);
Value *FAMSize =
Value *Res =
Builder.CreateMul(CountedByInst, ElemSize, "", !IsSigned, IsSigned);
FAMSize = Builder.CreateIntCast(FAMSize, ResType, IsSigned);
Value *Res = FAMSize;

if (isa<DeclRefExpr>(Base)) {
// The whole struct is specificed in the __bdos.
const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(OuterRD);

// Get the offset of the FAM.
llvm::Constant *FAMOffset = ConstantInt::get(ResType, Offset, IsSigned);
Value *OffsetAndFAMSize =
Builder.CreateAdd(FAMOffset, Res, "", !IsSigned, IsSigned);

// Get the full size of the struct.
llvm::Constant *SizeofStruct =
ConstantInt::get(ResType, Layout.getSize().getQuantity(), IsSigned);

// max(sizeof(struct s),
// offsetof(struct s, array) + p->count * sizeof(*p->array))
Res = IsSigned
? Builder.CreateBinaryIntrinsic(llvm::Intrinsic::smax,
OffsetAndFAMSize, SizeofStruct)
: Builder.CreateBinaryIntrinsic(llvm::Intrinsic::umax,
OffsetAndFAMSize, SizeofStruct);
}
Res = Builder.CreateIntCast(Res, ResType, IsSigned);

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

0 comments on commit 5ef6cde

Please sign in to comment.