Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang] Add __builtin_counted_by_ref builtin #114495

Open
wants to merge 68 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 59 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
fe49528
[Clang] Add __builtin_counted_by_ref builtin
bwendling Aug 7, 2024
e3583c7
If the 'counted_by' attribute doesn't exist, then return nullptr.
bwendling Aug 7, 2024
053bfcd
Remove debugging code.
bwendling Aug 7, 2024
4ce6525
Make returning a nullptr the default.
bwendling Aug 7, 2024
8017e73
clang-format
bwendling Aug 7, 2024
8808431
Use a visitor to retrieve the MemberExpr. It's intentionally harsh an…
bwendling Aug 7, 2024
122d2b8
Add testcases.
bwendling Aug 7, 2024
2b58283
Combine the code to grab the MemberExpr, if it exists.
bwendling Aug 8, 2024
76f01fe
Move FindCountedByField into a central place.
bwendling Aug 9, 2024
c95e38c
Use CustomTypeChecking for the builtin. It allows us explicitly to se…
bwendling Aug 14, 2024
925fbd2
[Clang][NFC] Move FindCountedByField into FieldDecl (#104235)
bwendling Aug 15, 2024
ec62a2b
Fix a build failure.
bwendling Aug 15, 2024
d3ca48c
Remove empty line.
bwendling Aug 15, 2024
85b4474
Remove merge errors.
bwendling Aug 15, 2024
91a7d1b
Use 'size_t *' for the default return type and improve the testcase.
bwendling Aug 15, 2024
2d95e44
Don't use a Visitor to retrieve the MemberExpr. Instead accept only n…
bwendling Aug 15, 2024
9628b0f
Improve error messages. Make the side-effects error a warning.
bwendling Aug 15, 2024
c278c1f
Add _Generic checks.
bwendling Aug 15, 2024
0f42e5d
Remove unrelated change
bwendling Aug 21, 2024
751a55d
Use MemberExpr::Create instead of MemberExpr::CreateImplicit.
bwendling Aug 21, 2024
53272cf
Regenerate
bwendling Aug 21, 2024
ecda8e0
Add -W flag for warning.
bwendling Aug 21, 2024
612000f
Handle any anonymous structs surrounding the flexible array member an…
bwendling Aug 22, 2024
b555af9
Add anonymous struct testcase.
bwendling Aug 22, 2024
48707ae
Report a fatal error if we can't find the 'count' field.
bwendling Aug 23, 2024
5c778ca
Use 'cast' instead of 'dyn_cast' and default to 'void *' for the retu…
isanbard Aug 23, 2024
a1a6ead
A 'void *' can result in an error message about assigning to a 'void'…
bwendling Aug 23, 2024
6b388b5
Add some documentation.
bwendling Aug 26, 2024
791443b
Rename to __builtin_counted_by_ref.
bwendling Sep 10, 2024
768e7b1
Rename method to reflect new builtin name.
bwendling Sep 10, 2024
4111cc0
Emit an error if the 'count' reference is assigned to a variable, use…
bwendling Sep 11, 2024
d897d09
Ignore paren casts.
bwendling Sep 11, 2024
4f1c313
Don't allow __builtin_counted_by_ref in complex expressions.
bwendling Sep 11, 2024
7002f50
Add test case for pointers.
bwendling Sep 11, 2024
7ef517b
Add SourceRange to the diagnostic.
bwendling Sep 12, 2024
19e00f1
Check that __builtin_counted_by_ref isn't used on the LHS by a comple…
bwendling Sep 13, 2024
8f6ef28
Update clang/include/clang/Basic/DiagnosticSemaKinds.td
bwendling Sep 13, 2024
cd65acc
Add a flag indicating that the 'MemberExpr' was once a call to '__bui…
bwendling Sep 19, 2024
9b36f19
Remove invalid tests that take the address of the bounds counter.
bwendling Sep 20, 2024
c08d8d9
Add testcase that discards side-effects.
bwendling Sep 20, 2024
906dd2e
Delay warnings for _Generic statements.
bwendling Sep 26, 2024
e3fbc03
Marking some diagnostics as 'potentially evaluated' somehow prevented…
bwendling Oct 3, 2024
ac5ff7c
Regenerate testcase.
bwendling Oct 3, 2024
072f7e4
Fix testcase.
bwendling Oct 3, 2024
391e2f6
Fix some of the naming and remove dead code.
bwendling Oct 4, 2024
4555763
Correct include ordering.
bwendling Oct 4, 2024
4e22bd6
Improve the argument checking so that it's checking specifically for …
bwendling Oct 16, 2024
985cd02
Move setting that the counter is a bounds-safety counter to where __b…
bwendling Oct 18, 2024
f9ebf9f
Retain the __builtin_counted_by_ref CallExpr for AST dumping reasons.…
bwendling Nov 1, 2024
a5245e5
Fix
bwendling Nov 1, 2024
271bb8f
Reformat
bwendling Nov 1, 2024
ceb43e5
Add correct attribute checking and set the return type.
bwendling Nov 1, 2024
b01526d
Generate LLVM IR for the __builtin_counted_by_ref.
bwendling Nov 1, 2024
76aabde
Set InBounds on the GEP.
bwendling Nov 1, 2024
c390168
Regenerate testcase.
bwendling Nov 1, 2024
213b00d
Remove warning that no longer shows up.
bwendling Nov 1, 2024
c4509f2
Regenerate testcase
bwendling Nov 1, 2024
e22f7ef
Expand testing. This includes a hack to prevent multiple diagnostics …
bwendling Nov 1, 2024
90d6ea8
Add test rejecting using of 'counted_by' in C++.
bwendling Nov 1, 2024
894a7da
Add -ast-print testcase.
bwendling Nov 4, 2024
f8de7dc
Re-add inbounds to the GEP.
bwendling Nov 4, 2024
ed875eb
Add test for &ptr->array
bwendling Nov 4, 2024
3b50e8d
Remote -O2 from codegen.
bwendling Nov 4, 2024
d2112e5
Remove setting the 'inBounds' flag.
bwendling Nov 4, 2024
d595618
Upgrade the side-effects warning into an error.
bwendling Nov 5, 2024
faffcf8
Restrict __builtin_counted_by_ref to 'ptr->array' to match GCC's impl…
bwendling Nov 5, 2024
426782f
Regenerate.
bwendling Nov 5, 2024
4892df7
Revert the modifications as they're not affected by this feature.
bwendling Nov 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions clang/docs/LanguageExtensions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3774,6 +3774,74 @@ type-generic alternative to the ``__builtin_clz{,l,ll}`` (respectively
``__builtin_ctz{,l,ll}``) builtins, with support for other integer types, such
as ``unsigned __int128`` and C23 ``unsigned _BitInt(N)``.

``__builtin_counted_by_ref``
----------------------------

``__builtin_counted_by_ref`` returns a pointer to the count field from the
``counted_by`` attribute.

The argument must be a flexible array member. If the argument isn't a flexible
array member or doesn't have the ``counted_by`` attribute, the builtin returns
``(void *)0``.

**Syntax**:

.. code-block:: c

T *__builtin_counted_by_ref(void *array)

**Examples**:

.. code-block:: c

#define alloc(P, FAM, COUNT) ({ \
size_t __ignored_assignment; \
typeof(P) __p = NULL; \
__p = malloc(MAX(sizeof(*__p), \
sizeof(*__p) + sizeof(*__p->FAM) * COUNT)); \
\
*_Generic( \
__builtin_counted_by_ref(__p->FAM), \
void *: &__ignored_assignment, \
default: __builtin_counted_by_ref(__p->FAM)) = COUNT; \
\
__p; \
})

