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 <mo...@google.com> Date: Fri, 15 Nov 2024 15:41:48 -0800 Subject: [PATCH 1/9] [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 <mo...@google.com> Date: Tue, 19 Nov 2024 17:32:09 -0800 Subject: [PATCH 2/9] 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 <mo...@google.com> Date: Tue, 19 Nov 2024 17:39:19 -0800 Subject: [PATCH 3/9] 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 <mo...@google.com> Date: Wed, 20 Nov 2024 13:24:59 -0800 Subject: [PATCH 4/9] 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 <mo...@google.com> Date: Wed, 20 Nov 2024 13:40:04 -0800 Subject: [PATCH 5/9] 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 <mo...@google.com> Date: Wed, 20 Nov 2024 13:40:30 -0800 Subject: [PATCH 6/9] 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 <mo...@google.com> Date: Wed, 20 Nov 2024 13:42:10 -0800 Subject: [PATCH 7/9] 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 <mo...@google.com> Date: Thu, 21 Nov 2024 13:34:18 -0800 Subject: [PATCH 8/9] 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); >From f34c1871a011b4aaa9c8023a745a5abe2312689b Mon Sep 17 00:00:00 2001 From: Bill Wendling <mo...@google.com> Date: Wed, 27 Nov 2024 09:41:46 -0800 Subject: [PATCH 9/9] Create a separate checking function. --- .../clang/Basic/DiagnosticSemaKinds.td | 9 +++-- clang/include/clang/Sema/Sema.h | 16 +++++--- clang/lib/Sema/SemaChecking.cpp | 39 +++++++++++++++++++ clang/lib/Sema/SemaDecl.cpp | 7 +--- clang/lib/Sema/SemaExpr.cpp | 31 +++------------ clang/lib/Sema/SemaStmt.cpp | 5 +-- clang/test/Sema/builtin-counted-by-ref.c | 14 +++---- 7 files changed, 70 insertions(+), 51 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 37fb44d4bf74cd..268add884bab9c 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">; + "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 a37e7a69140adf..b67cfd95c27703 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2518,11 +2518,17 @@ class Sema final : public SemaBase { bool BuiltinNonDeterministicValue(CallExpr *TheCall); - 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; - } + 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 2d4a7cd287b70d..d46aa45ec79367 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -5621,6 +5621,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 5f272ef31ccde4..a573802af16906 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -14690,12 +14690,7 @@ 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 (IsBuiltinCountedByRef(VD->getInit())) - Diag(VD->getInit()->getExprLoc(), - diag::err_builtin_counted_by_ref_cannot_leak_reference) - << VD->getInit()->getSourceRange(); + CheckInvalidBuiltinCountedByRef(VD->getInit(), InitializerKind); checkAttributesAfterMerging(*this, *VD); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 26b951f54b3ced..a80c5911ba9819 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -4894,11 +4894,7 @@ 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. - if (IsBuiltinCountedByRef(base)) - Diag(base->getExprLoc(), diag::err_builtin_counted_by_ref_invalid_use) - << 0 << base->getSourceRange(); + CheckInvalidBuiltinCountedByRef(base, ArraySubscriptKind); // Handle any non-overload placeholder types in the base and index // expressions. We can't handle overloads here because the other @@ -6495,12 +6491,8 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc, // 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(); + if (CheckInvalidBuiltinCountedByRef(Arg, FunctionArgKind)) return ExprError(); - } if (getLangOpts().CPlusPlus) { // If this is a pseudo-destructor expression, build the call immediately. @@ -15222,22 +15214,11 @@ 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 CheckBuiltinCountedByRef = [&](const Expr *E) { - if (IsBuiltinCountedByRef(E)) { - 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(); - } - }; + BuiltinCountedByRefKind K = + BinaryOperator::isAssignmentOp(Opc) ? AssignmentKind : BinaryExprKind; - CheckBuiltinCountedByRef(LHSExpr); - CheckBuiltinCountedByRef(RHSExpr); + 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 06ca5231f61dba..c7502fc52f54f7 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -3765,10 +3765,7 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp, << FSI->getFirstCoroutineStmtKeyword(); } - if (IsBuiltinCountedByRef(RetVal.get())) - Diag(RetVal.get()->getExprLoc(), - diag::err_builtin_counted_by_ref_cannot_leak_reference) - << RetVal.get()->getSourceRange(); + CheckInvalidBuiltinCountedByRef(RetVal.get(), ReturnArgKind); StmtResult R = BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true); diff --git a/clang/test/Sema/builtin-counted-by-ref.c b/clang/test/Sema/builtin-counted-by-ref.c index c21c7093468dde..a9f46a32230065 100644 --- a/clang/test/Sema/builtin-counted-by-ref.c +++ b/clang/test/Sema/builtin-counted-by-ref.c @@ -48,21 +48,21 @@ void test4(struct fam_struct *ptr, int idx) { } 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}} - 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}} + 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 (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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits