[clang] b185b85 - [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (#116719)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 27 09:42:35 PST 2024
Author: Bill Wendling
Date: 2024-11-27T17:42:32Z
New Revision: b185b8512b2c7bf92ba87ea260a7b94d71dec4ee
URL: https://github.com/llvm/llvm-project/commit/b185b8512b2c7bf92ba87ea260a7b94d71dec4ee
DIFF: https://github.com/llvm/llvm-project/commit/b185b8512b2c7bf92ba87ea260a7b94d71dec4ee.diff
LOG: [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (#116719)
Implement the sema checks with a placeholder. We then check for that
placeholder in all of the places we care to emit a diagnostic.
Fixes: #115520
Added:
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/Sema/builtin-counted-by-ref.c
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c1cdd811db446d..1d777f670097b5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6670,14 +6670,15 @@ def warn_counted_by_attr_elt_type_unknown_size :
// __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_has_side_effects : Error<
+ "'__builtin_counted_by_ref' argument cannot have side-effects">;
+
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 %select{assigned to a "
+ "variable|passed into a function|returned from a function}0">;
+def err_builtin_counted_by_ref_invalid_use : Error<
"value returned by '__builtin_counted_by_ref' cannot be used in "
"%select{an array subscript|a binary}0 expression">;
-def err_builtin_counted_by_ref_has_side_effects : Error<
- "'__builtin_counted_by_ref' argument cannot have side-effects">;
let CategoryName = "ARC Semantic Issue" in {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 24abd5d95dd844..b8684d11460eda 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2524,6 +2524,17 @@ class Sema final : public SemaBase {
bool BuiltinNonDeterministicValue(CallExpr *TheCall);
+ enum BuiltinCountedByRefKind {
+ AssignmentKind,
+ InitializerKind,
+ FunctionArgKind,
+ ReturnArgKind,
+ ArraySubscriptKind,
+ BinaryExprKind,
+ };
+
+ bool CheckInvalidBuiltinCountedByRef(const Expr *E,
+ BuiltinCountedByRefKind K);
bool BuiltinCountedByRef(CallExpr *TheCall);
// Matrix builtin handling.
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a49605e4867651..e071e2b7f33500 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5664,6 +5664,45 @@ bool Sema::BuiltinCountedByRef(CallExpr *TheCall) {
return false;
}
+/// The result of __builtin_counted_by_ref cannot be assigned to a variable.
+/// It allows leaking and modification of bounds safety information.
+bool Sema::CheckInvalidBuiltinCountedByRef(const Expr *E,
+ BuiltinCountedByRefKind K) {
+ const CallExpr *CE =
+ E ? dyn_cast<CallExpr>(E->IgnoreParenImpCasts()) : nullptr;
+ if (!CE || CE->getBuiltinCallee() != Builtin::BI__builtin_counted_by_ref)
+ return false;
+
+ switch (K) {
+ case AssignmentKind:
+ case InitializerKind:
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << 0 << E->getSourceRange();
+ break;
+ case FunctionArgKind:
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << 1 << E->getSourceRange();
+ break;
+ case ReturnArgKind:
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << 2 << E->getSourceRange();
+ break;
+ case ArraySubscriptKind:
+ Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
+ << 0 << E->getSourceRange();
+ break;
+ case BinaryExprKind:
+ Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
+ << 1 << E->getSourceRange();
+ break;
+ }
+
+ return true;
+}
+
namespace {
class UncoveredArgHandler {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 63897dd7a319c2..c33701c0157714 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14699,6 +14699,8 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
}
}
+ CheckInvalidBuiltinCountedByRef(VD->getInit(), InitializerKind);
+
checkAttributesAfterMerging(*this, *VD);
if (VD->isStaticLocal())
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c9d7444d5865a5..a85924f78c9e27 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4897,6 +4897,8 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base,
return ExprError();
}
+ CheckInvalidBuiltinCountedByRef(base, ArraySubscriptKind);
+
// Handle any non-overload placeholder types in the base and index
// expressions. We can't handle overloads here because the other
// operand might be an overloadable type, in which case the overload
@@ -6489,6 +6491,12 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
if (CheckArgsForPlaceholders(ArgExprs))
return ExprError();
+ // The result of __builtin_counted_by_ref cannot be used as a function
+ // argument. It allows leaking and modification of bounds safety information.
+ for (const Expr *Arg : ArgExprs)
+ if (CheckInvalidBuiltinCountedByRef(Arg, FunctionArgKind))
+ return ExprError();
+
if (getLangOpts().CPlusPlus) {
// If this is a pseudo-destructor expression, build the call immediately.
if (isa<CXXPseudoDestructorExpr>(Fn)) {
@@ -9199,38 +9207,6 @@ Sema::CheckAssignmentConstraints(QualType LHSType, ExprResult &RHS,
LHSType = Context.getCanonicalType(LHSType).getUnqualifiedType();
RHSType = Context.getCanonicalType(RHSType).getUnqualifiedType();
- // __builtin_counted_by_ref cannot be assigned to a variable, used in
- // function call, or in a return.
- auto FindBuiltinCountedByRefExpr = [&](Expr *E) -> CallExpr * {
- struct BuiltinCountedByRefVisitor : DynamicRecursiveASTVisitor {
- CallExpr *TheCall = nullptr;
- bool VisitCallExpr(CallExpr *CE) override {
- if (CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref) {
- TheCall = CE;
- return false;
- }
- return true;
- }
- bool
- VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *UE) override {
- // A UnaryExprOrTypeTraitExpr---e.g. sizeof, __alignof, etc.---isn't
- // the same as a CallExpr, so if we find a __builtin_counted_by_ref()
- // call in one, ignore it.
- return false;
- }
- } V;
- V.TraverseStmt(E);
- return V.TheCall;
- };
- static llvm::SmallPtrSet<CallExpr *, 4> Diagnosed;
- if (auto *CE = FindBuiltinCountedByRefExpr(RHS.get());
- CE && !Diagnosed.count(CE)) {
- Diagnosed.insert(CE);
- Diag(CE->getExprLoc(),
- diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << CE->getSourceRange();
- }
-
// Common case: no conversion required.
if (LHSType == RHSType) {
Kind = CK_NoOp;
@@ -13781,42 +13757,6 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
ConvTy = CheckAssignmentConstraints(Loc, LHSType, RHSType);
}
- // __builtin_counted_by_ref can't be used in a binary expression or array
- // subscript on the LHS.
- int DiagOption = -1;
- auto FindInvalidUseOfBoundsSafetyCounter = [&](Expr *E) -> CallExpr * {
- struct BuiltinCountedByRefVisitor : DynamicRecursiveASTVisitor {
- CallExpr *CE = nullptr;
- bool InvalidUse = false;
- int Option = -1;
-
- bool VisitCallExpr(CallExpr *E) override {
- if (E->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref) {
- CE = E;
- return false;
- }
- return true;
- }
-
- bool VisitArraySubscriptExpr(ArraySubscriptExpr *E) override {
- InvalidUse = true;
- Option = 0; // report 'array expression' in diagnostic.
- return true;
- }
- bool VisitBinaryOperator(BinaryOperator *E) override {
- InvalidUse = true;
- Option = 1; // report 'binary expression' in diagnostic.
- return true;
- }
- } V;
- V.TraverseStmt(E);
- DiagOption = V.Option;
- return V.InvalidUse ? V.CE : nullptr;
- };
- if (auto *CE = FindInvalidUseOfBoundsSafetyCounter(LHSExpr))
- Diag(CE->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_lhs_use)
- << DiagOption << CE->getSourceRange();
-
if (DiagnoseAssignmentResult(ConvTy, Loc, LHSType, RHSType, RHS.get(),
AssignmentAction::Assigning))
return QualType();
@@ -15277,6 +15217,12 @@ ExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
if (Kind == tok::TokenKind::slash)
DetectPrecisionLossInComplexDivision(*this, TokLoc, LHSExpr);
+ BuiltinCountedByRefKind K =
+ BinaryOperator::isAssignmentOp(Opc) ? AssignmentKind : BinaryExprKind;
+
+ CheckInvalidBuiltinCountedByRef(LHSExpr, K);
+ CheckInvalidBuiltinCountedByRef(RHSExpr, K);
+
return BuildBinOp(S, TokLoc, Opc, LHSExpr, RHSExpr);
}
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index d6bc66246c758f..0e5c6cd49dccad 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3778,6 +3778,8 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
<< FSI->getFirstCoroutineStmtKeyword();
}
+ CheckInvalidBuiltinCountedByRef(RetVal.get(), ReturnArgKind);
+
StmtResult R =
BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
if (R.isInvalid() || ExprEvalContexts.back().isDiscardedStatementContext())
diff --git a/clang/test/Sema/builtin-counted-by-ref.c b/clang/test/Sema/builtin-counted-by-ref.c
index 5a7ecefcb78976..a9f46a32230065 100644
--- a/clang/test/Sema/builtin-counted-by-ref.c
+++ b/clang/test/Sema/builtin-counted-by-ref.c
@@ -11,62 +11,63 @@ struct fam_struct {
int array[] __attribute__((counted_by(count)));
};
-void test1(struct fam_struct *ptr, int size, int idx) {
- size_t size_of = sizeof(__builtin_counted_by_ref(ptr->array)); // ok
-
- *__builtin_counted_by_ref(ptr->array) = size; // ok
+void g(char *);
+void h(char);
- {
- size_t __ignored_assignment;
- *_Generic(__builtin_counted_by_ref(ptr->array),
- void *: &__ignored_assignment,
- default: __builtin_counted_by_ref(ptr->array)) = 42; // ok
- }
+void test1(struct fam_struct *ptr, int size, int idx) {
+ size_t size_of = sizeof(__builtin_counted_by_ref(ptr->array)); // ok
+ int align_of = __alignof(__builtin_counted_by_ref(ptr->array)); // ok
+ size_t __ignored_assignment;
+
+ *__builtin_counted_by_ref(ptr->array) = size; // ok
+ *_Generic(__builtin_counted_by_ref(ptr->array),
+ void *: &__ignored_assignment,
+ default: __builtin_counted_by_ref(ptr->array)) = 42; // ok
+ h(*__builtin_counted_by_ref(ptr->array)); // ok
}
void test2(struct fam_struct *ptr, int idx) {
- __builtin_counted_by_ref(); // expected-error {{too few arguments to function call, expected 1, have 0}}
- __builtin_counted_by_ref(ptr->array, ptr->x, ptr->count); // expected-error {{too many arguments to function call, expected 1, have 3}}
+ __builtin_counted_by_ref(); // expected-error {{too few arguments to function call, expected 1, have 0}}
+ __builtin_counted_by_ref(ptr->array, ptr->x, ptr->count); // expected-error {{too many arguments to function call, expected 1, have 3}}
}
void test3(struct fam_struct *ptr, int idx) {
- __builtin_counted_by_ref(&ptr->array[0]); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- __builtin_counted_by_ref(&ptr->array[idx]); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- __builtin_counted_by_ref(&ptr->array); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- __builtin_counted_by_ref(ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- __builtin_counted_by_ref(&ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- __builtin_counted_by_ref(global_array); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- __builtin_counted_by_ref(global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- __builtin_counted_by_ref(&global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(&ptr->array[0]); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(&ptr->array[idx]); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(&ptr->array); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(&ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(global_array); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ __builtin_counted_by_ref(&global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
}
void test4(struct fam_struct *ptr, int idx) {
- __builtin_counted_by_ref(ptr++->array); // expected-error {{'__builtin_counted_by_ref' argument cannot have side-effects}}
- __builtin_counted_by_ref(&ptr->array[idx++]); // expected-error {{'__builtin_counted_by_ref' argument cannot have side-effects}}
+ __builtin_counted_by_ref(ptr++->array); // expected-error {{'__builtin_counted_by_ref' argument cannot have side-effects}}
+ __builtin_counted_by_ref(&ptr->array[idx++]); // expected-error {{'__builtin_counted_by_ref' argument cannot have side-effects}}
}
-void foo(char *);
-
void *test5(struct fam_struct *ptr, int size, int idx) {
- char *ref = __builtin_counted_by_ref(ptr->array); // expected-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}}
+ char *ref = __builtin_counted_by_ref(ptr->array); // expected-error {{value returned by '__builtin_counted_by_ref' cannot be assigned to a variable}}
+ char *int_ptr;
+ char *p;
- ref = __builtin_counted_by_ref(ptr->array); // expected-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}}
- ref = (char *)(int *)(42 + &*__builtin_counted_by_ref(ptr->array)); // expected-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}}
- foo(__builtin_counted_by_ref(ptr->array)); // expected-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}}
- foo(ref = __builtin_counted_by_ref(ptr->array)); // expected-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}}
+ ref = __builtin_counted_by_ref(ptr->array); // expected-error {{value returned by '__builtin_counted_by_ref' cannot be assigned to a variable}}
+ g(__builtin_counted_by_ref(ptr->array)); // expected-error {{value returned by '__builtin_counted_by_ref' cannot be passed into a function}}
+ g(ref = __builtin_counted_by_ref(ptr->array)); // expected-error {{value returned by '__builtin_counted_by_ref' cannot be assigned to a variable}}
- if ((ref = __builtin_counted_by_ref(ptr->array))) // expected-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}}
+ if ((ref = __builtin_counted_by_ref(ptr->array))) // expected-error {{value returned by '__builtin_counted_by_ref' cannot be assigned to a variable}}
;
- for (char *p = __builtin_counted_by_ref(ptr->array); p && *p; ++p) // expected-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}}
+ for (p = __builtin_counted_by_ref(ptr->array); p && *p; ++p) // expected-error {{value returned by '__builtin_counted_by_ref' cannot be assigned to a variable}}
;
- return __builtin_counted_by_ref(ptr->array); // expected-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}}
+ return __builtin_counted_by_ref(ptr->array); // expected-error {{value returned by '__builtin_counted_by_ref' cannot be returned from a function}}
}
void test6(struct fam_struct *ptr, int size, int idx) {
- *(__builtin_counted_by_ref(ptr->array) + 4) = 37; // expected-error {{value returned by '__builtin_counted_by_ref' cannot be used in a binary expression}}
- __builtin_counted_by_ref(ptr->array)[3] = 37; // expected-error {{value returned by '__builtin_counted_by_ref' cannot be used in an array subscript expression}}
+ *(__builtin_counted_by_ref(ptr->array) + 4) = 37; // expected-error {{value returned by '__builtin_counted_by_ref' cannot be used in a binary expression}}
+ __builtin_counted_by_ref(ptr->array)[3] = 37; // expected-error {{value returned by '__builtin_counted_by_ref' cannot be used in an array subscript expression}}
}
struct non_fam_struct {
@@ -77,10 +78,10 @@ struct non_fam_struct {
};
void *test7(struct non_fam_struct *ptr, int size) {
- *__builtin_counted_by_ref(ptr->array) = size // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- *__builtin_counted_by_ref(&ptr->array[0]) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- *__builtin_counted_by_ref(ptr->pointer) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
- *__builtin_counted_by_ref(&ptr->pointer[0]) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ *__builtin_counted_by_ref(ptr->array) = size // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ *__builtin_counted_by_ref(&ptr->array[0]) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ *__builtin_counted_by_ref(ptr->pointer) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
+ *__builtin_counted_by_ref(&ptr->pointer[0]) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}}
}
struct char_count {
More information about the cfe-commits
mailing list