**Description**:

The ``__builtin_counted_by_ref`` builtin allows the programmer to prevent a
common error associated with the ``counted_by`` attribute. When using the
``counted_by`` attribute, the ``count`` field **must** be set before the
flexible array member can be accessed. Otherwise, the sanitizers may view such
accesses as false positives. For instance, it's not uncommon for programmers to
initialize the flexible array before setting the ``count`` field:

.. code-block:: c

struct s {
int dummy;
short count;
long array[] __attribute__((counted_by(count)));
};

struct s *ptr = malloc(sizeof(struct s) + sizeof(long) * COUNT);

for (int i = 0; i < COUNT; ++i)
ptr->array[i] = i;

ptr->count = COUNT;

Enforcing the rule that ``ptr->count = COUNT;`` must occur after every
allocation of a struct with a flexible array member with the ``counted_by``
attribute is prone to failure in large code bases. This builtin mitigates this
for allocators (like in Linux) that are implemented in a way where the counter
assignment can happen automatically.

**Note: The value returned by ``__builtin_counted_by_ref`` cannot be assigned
to a variable, have its address taken, or passed into or returned from a
function, because doing so violates bounds safety conventions.**

Multiprecision Arithmetic Builtins
----------------------------------

Expand Down
23 changes: 23 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,29 @@ Non-comprehensive list of changes in this release
as well as declarations.
- ``__builtin_abs`` function can now be used in constant expressions.

- The new builtin ``__builtin_counted_by_ref`` was added. In contexts where the
programmer needs access to the ``counted_by`` attribute's field, but it's not
available --- e.g. in macros. For instace, it can be used to automatically
set the counter during allocation in the Linux kernel:

.. code-block:: c

/* A simplified version of Linux allocation macros */
#define alloc(PTR, FAM, COUNT) ({ \
sizeof_t __ignored_assignment; \
typeof(P) __p; \
size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
__p = malloc(__size); \
*_Generic( \
__builtin_counted_by_ref(__p->FAM), \
void *: &__ignored_assignment, \
default: __builtin_counted_by_ref(__p->FAM)) = COUNT; \
__p; \
})

The flexible array member (FAM) can now be accessed immediately without causing
issues with the sanitizer because the counter is automatically set.

New Compiler Flags
------------------

Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/Builtins.td
Original file line number Diff line number Diff line change
Expand Up @@ -4914,3 +4914,9 @@ def ArithmeticFence : LangBuiltin<"ALL_LANGUAGES"> {
let Attributes = [CustomTypeChecking, Constexpr];
let Prototype = "void(...)";
}

def CountedByRef : Builtin {
let Spellings = ["__builtin_counted_by_ref"];
let Attributes = [NoThrow, CustomTypeChecking];
let Prototype = "int(...)";
rapidsna marked this conversation as resolved.
Show resolved Hide resolved
}
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -1482,6 +1482,10 @@ def NoDeref : DiagGroup<"noderef">;
def BoundsSafetyCountedByEltTyUnknownSize :
DiagGroup<"bounds-safety-counted-by-elt-type-unknown-size">;

// counted_by warnings
def CountedByRefSideEffects
: DiagGroup<"counted-by-ref-side-effects">;

// A group for cross translation unit static analysis related warnings.
def CrossTU : DiagGroup<"ctu">;

Expand Down
14 changes: 14 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -6652,6 +6652,20 @@ def warn_counted_by_attr_elt_type_unknown_size :
Warning<err_counted_by_attr_pointee_unknown_size.Summary>,
InGroup<BoundsSafetyCountedByEltTyUnknownSize>;

// __builtin_counted_by_ref diagnostics:
def err_builtin_counted_by_ref_must_be_flex_array_member : Error<
"'__builtin_counted_by_ref' argument must reference a flexible array member">;
def err_builtin_counted_by_ref_cannot_leak_reference : Error<
"value returned by '__builtin_counted_by_ref' cannot be assigned to a "
"variable, have its address taken, or passed into or returned from a function">;
def err_builtin_counted_by_ref_invalid_lhs_use : Error<
"value returned by '__builtin_counted_by_ref' cannot be used in "
"%select{an array subscript|a binary}0 expression">;

def warn_builtin_counted_by_ref_has_side_effects : Warning<
"'__builtin_counted_by_ref' argument has side-effects that will be discarded">,
InGroup<CountedByRefSideEffects>;

