[clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
Bill Wendling via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 21 13:34:34 PST 2024
https://github.com/bwendling updated https://github.com/llvm/llvm-project/pull/116719
>From 2dcf18163de2ccce959f46bf82df1fa40e3fd1fc Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Fri, 15 Nov 2024 15:41:48 -0800
Subject: [PATCH 1/8] [Clang] Improve Sema diagnostic performance for
__builtin_counted_by_ref
Implement the sema checks with a placeholder. We then check for that
placeholder in all of the places we care to emit a diagnostic.
---
clang/include/clang/AST/ASTContext.h | 1 +
clang/include/clang/AST/BuiltinTypes.def | 3 +
.../clang/Basic/DiagnosticSemaKinds.td | 2 +-
.../include/clang/Serialization/ASTBitCodes.h | 5 +-
clang/lib/AST/ASTContext.cpp | 4 +
clang/lib/AST/NSAPI.cpp | 1 +
clang/lib/AST/Type.cpp | 3 +
clang/lib/AST/TypeLoc.cpp | 1 +
clang/lib/Sema/SemaChecking.cpp | 1 +
clang/lib/Sema/SemaDecl.cpp | 11 ++
clang/lib/Sema/SemaExpr.cpp | 138 +++++++++---------
clang/lib/Sema/SemaStmt.cpp | 10 ++
clang/lib/Serialization/ASTCommon.cpp | 3 +
clang/lib/Serialization/ASTReader.cpp | 3 +
clang/test/Modules/no-external-type-id.cppm | 2 +-
clang/test/Sema/builtin-counted-by-ref.c | 77 +++++-----
.../TypeSystem/Clang/TypeSystemClang.cpp | 2 +
17 files changed, 154 insertions(+), 113 deletions(-)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 89fcb6789d880a..39cad95d911a33 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1184,6 +1184,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
CanQualType DependentTy, OverloadTy, BoundMemberTy, UnresolvedTemplateTy,
UnknownAnyTy;
CanQualType BuiltinFnTy;
+ CanQualType BuiltinCountedByRefTy;
CanQualType PseudoObjectTy, ARCUnbridgedCastTy;
CanQualType ObjCBuiltinIdTy, ObjCBuiltinClassTy, ObjCBuiltinSelTy;
CanQualType ObjCBuiltinBoolTy;
diff --git a/clang/include/clang/AST/BuiltinTypes.def b/clang/include/clang/AST/BuiltinTypes.def
index 444be4311a7431..4eae6962a46de6 100644
--- a/clang/include/clang/AST/BuiltinTypes.def
+++ b/clang/include/clang/AST/BuiltinTypes.def
@@ -314,6 +314,9 @@ PLACEHOLDER_TYPE(UnknownAny, UnknownAnyTy)
PLACEHOLDER_TYPE(BuiltinFn, BuiltinFnTy)
+// A placeholder type for __builtin_counted_by_ref.
+PLACEHOLDER_TYPE(BuiltinCountedByRef, BuiltinCountedByRefTy)
+
// The type of a cast which, in ARC, would normally require a
// __bridge, but which might be okay depending on the immediate
// context.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3caf471d3037f9..37fb44d4bf74cd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6673,7 +6673,7 @@ def err_builtin_counted_by_ref_must_be_flex_array_member : Error<
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<
+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<
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index fd834c14ce790f..f4b71861968e77 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1111,6 +1111,9 @@ enum PredefinedTypeIDs {
/// \brief The '__ibm128' type
PREDEF_TYPE_IBM128_ID = 74,
+ /// \brief The placeholder type for __builtin_counted_by_ref.
+ PREDEF_TYPE_BUILTIN_COUNTED_BY_REF = 75,
+
/// OpenCL image types with auto numeration
#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
PREDEF_TYPE_##Id##_ID,
@@ -1148,7 +1151,7 @@ enum PredefinedTypeIDs {
///
/// Type IDs for non-predefined types will start at
/// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 513;
+const unsigned NUM_PREDEF_TYPE_IDS = 514;
// Ensure we do not overrun the predefined types we reserved
// in the enum PredefinedTypeIDs above.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 14fbadbc35ae5d..06226afaf23aab 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1390,6 +1390,10 @@ void ASTContext::InitBuiltinTypes(const TargetInfo &Target,
if (LangOpts.MatrixTypes)
InitBuiltinType(IncompleteMatrixIdxTy, BuiltinType::IncompleteMatrixIdx);
+ // Placeholder for __builtin_counted_by_ref().
+ if (!LangOpts.CPlusPlus)
+ InitBuiltinType(BuiltinCountedByRefTy, BuiltinType::BuiltinCountedByRef);
+
// Builtin types for 'id', 'Class', and 'SEL'.
InitBuiltinType(ObjCBuiltinIdTy, BuiltinType::ObjCId);
InitBuiltinType(ObjCBuiltinClassTy, BuiltinType::ObjCClass);
diff --git a/clang/lib/AST/NSAPI.cpp b/clang/lib/AST/NSAPI.cpp
index 311fec32bbfa90..6a722b89af6205 100644
--- a/clang/lib/AST/NSAPI.cpp
+++ b/clang/lib/AST/NSAPI.cpp
@@ -466,6 +466,7 @@ NSAPI::getNSNumberFactoryMethodKind(QualType T) const {
case BuiltinType::Half:
case BuiltinType::PseudoObject:
case BuiltinType::BuiltinFn:
+ case BuiltinType::BuiltinCountedByRef:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
case BuiltinType::OMPArrayShaping:
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index b70f86ef31442d..1e44c2bd234fbb 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3444,6 +3444,8 @@ StringRef BuiltinType::getName(const PrintingPolicy &Policy) const {
return "<ARC unbridged cast type>";
case BuiltinFn:
return "<builtin fn type>";
+ case BuiltinCountedByRef:
+ return "<builtin counted by ref type>";
case ObjCId:
return "id";
case ObjCClass:
@@ -4861,6 +4863,7 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
#define HLSL_INTANGIBLE_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
#include "clang/Basic/HLSLIntangibleTypes.def"
case BuiltinType::BuiltinFn:
+ case BuiltinType::BuiltinCountedByRef:
case BuiltinType::NullPtr:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
diff --git a/clang/lib/AST/TypeLoc.cpp b/clang/lib/AST/TypeLoc.cpp
index fbb7fc5cd76902..088bd5c379d8c4 100644
--- a/clang/lib/AST/TypeLoc.cpp
+++ b/clang/lib/AST/TypeLoc.cpp
@@ -433,6 +433,7 @@ TypeSpecifierType BuiltinTypeLoc::getWrittenTypeSpec() const {
#define HLSL_INTANGIBLE_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
#include "clang/Basic/HLSLIntangibleTypes.def"
case BuiltinType::BuiltinFn:
+ case BuiltinType::BuiltinCountedByRef:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
case BuiltinType::OMPArrayShaping:
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2d4a7cd287b70d..52b47a995624f4 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -755,6 +755,7 @@ static ExprResult BuiltinDumpStruct(Sema &S, CallExpr *TheCall) {
case BuiltinType::PseudoObject:
case BuiltinType::UnknownAny:
case BuiltinType::BuiltinFn:
+ case BuiltinType::BuiltinCountedByRef:
// This might be a callable.
break;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a36ca61a1bef30..d4a90cf262b09f 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14690,6 +14690,17 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
}
}
+ // The result of __builtin_counted_by_ref cannot be assigned to a variable.
+ // It allows leaking and modification of bounds safety information.
+ if (const auto *CE = dyn_cast_if_present<CallExpr>(VD->getInit())) {
+ const Expr *E = CE->getCallee()->IgnoreParenImpCasts();
+ if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
+ PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef)
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << E->getSourceRange();
+ }
+
checkAttributesAfterMerging(*this, *VD);
if (VD->isStaticLocal())
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index dcf495b700540f..f1925bae9b91e3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3382,7 +3382,9 @@ ExprResult Sema::BuildDeclarationNameExpr(
case Decl::Function: {
if (unsigned BID = cast<FunctionDecl>(VD)->getBuiltinID()) {
if (!Context.BuiltinInfo.isDirectlyAddressable(BID)) {
- type = Context.BuiltinFnTy;
+ type = (BID == Builtin::BI__builtin_counted_by_ref)
+ ? Context.BuiltinCountedByRefTy
+ : Context.BuiltinFnTy;
valueKind = VK_PRValue;
break;
}
@@ -4894,6 +4896,18 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base,
return ExprError();
}
+ // We cannot use __builtin_counted_by_ref in a binary expression. It's
+ // possible to leak the reference and violate bounds security.
+ Expr *E = base->IgnoreParenImpCasts();
+ if (auto *CE = dyn_cast<CallExpr>(E)) {
+ E = CE->getCallee()->IgnoreParenImpCasts();
+ if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
+ PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
+ Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
+ << 0 << E->getSourceRange();
+ }
+ }
+
// 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
@@ -6188,6 +6202,7 @@ static bool isPlaceholderToRemoveAsArg(QualType type) {
// These are always invalid as call arguments and should be reported.
case BuiltinType::BoundMember:
case BuiltinType::BuiltinFn:
+ case BuiltinType::BuiltinCountedByRef:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
case BuiltinType::OMPArrayShaping:
@@ -6205,10 +6220,27 @@ bool Sema::CheckArgsForPlaceholders(MultiExprArg args) {
for (size_t i = 0, e = args.size(); i != e; i++) {
if (isPlaceholderToRemoveAsArg(args[i]->getType())) {
ExprResult result = CheckPlaceholderExpr(args[i]);
- if (result.isInvalid()) hasInvalid = true;
- else args[i] = result.get();
+ if (result.isInvalid())
+ hasInvalid = true;
+ else
+ args[i] = result.get();
+ }
+
+ // The result of __builtin_counted_by_ref cannot be used as a function
+ // argument. It allows leaking and modification of bounds safety
+ // information.
+ if (const auto *CE = dyn_cast<CallExpr>(args[i])) {
+ const Expr *E = CE->getCallee()->IgnoreParenImpCasts();
+ if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
+ PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
+ hasInvalid = true;
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << E->getSourceRange();
+ }
}
}
+
return hasInvalid;
}
@@ -6770,7 +6802,9 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
ExprResult Result;
QualType ResultTy;
if (BuiltinID &&
- Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
+ (Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn) ||
+ Fn->getType()->isSpecificBuiltinType(
+ BuiltinType::BuiltinCountedByRef))) {
// Extract the return type from the (builtin) function pointer type.
// FIXME Several builtins still have setType in
// Sema::CheckBuiltinFunctionCall. One should review their definitions in
@@ -9196,38 +9230,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;
@@ -13778,42 +13780,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();
@@ -15274,6 +15240,27 @@ ExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
if (Kind == tok::TokenKind::slash)
DetectPrecisionLossInComplexDivision(*this, TokLoc, LHSExpr);
+ // We cannot use __builtin_counted_by_ref in a binary expression. It's
+ // possible to leak the reference and violate bounds security.
+ auto CheckBuiltinCountedByRefPlaceholder = [&](const Expr *E) {
+ if (const auto *CE = dyn_cast<CallExpr>(E->IgnoreParenImpCasts())) {
+ E = CE->getCallee()->IgnoreParenImpCasts();
+ if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
+ PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
+ if (BinaryOperator::isAssignmentOp(Opc))
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << E->getSourceRange();
+ else
+ Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
+ << 1 << E->getSourceRange();
+ }
+ }
+ };
+
+ CheckBuiltinCountedByRefPlaceholder(LHSExpr);
+ CheckBuiltinCountedByRefPlaceholder(RHSExpr);
+
return BuildBinOp(S, TokLoc, Opc, LHSExpr, RHSExpr);
}
@@ -21074,6 +21061,13 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
return ExprError();
}
+ case BuiltinType::BuiltinCountedByRef: {
+ Diag(E->IgnoreParens()->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << E->IgnoreParens()->getSourceRange();
+ return ExprError();
+ }
+
case BuiltinType::IncompleteMatrixIdx:
Diag(cast<MatrixSubscriptExpr>(E->IgnoreParens())
->getRowIdx()
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index f3ee5211acdd11..024dc05f6c5636 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3765,6 +3765,16 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
<< FSI->getFirstCoroutineStmtKeyword();
}
+ if (const auto *CE = dyn_cast_if_present<CallExpr>(RetVal.get())) {
+ const Expr *E = CE->getCallee()->IgnoreParenImpCasts();
+ if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
+ PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << E->getSourceRange();
+ }
+ }
+
StmtResult R =
BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
if (R.isInvalid() || ExprEvalContexts.back().isDiscardedStatementContext())
diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp
index ec18e84255ca8e..e7f54cf8839c70 100644
--- a/clang/lib/Serialization/ASTCommon.cpp
+++ b/clang/lib/Serialization/ASTCommon.cpp
@@ -274,6 +274,9 @@ serialization::TypeIdxFromBuiltin(const BuiltinType *BT) {
case BuiltinType::BuiltinFn:
ID = PREDEF_TYPE_BUILTIN_FN;
break;
+ case BuiltinType::BuiltinCountedByRef:
+ ID = PREDEF_TYPE_BUILTIN_COUNTED_BY_REF;
+ break;
case BuiltinType::IncompleteMatrixIdx:
ID = PREDEF_TYPE_INCOMPLETE_MATRIX_IDX;
break;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index ec85fad3389a1c..76dfaa5481e424 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7458,6 +7458,9 @@ QualType ASTReader::GetType(TypeID ID) {
case PREDEF_TYPE_BUILTIN_FN:
T = Context.BuiltinFnTy;
break;
+ case PREDEF_TYPE_BUILTIN_COUNTED_BY_REF:
+ T = Context.BuiltinCountedByRefTy;
+ break;
case PREDEF_TYPE_INCOMPLETE_MATRIX_IDX:
T = Context.IncompleteMatrixIdxTy;
break;
diff --git a/clang/test/Modules/no-external-type-id.cppm b/clang/test/Modules/no-external-type-id.cppm
index d067e574e72e37..162300478632db 100644
--- a/clang/test/Modules/no-external-type-id.cppm
+++ b/clang/test/Modules/no-external-type-id.cppm
@@ -23,7 +23,7 @@ export module b;
import a;
export int b();
-// CHECK: <DECL_FUNCTION {{.*}} op8=4120
+// CHECK: <DECL_FUNCTION {{.*}} op8=4128
// CHECK: <TYPE_FUNCTION_PROTO
//--- a.v1.cppm
diff --git a/clang/test/Sema/builtin-counted-by-ref.c b/clang/test/Sema/builtin-counted-by-ref.c
index 5a7ecefcb78976..c21c7093468dde 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, have its address taken, or passed into or returned from a function}}
+ 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, have its address taken, or passed into or returned from a function}}
+ g(__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}}
+ g(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, 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, have its address taken, or passed into or returned from a function}}
;
- 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, 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 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 assigned to a variable, have its address taken, or passed into or 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 {
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 5f8163211857c3..4e02acff3f2076 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4951,6 +4951,7 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
case clang::BuiltinType::Kind::ARCUnbridgedCast:
case clang::BuiltinType::Kind::BoundMember:
case clang::BuiltinType::Kind::BuiltinFn:
+ case clang::BuiltinType::Kind::BuiltinCountedByRef:
case clang::BuiltinType::Kind::Dependent:
case clang::BuiltinType::Kind::OCLClkEvent:
case clang::BuiltinType::Kind::OCLEvent:
@@ -6111,6 +6112,7 @@ uint32_t TypeSystemClang::GetNumPointeeChildren(clang::QualType type) {
case clang::BuiltinType::ARCUnbridgedCast:
case clang::BuiltinType::PseudoObject:
case clang::BuiltinType::BuiltinFn:
+ case clang::BuiltinType::BuiltinCountedByRef:
case clang::BuiltinType::ArraySection:
return 1;
default:
>From b32983c5159634df24fbb53c47e0e5bb3e8f2f45 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Tue, 19 Nov 2024 17:32:09 -0800
Subject: [PATCH 2/8] Don't use a placeholder. Instead, place diagnostics in
the situations we want to disallow.
---
clang/include/clang/AST/ASTContext.h | 1 -
clang/include/clang/AST/BuiltinTypes.def | 3 -
.../include/clang/Serialization/ASTBitCodes.h | 5 +-
clang/lib/AST/ASTContext.cpp | 4 --
clang/lib/AST/NSAPI.cpp | 1 -
clang/lib/AST/Type.cpp | 3 -
clang/lib/AST/TypeLoc.cpp | 1 -
clang/lib/Sema/SemaChecking.cpp | 1 -
clang/lib/Sema/SemaDecl.cpp | 13 ++--
clang/lib/Sema/SemaExpr.cpp | 71 +++++++------------
clang/lib/Sema/SemaStmt.cpp | 14 ++--
clang/lib/Serialization/ASTCommon.cpp | 3 -
clang/lib/Serialization/ASTReader.cpp | 3 -
clang/test/Modules/no-external-type-id.cppm | 2 +-
14 files changed, 36 insertions(+), 89 deletions(-)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 39cad95d911a33..89fcb6789d880a 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1184,7 +1184,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
CanQualType DependentTy, OverloadTy, BoundMemberTy, UnresolvedTemplateTy,
UnknownAnyTy;
CanQualType BuiltinFnTy;
- CanQualType BuiltinCountedByRefTy;
CanQualType PseudoObjectTy, ARCUnbridgedCastTy;
CanQualType ObjCBuiltinIdTy, ObjCBuiltinClassTy, ObjCBuiltinSelTy;
CanQualType ObjCBuiltinBoolTy;
diff --git a/clang/include/clang/AST/BuiltinTypes.def b/clang/include/clang/AST/BuiltinTypes.def
index 4eae6962a46de6..444be4311a7431 100644
--- a/clang/include/clang/AST/BuiltinTypes.def
+++ b/clang/include/clang/AST/BuiltinTypes.def
@@ -314,9 +314,6 @@ PLACEHOLDER_TYPE(UnknownAny, UnknownAnyTy)
PLACEHOLDER_TYPE(BuiltinFn, BuiltinFnTy)
-// A placeholder type for __builtin_counted_by_ref.
-PLACEHOLDER_TYPE(BuiltinCountedByRef, BuiltinCountedByRefTy)
-
// The type of a cast which, in ARC, would normally require a
// __bridge, but which might be okay depending on the immediate
// context.
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index f4b71861968e77..fd834c14ce790f 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -1111,9 +1111,6 @@ enum PredefinedTypeIDs {
/// \brief The '__ibm128' type
PREDEF_TYPE_IBM128_ID = 74,
- /// \brief The placeholder type for __builtin_counted_by_ref.
- PREDEF_TYPE_BUILTIN_COUNTED_BY_REF = 75,
-
/// OpenCL image types with auto numeration
#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
PREDEF_TYPE_##Id##_ID,
@@ -1151,7 +1148,7 @@ enum PredefinedTypeIDs {
///
/// Type IDs for non-predefined types will start at
/// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 514;
+const unsigned NUM_PREDEF_TYPE_IDS = 513;
// Ensure we do not overrun the predefined types we reserved
// in the enum PredefinedTypeIDs above.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 06226afaf23aab..14fbadbc35ae5d 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1390,10 +1390,6 @@ void ASTContext::InitBuiltinTypes(const TargetInfo &Target,
if (LangOpts.MatrixTypes)
InitBuiltinType(IncompleteMatrixIdxTy, BuiltinType::IncompleteMatrixIdx);
- // Placeholder for __builtin_counted_by_ref().
- if (!LangOpts.CPlusPlus)
- InitBuiltinType(BuiltinCountedByRefTy, BuiltinType::BuiltinCountedByRef);
-
// Builtin types for 'id', 'Class', and 'SEL'.
InitBuiltinType(ObjCBuiltinIdTy, BuiltinType::ObjCId);
InitBuiltinType(ObjCBuiltinClassTy, BuiltinType::ObjCClass);
diff --git a/clang/lib/AST/NSAPI.cpp b/clang/lib/AST/NSAPI.cpp
index 6a722b89af6205..311fec32bbfa90 100644
--- a/clang/lib/AST/NSAPI.cpp
+++ b/clang/lib/AST/NSAPI.cpp
@@ -466,7 +466,6 @@ NSAPI::getNSNumberFactoryMethodKind(QualType T) const {
case BuiltinType::Half:
case BuiltinType::PseudoObject:
case BuiltinType::BuiltinFn:
- case BuiltinType::BuiltinCountedByRef:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
case BuiltinType::OMPArrayShaping:
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 1e44c2bd234fbb..b70f86ef31442d 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3444,8 +3444,6 @@ StringRef BuiltinType::getName(const PrintingPolicy &Policy) const {
return "<ARC unbridged cast type>";
case BuiltinFn:
return "<builtin fn type>";
- case BuiltinCountedByRef:
- return "<builtin counted by ref type>";
case ObjCId:
return "id";
case ObjCClass:
@@ -4863,7 +4861,6 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const {
#define HLSL_INTANGIBLE_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
#include "clang/Basic/HLSLIntangibleTypes.def"
case BuiltinType::BuiltinFn:
- case BuiltinType::BuiltinCountedByRef:
case BuiltinType::NullPtr:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
diff --git a/clang/lib/AST/TypeLoc.cpp b/clang/lib/AST/TypeLoc.cpp
index 088bd5c379d8c4..fbb7fc5cd76902 100644
--- a/clang/lib/AST/TypeLoc.cpp
+++ b/clang/lib/AST/TypeLoc.cpp
@@ -433,7 +433,6 @@ TypeSpecifierType BuiltinTypeLoc::getWrittenTypeSpec() const {
#define HLSL_INTANGIBLE_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
#include "clang/Basic/HLSLIntangibleTypes.def"
case BuiltinType::BuiltinFn:
- case BuiltinType::BuiltinCountedByRef:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
case BuiltinType::OMPArrayShaping:
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 52b47a995624f4..2d4a7cd287b70d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -755,7 +755,6 @@ static ExprResult BuiltinDumpStruct(Sema &S, CallExpr *TheCall) {
case BuiltinType::PseudoObject:
case BuiltinType::UnknownAny:
case BuiltinType::BuiltinFn:
- case BuiltinType::BuiltinCountedByRef:
// This might be a callable.
break;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d4a90cf262b09f..695c8bafea72e6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14692,14 +14692,11 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
// The result of __builtin_counted_by_ref cannot be assigned to a variable.
// It allows leaking and modification of bounds safety information.
- if (const auto *CE = dyn_cast_if_present<CallExpr>(VD->getInit())) {
- const Expr *E = CE->getCallee()->IgnoreParenImpCasts();
- if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
- PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef)
- Diag(E->getExprLoc(),
- diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << E->getSourceRange();
- }
+ if (const auto *CE = dyn_cast_if_present<CallExpr>(VD->getInit());
+ CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref)
+ Diag(CE->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << CE->getSourceRange();
checkAttributesAfterMerging(*this, *VD);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index f1925bae9b91e3..83b2c95c6bfba0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3382,9 +3382,7 @@ ExprResult Sema::BuildDeclarationNameExpr(
case Decl::Function: {
if (unsigned BID = cast<FunctionDecl>(VD)->getBuiltinID()) {
if (!Context.BuiltinInfo.isDirectlyAddressable(BID)) {
- type = (BID == Builtin::BI__builtin_counted_by_ref)
- ? Context.BuiltinCountedByRefTy
- : Context.BuiltinFnTy;
+ type = Context.BuiltinFnTy;
valueKind = VK_PRValue;
break;
}
@@ -4898,15 +4896,10 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base,
// We cannot use __builtin_counted_by_ref in a binary expression. It's
// possible to leak the reference and violate bounds security.
- Expr *E = base->IgnoreParenImpCasts();
- if (auto *CE = dyn_cast<CallExpr>(E)) {
- E = CE->getCallee()->IgnoreParenImpCasts();
- if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
- PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
- Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
- << 0 << E->getSourceRange();
- }
- }
+ if (auto *CE = dyn_cast<CallExpr>(base->IgnoreParenImpCasts());
+ CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref)
+ Diag(CE->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
+ << 0 << CE->getSourceRange();
// Handle any non-overload placeholder types in the base and index
// expressions. We can't handle overloads here because the other
@@ -6202,7 +6195,6 @@ static bool isPlaceholderToRemoveAsArg(QualType type) {
// These are always invalid as call arguments and should be reported.
case BuiltinType::BoundMember:
case BuiltinType::BuiltinFn:
- case BuiltinType::BuiltinCountedByRef:
case BuiltinType::IncompleteMatrixIdx:
case BuiltinType::ArraySection:
case BuiltinType::OMPArrayShaping:
@@ -6229,15 +6221,12 @@ bool Sema::CheckArgsForPlaceholders(MultiExprArg args) {
// The result of __builtin_counted_by_ref cannot be used as a function
// argument. It allows leaking and modification of bounds safety
// information.
- if (const auto *CE = dyn_cast<CallExpr>(args[i])) {
- const Expr *E = CE->getCallee()->IgnoreParenImpCasts();
- if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
- PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
- hasInvalid = true;
- Diag(E->getExprLoc(),
- diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << E->getSourceRange();
- }
+ if (const auto *CE = dyn_cast<CallExpr>(args[i]);
+ CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref) {
+ hasInvalid = true;
+ Diag(CE->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << CE->getSourceRange();
}
}
@@ -6802,9 +6791,7 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
ExprResult Result;
QualType ResultTy;
if (BuiltinID &&
- (Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn) ||
- Fn->getType()->isSpecificBuiltinType(
- BuiltinType::BuiltinCountedByRef))) {
+ Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
// Extract the return type from the (builtin) function pointer type.
// FIXME Several builtins still have setType in
// Sema::CheckBuiltinFunctionCall. One should review their definitions in
@@ -15242,24 +15229,21 @@ ExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
// We cannot use __builtin_counted_by_ref in a binary expression. It's
// possible to leak the reference and violate bounds security.
- auto CheckBuiltinCountedByRefPlaceholder = [&](const Expr *E) {
- if (const auto *CE = dyn_cast<CallExpr>(E->IgnoreParenImpCasts())) {
- E = CE->getCallee()->IgnoreParenImpCasts();
- if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
- PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
- if (BinaryOperator::isAssignmentOp(Opc))
- Diag(E->getExprLoc(),
- diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << E->getSourceRange();
- else
- Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
- << 1 << E->getSourceRange();
- }
+ auto CheckBuiltinCountedByRef = [&](const Expr *E) {
+ if (const auto *CE = dyn_cast<CallExpr>(E->IgnoreParenImpCasts());
+ CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref) {
+ if (BinaryOperator::isAssignmentOp(Opc))
+ Diag(E->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << E->getSourceRange();
+ else
+ Diag(E->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
+ << 1 << E->getSourceRange();
}
};
- CheckBuiltinCountedByRefPlaceholder(LHSExpr);
- CheckBuiltinCountedByRefPlaceholder(RHSExpr);
+ CheckBuiltinCountedByRef(LHSExpr);
+ CheckBuiltinCountedByRef(RHSExpr);
return BuildBinOp(S, TokLoc, Opc, LHSExpr, RHSExpr);
}
@@ -21061,13 +21045,6 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
return ExprError();
}
- case BuiltinType::BuiltinCountedByRef: {
- Diag(E->IgnoreParens()->getExprLoc(),
- diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << E->IgnoreParens()->getSourceRange();
- return ExprError();
- }
-
case BuiltinType::IncompleteMatrixIdx:
Diag(cast<MatrixSubscriptExpr>(E->IgnoreParens())
->getRowIdx()
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 024dc05f6c5636..6dc5867ed71eb9 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3765,15 +3765,11 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
<< FSI->getFirstCoroutineStmtKeyword();
}
- if (const auto *CE = dyn_cast_if_present<CallExpr>(RetVal.get())) {
- const Expr *E = CE->getCallee()->IgnoreParenImpCasts();
- if (const BuiltinType *PTy = E->getType()->getAsPlaceholderType();
- PTy && PTy->getKind() == BuiltinType::BuiltinCountedByRef) {
- Diag(E->getExprLoc(),
- diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << E->getSourceRange();
- }
- }
+ if (const auto *CE = dyn_cast_if_present<CallExpr>(RetVal.get());
+ CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref)
+ Diag(CE->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << CE->getSourceRange();
StmtResult R =
BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp
index e7f54cf8839c70..ec18e84255ca8e 100644
--- a/clang/lib/Serialization/ASTCommon.cpp
+++ b/clang/lib/Serialization/ASTCommon.cpp
@@ -274,9 +274,6 @@ serialization::TypeIdxFromBuiltin(const BuiltinType *BT) {
case BuiltinType::BuiltinFn:
ID = PREDEF_TYPE_BUILTIN_FN;
break;
- case BuiltinType::BuiltinCountedByRef:
- ID = PREDEF_TYPE_BUILTIN_COUNTED_BY_REF;
- break;
case BuiltinType::IncompleteMatrixIdx:
ID = PREDEF_TYPE_INCOMPLETE_MATRIX_IDX;
break;
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 76dfaa5481e424..ec85fad3389a1c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7458,9 +7458,6 @@ QualType ASTReader::GetType(TypeID ID) {
case PREDEF_TYPE_BUILTIN_FN:
T = Context.BuiltinFnTy;
break;
- case PREDEF_TYPE_BUILTIN_COUNTED_BY_REF:
- T = Context.BuiltinCountedByRefTy;
- break;
case PREDEF_TYPE_INCOMPLETE_MATRIX_IDX:
T = Context.IncompleteMatrixIdxTy;
break;
diff --git a/clang/test/Modules/no-external-type-id.cppm b/clang/test/Modules/no-external-type-id.cppm
index 162300478632db..d067e574e72e37 100644
--- a/clang/test/Modules/no-external-type-id.cppm
+++ b/clang/test/Modules/no-external-type-id.cppm
@@ -23,7 +23,7 @@ export module b;
import a;
export int b();
-// CHECK: <DECL_FUNCTION {{.*}} op8=4128
+// CHECK: <DECL_FUNCTION {{.*}} op8=4120
// CHECK: <TYPE_FUNCTION_PROTO
//--- a.v1.cppm
>From b8d93b60db2a013ff2feac44b22d4011157006f8 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Tue, 19 Nov 2024 17:39:19 -0800
Subject: [PATCH 3/8] Remove removed placeholder type.
---
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 4e02acff3f2076..5f8163211857c3 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4951,7 +4951,6 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
case clang::BuiltinType::Kind::ARCUnbridgedCast:
case clang::BuiltinType::Kind::BoundMember:
case clang::BuiltinType::Kind::BuiltinFn:
- case clang::BuiltinType::Kind::BuiltinCountedByRef:
case clang::BuiltinType::Kind::Dependent:
case clang::BuiltinType::Kind::OCLClkEvent:
case clang::BuiltinType::Kind::OCLEvent:
@@ -6112,7 +6111,6 @@ uint32_t TypeSystemClang::GetNumPointeeChildren(clang::QualType type) {
case clang::BuiltinType::ARCUnbridgedCast:
case clang::BuiltinType::PseudoObject:
case clang::BuiltinType::BuiltinFn:
- case clang::BuiltinType::BuiltinCountedByRef:
case clang::BuiltinType::ArraySection:
return 1;
default:
>From 74109c4c03a689fde9686cde24e23f77120d09ec Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Wed, 20 Nov 2024 13:24:59 -0800
Subject: [PATCH 4/8] Remove unused code.
---
clang/lib/Sema/SemaExpr.cpp | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 83b2c95c6bfba0..1f4d903c4d2b80 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6212,24 +6212,10 @@ bool Sema::CheckArgsForPlaceholders(MultiExprArg args) {
for (size_t i = 0, e = args.size(); i != e; i++) {
if (isPlaceholderToRemoveAsArg(args[i]->getType())) {
ExprResult result = CheckPlaceholderExpr(args[i]);
- if (result.isInvalid())
- hasInvalid = true;
- else
- args[i] = result.get();
- }
-
- // The result of __builtin_counted_by_ref cannot be used as a function
- // argument. It allows leaking and modification of bounds safety
- // information.
- if (const auto *CE = dyn_cast<CallExpr>(args[i]);
- CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref) {
- hasInvalid = true;
- Diag(CE->getExprLoc(),
- diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << CE->getSourceRange();
+ if (result.isInvalid()) hasInvalid = true;
+ else args[i] = result.get();
}
}
-
return hasInvalid;
}
>From ee9e2b63a3c3cb2e7e383ec463414d553513d599 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Wed, 20 Nov 2024 13:40:04 -0800
Subject: [PATCH 5/8] Extract check for __builtin_counted_by_ref CallExpr into
a helper method.
---
clang/include/clang/Sema/Sema.h | 5 +++++
clang/lib/Sema/SemaExpr.cpp | 20 ++++++++++++++------
clang/lib/Sema/SemaStmt.cpp | 7 +++----
3 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d6f3508a5243f3..8f4b6c61fd9d11 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2518,6 +2518,11 @@ class Sema final : public SemaBase {
bool BuiltinNonDeterministicValue(CallExpr *TheCall);
+ bool IsBuiltinCountedByRef(const Expr *E) {
+ const CallExpr *CE = E ? dyn_cast<CallExpr>(E->IgnoreParenImpCasts())
+ : nullptr;
+ return CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref;
+ }
bool BuiltinCountedByRef(CallExpr *TheCall);
// Matrix builtin handling.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 1f4d903c4d2b80..26b951f54b3ced 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4896,10 +4896,9 @@ ExprResult Sema::ActOnArraySubscriptExpr(Scope *S, Expr *base,
// We cannot use __builtin_counted_by_ref in a binary expression. It's
// possible to leak the reference and violate bounds security.
- if (auto *CE = dyn_cast<CallExpr>(base->IgnoreParenImpCasts());
- CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref)
- Diag(CE->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
- << 0 << CE->getSourceRange();
+ if (IsBuiltinCountedByRef(base))
+ Diag(base->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use)
+ << 0 << base->getSourceRange();
// Handle any non-overload placeholder types in the base and index
// expressions. We can't handle overloads here because the other
@@ -6493,6 +6492,16 @@ 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 (IsBuiltinCountedByRef(Arg)) {
+ Diag(Arg->getExprLoc(),
+ diag::err_builtin_counted_by_ref_cannot_leak_reference)
+ << Arg->getSourceRange();
+ return ExprError();
+ }
+
if (getLangOpts().CPlusPlus) {
// If this is a pseudo-destructor expression, build the call immediately.
if (isa<CXXPseudoDestructorExpr>(Fn)) {
@@ -15216,8 +15225,7 @@ ExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
// We cannot use __builtin_counted_by_ref in a binary expression. It's
// possible to leak the reference and violate bounds security.
auto CheckBuiltinCountedByRef = [&](const Expr *E) {
- if (const auto *CE = dyn_cast<CallExpr>(E->IgnoreParenImpCasts());
- CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref) {
+ if (IsBuiltinCountedByRef(E)) {
if (BinaryOperator::isAssignmentOp(Opc))
Diag(E->getExprLoc(),
diag::err_builtin_counted_by_ref_cannot_leak_reference)
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 6dc5867ed71eb9..06ca5231f61dba 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3765,11 +3765,10 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
<< FSI->getFirstCoroutineStmtKeyword();
}
- if (const auto *CE = dyn_cast_if_present<CallExpr>(RetVal.get());
- CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref)
- Diag(CE->getExprLoc(),
+ if (IsBuiltinCountedByRef(RetVal.get()))
+ Diag(RetVal.get()->getExprLoc(),
diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << CE->getSourceRange();
+ << RetVal.get()->getSourceRange();
StmtResult R =
BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
>From 54b277c34f5cf81c996b952a984322e168d142cf Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Wed, 20 Nov 2024 13:40:30 -0800
Subject: [PATCH 6/8] Reformat.
---
clang/include/clang/Sema/Sema.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8f4b6c61fd9d11..769e72fb84bd3e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2519,8 +2519,8 @@ class Sema final : public SemaBase {
bool BuiltinNonDeterministicValue(CallExpr *TheCall);
bool IsBuiltinCountedByRef(const Expr *E) {
- const CallExpr *CE = E ? dyn_cast<CallExpr>(E->IgnoreParenImpCasts())
- : nullptr;
+ const CallExpr *CE =
+ E ? dyn_cast<CallExpr>(E->IgnoreParenImpCasts()) : nullptr;
return CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref;
}
bool BuiltinCountedByRef(CallExpr *TheCall);
>From 8ab376a9e8cd111d930c85bbc47e6bdce5545bbe Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Wed, 20 Nov 2024 13:42:10 -0800
Subject: [PATCH 7/8] Turn into a const method.
---
clang/include/clang/Sema/Sema.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 769e72fb84bd3e..a37e7a69140adf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2518,7 +2518,7 @@ class Sema final : public SemaBase {
bool BuiltinNonDeterministicValue(CallExpr *TheCall);
- bool IsBuiltinCountedByRef(const Expr *E) {
+ bool IsBuiltinCountedByRef(const Expr *E) const {
const CallExpr *CE =
E ? dyn_cast<CallExpr>(E->IgnoreParenImpCasts()) : nullptr;
return CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref;
>From 90cdf04c9ee30a92397b3a78b5b7c5fc4e239e84 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Thu, 21 Nov 2024 13:34:18 -0800
Subject: [PATCH 8/8] Use helper function.
---
clang/lib/Sema/SemaDecl.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 695c8bafea72e6..5f272ef31ccde4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14692,11 +14692,10 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
// The result of __builtin_counted_by_ref cannot be assigned to a variable.
// It allows leaking and modification of bounds safety information.
- if (const auto *CE = dyn_cast_if_present<CallExpr>(VD->getInit());
- CE && CE->getBuiltinCallee() == Builtin::BI__builtin_counted_by_ref)
- Diag(CE->getExprLoc(),
+ if (IsBuiltinCountedByRef(VD->getInit()))
+ Diag(VD->getInit()->getExprLoc(),
diag::err_builtin_counted_by_ref_cannot_leak_reference)
- << CE->getSourceRange();
+ << VD->getInit()->getSourceRange();
checkAttributesAfterMerging(*this, *VD);
More information about the cfe-commits
mailing list