let CategoryName = "ARC Semantic Issue" in {

// ARC-mode diagnostics.
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -2510,6 +2510,8 @@ class Sema final : public SemaBase {

bool BuiltinNonDeterministicValue(CallExpr *TheCall);

bool BuiltinCountedByRef(CallExpr *TheCall);

// Matrix builtin handling.
ExprResult BuiltinMatrixTranspose(CallExpr *TheCall, ExprResult CallResult);
ExprResult BuiltinMatrixColumnMajorLoad(CallExpr *TheCall,
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3656,6 +3656,10 @@ unsigned FunctionDecl::getBuiltinID(bool ConsiderWrapperFunctions) const {
(!hasAttr<ArmBuiltinAliasAttr>() && !hasAttr<BuiltinAliasAttr>()))
return 0;

if (getASTContext().getLangOpts().CPlusPlus &&
BuiltinID == Builtin::BI__builtin_counted_by_ref)
return 0;

const ASTContext &Context = getASTContext();
if (!Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
return BuiltinID;
Expand Down
32 changes: 32 additions & 0 deletions clang/lib/CodeGen/CGBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3691,6 +3691,38 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
return RValue::get(emitBuiltinObjectSize(E->getArg(0), Type, ResType,
/*EmittedE=*/nullptr, IsDynamic));
}
case Builtin::BI__builtin_counted_by_ref: {
// Default to returning '(void *) 0'.
llvm::Value *Result = llvm::ConstantPointerNull::get(
llvm::PointerType::getUnqual(getLLVMContext()));

const Expr *Arg = E->getArg(0)->IgnoreParenImpCasts();

if (auto *UO = dyn_cast<UnaryOperator>(Arg);
UO && UO->getOpcode() == UO_AddrOf) {
Arg = UO->getSubExpr()->IgnoreParenImpCasts();

if (auto *ASE = dyn_cast<ArraySubscriptExpr>(Arg))
Arg = ASE->getBase()->IgnoreParenImpCasts();
}

if (const MemberExpr *ME = dyn_cast_if_present<MemberExpr>(Arg)) {
if (auto *CATy =
ME->getMemberDecl()->getType()->getAs<CountAttributedType>();
CATy && CATy->getKind() == CountAttributedType::CountedBy) {
const auto *FAMDecl = cast<FieldDecl>(ME->getMemberDecl());
if (const FieldDecl *CountFD = FAMDecl->findCountedByField()) {
Result = GetCountedByFieldExprGEP(Arg, FAMDecl, CountFD);
cast<llvm::GetElementPtrInst>(Result)->setNoWrapFlags(
bwendling marked this conversation as resolved.
Show resolved Hide resolved
GEPNoWrapFlags::inBounds());
} else {
llvm::report_fatal_error("Cannot find the counted_by 'count' field");
}
}
}

return RValue::get(Result);
}
case Builtin::BI__builtin_prefetch: {
Value *Locality, *RW, *Address = EmitScalarExpr(E->getArg(0));
// FIXME: Technically these constants should of type 'int', yes?
Expand Down
33 changes: 19 additions & 14 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1145,15 +1145,7 @@ static bool getGEPIndicesToField(CodeGenFunction &CGF, const RecordDecl *RD,
return false;
}

/// This method is typically called in contexts where we can't generate
/// side-effects, like in __builtin_dynamic_object_size. When finding
/// expressions, only choose those that have either already been emitted or can
/// be loaded without side-effects.
///
/// - \p FAMDecl: the \p Decl for the flexible array member. It may not be
/// within the top-level struct.
/// - \p CountDecl: must be within the same non-anonymous struct as \p FAMDecl.
llvm::Value *CodeGenFunction::EmitLoadOfCountedByField(
llvm::Value *CodeGenFunction::GetCountedByFieldExprGEP(
const Expr *Base, const FieldDecl *FAMDecl, const FieldDecl *CountDecl) {
const RecordDecl *RD = CountDecl->getParent()->getOuterLexicalRecordContext();

Expand Down Expand Up @@ -1182,12 +1174,25 @@ llvm::Value *CodeGenFunction::EmitLoadOfCountedByField(
return nullptr;

Indices.push_back(Builder.getInt32(0));
Res = Builder.CreateInBoundsGEP(
ConvertType(QualType(RD->getTypeForDecl(), 0)), Res,
RecIndicesTy(llvm::reverse(Indices)), "..counted_by.gep");
return Builder.CreateGEP(ConvertType(QualType(RD->getTypeForDecl(), 0)), Res,
bwendling marked this conversation as resolved.
Show resolved Hide resolved
RecIndicesTy(llvm::reverse(Indices)),
"..counted_by.gep");
}

return Builder.CreateAlignedLoad(ConvertType(CountDecl->getType()), Res,
getIntAlign(), "..counted_by.load");
/// This method is typically called in contexts where we can't generate
/// side-effects, like in __builtin_dynamic_object_size. When finding
/// expressions, only choose those that have either already been emitted or can
/// be loaded without side-effects.
///
/// - \p FAMDecl: the \p Decl for the flexible array member. It may not be
/// within the top-level struct.
/// - \p CountDecl: must be within the same non-anonymous struct as \p FAMDecl.
llvm::Value *CodeGenFunction::EmitLoadOfCountedByField(
const Expr *Base, const FieldDecl *FAMDecl, const FieldDecl *CountDecl) {
if (llvm::Value *GEP = GetCountedByFieldExprGEP(Base, FAMDecl, CountDecl))
return Builder.CreateAlignedLoad(ConvertType(CountDecl->getType()), GEP,
getIntAlign(), "..counted_by.load");
return nullptr;
}

void CodeGenFunction::EmitBoundsCheck(const Expr *E, const Expr *Base,
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/CodeGen/CodeGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -3305,6 +3305,10 @@ class CodeGenFunction : public CodeGenTypeCache {
const FieldDecl *FAMDecl,
uint64_t &Offset);

llvm::Value *GetCountedByFieldExprGEP(const Expr *Base,
const FieldDecl *FAMDecl,
const FieldDecl *CountDecl);

/// Build an expression accessing the "counted_by" field.
llvm::Value *EmitLoadOfCountedByField(const Expr *Base,
const FieldDecl *FAMDecl,
Expand Down
60 changes: 60 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2971,6 +2971,10 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
}
break;
}
case Builtin::BI__builtin_counted_by_ref:
if (BuiltinCountedByRef(TheCall))
return ExprError();
break;
}

if (getLangOpts().HLSL && HLSL().CheckBuiltinFunctionCall(BuiltinID, TheCall))
Expand Down Expand Up @@ -5573,6 +5577,62 @@ bool Sema::BuiltinSetjmp(CallExpr *TheCall) {
return false;
}

bool Sema::BuiltinCountedByRef(CallExpr *TheCall) {
if (checkArgCount(TheCall, 1))
return true;

ExprResult ArgRes = UsualUnaryConversions(TheCall->getArg(0));
if (ArgRes.isInvalid())
return true;

// For simplicity, we support only limited expressions for the argument.
// Specifically 'ptr->array' and '&ptr->array[0]'. This allows us to reject
// arguments with complex casting, which really shouldn't be a huge problem.
const Expr *Arg = ArgRes.get()->IgnoreParenImpCasts();
if (!isa<PointerType>(Arg->getType()) && !Arg->getType()->isArrayType())
return Diag(Arg->getBeginLoc(),
diag::err_builtin_counted_by_ref_must_be_flex_array_member)
<< Arg->getSourceRange();

if (Arg->HasSideEffects(Context))
Diag(Arg->getBeginLoc(), diag::warn_builtin_counted_by_ref_has_side_effects)
<< Arg->getSourceRange();

// See if we have something like '&ptr->fam[0]`.
if (auto *UO = dyn_cast<UnaryOperator>(Arg);
bwendling marked this conversation as resolved.
Show resolved Hide resolved
UO && UO->getOpcode() == UO_AddrOf) {
Arg = UO->getSubExpr()->IgnoreParenImpCasts();

if (auto *ASE = dyn_cast<ArraySubscriptExpr>(Arg))
Arg = ASE->getBase()->IgnoreParenImpCasts();
}

if (const auto *ME = dyn_cast<MemberExpr>(Arg)) {
if (!ME->isFlexibleArrayMemberLike(
Context, getLangOpts().getStrictFlexArraysLevel()))
return Diag(Arg->getBeginLoc(),
diag::err_builtin_counted_by_ref_must_be_flex_array_member)
<< Arg->getSourceRange();

if (auto *CATy =
ME->getMemberDecl()->getType()->getAs<CountAttributedType>();
CATy && CATy->getKind() == CountAttributedType::CountedBy) {
const auto *FAMDecl = cast<FieldDecl>(ME->getMemberDecl());
if (const FieldDecl *CountFD = FAMDecl->findCountedByField()) {
TheCall->setType(Context.getPointerType(CountFD->getType()));
return false;
}
}
} else {
return Diag(Arg->getBeginLoc(),
diag::err_builtin_counted_by_ref_must_be_flex_array_member)
<< Arg->getSourceRange();
}

TheCall->setType(Context.getPointerType(Context.VoidTy));
return false;
}

namespace {

class UncoveredArgHandler {
Expand Down
Loading