https://github.com/sam-mccall created https://github.com/llvm/llvm-project/pull/82705
This enables external nullability checkers to make use of these annotations on smart pointer types. Existing static warnings for raw pointers are extended to smart pointers: - nullptr used as return value or argument for non-null functions (`-Wnonnull`) - assigning or initializing nonnull variables with nullable values (`-Wnullable-to-nonnull-conversion`) It doesn't implicitly add these attributes based on the assume_nonnull pragma, nor warn on missing attributes where the pragma would apply them. I'm not confident that the pragma's current behavior will work well for C++ (where type-based metaprogramming is much more common than C/ObjC). We'd like to revisit this once we have more implementation experience. UBSan's `-fsanitize=nullability` will not check smart-pointer types. It can be made to do so by synthesizing calls to `operator bool`, but that's left for future work. >From 1a56ae6f27778750b8c7761e2630476833a061b8 Mon Sep 17 00:00:00 2001 From: Sam McCall <sam.mcc...@gmail.com> Date: Thu, 22 Feb 2024 16:00:44 +0100 Subject: [PATCH] [clang][nullability] allow _Nonnull etc on gsl::Pointer types This enables external nullability checkers to make use of these annotations on smart pointer types. Existing static warnings for raw pointers are extended to smart pointers: - nullptr used as return value or argument for non-null functions (`-Wnonnull`) - assigning or initializing nonnull variables with nullable values (`-Wnullable-to-nonnull-conversion`) It doesn't implicitly add these attributes based on the assume_nonnull pragma, nor warn on missing attributes where the pragma would apply them. I'm not confident that the pragma's current behavior will work well for C++ (where type-based metaprogramming is much more common than C/ObjC). We'd like to revisit this once we have more implementation experience. UBSan's `-fsanitize=nullability` will not check smart-pointer types. It can be made to do so by synthesizing calls to `operator bool`, but that's left for future work. --- clang/include/clang/Basic/AttrDocs.td | 4 +++ clang/lib/AST/Type.cpp | 44 ++++++++++++++++++------ clang/lib/CodeGen/CGCall.cpp | 3 +- clang/lib/CodeGen/CodeGenFunction.cpp | 3 +- clang/lib/Sema/SemaChecking.cpp | 9 +++++ clang/lib/Sema/SemaInit.cpp | 5 +++ clang/lib/Sema/SemaOverload.cpp | 7 ++++ clang/lib/Sema/SemaType.cpp | 18 +++++++--- clang/test/SemaCXX/nullability.cpp | 48 +++++++++++++++++++++++++-- 9 files changed, 123 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index b96fbddd51154c..28f201830fe56f 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -4096,6 +4096,10 @@ non-underscored keywords. For example: @property (assign, nullable) NSView *superview; @property (readonly, nonnull) NSArray *subviews; @end + +As well as built-in pointer types, ithe nullability attributes can be attached +to the standard C++ pointer types ``std::unique_ptr`` and ``std::shared_ptr``, +as well as C++ classes marked with the ``gsl::Pointer`` attribute. }]; } diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index 78dcd3f4007a5a..f65529e5c917f3 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -44,6 +44,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringSwitch.h" #include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/MathExtras.h" @@ -4524,6 +4525,26 @@ std::optional<NullabilityKind> Type::getNullability() const { return std::nullopt; } +// Smart pointers are well-known stdlib types or marked with [[gsl::Pointer]]. +static bool classCanHaveNullability(CXXRecordDecl *CRD) { + if (!CRD) + return false; + if (CRD->hasAttr<PointerAttr>()) + return true; + if (auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(CRD)) + if (CTSD->getSpecializedTemplate() + ->getTemplatedDecl() + ->hasAttr<PointerAttr>()) + return true; + if (CRD->isInStdNamespace()) + if (auto *II = CRD->getIdentifier()) + return llvm::StringSwitch<bool>(II->getName()) + .Cases("auto_ptr", "inout_ptr_t", "out_ptr_t", "shared_ptr", + "unique_ptr", true) + .Default(false); + return false; +} + bool Type::canHaveNullability(bool ResultIfUnknown) const { QualType type = getCanonicalTypeInternal(); @@ -4556,16 +4577,16 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const { case Type::Auto: return ResultIfUnknown; - // Dependent template specializations can instantiate to pointer - // types unless they're known to be specializations of a class - // template. + // Dependent template specializations could instantiate to pointer types. case Type::TemplateSpecialization: - if (TemplateDecl *templateDecl - = cast<TemplateSpecializationType>(type.getTypePtr()) - ->getTemplateName().getAsTemplateDecl()) { - if (isa<ClassTemplateDecl>(templateDecl)) - return false; - } + // If we know this is a class template, check the primary template. + // (Hopefully we don't have only some specializations defined as pointers!). + if (TemplateDecl *templateDecl = + cast<TemplateSpecializationType>(type.getTypePtr()) + ->getTemplateName() + .getAsTemplateDecl()) + if (auto *CTD = dyn_cast<ClassTemplateDecl>(templateDecl)) + return classCanHaveNullability(CTD->getTemplatedDecl()); return ResultIfUnknown; case Type::Builtin: @@ -4622,6 +4643,10 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const { } llvm_unreachable("unknown builtin type"); + case Type::Record: + return classCanHaveNullability( + cast<RecordType>(type)->getAsCXXRecordDecl()); + // Non-pointer types. case Type::Complex: case Type::LValueReference: @@ -4639,7 +4664,6 @@ bool Type::canHaveNullability(bool ResultIfUnknown) const { case Type::DependentAddressSpace: case Type::FunctionProto: case Type::FunctionNoProto: - case Type::Record: case Type::DeducedTemplateSpecialization: case Type::Enum: case Type::InjectedClassName: diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index d05cf1c6e1814e..83cfc8831b036c 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -4373,7 +4373,8 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType, NNAttr = getNonNullAttr(AC.getDecl(), PVD, ArgType, ArgNo); bool CanCheckNullability = false; - if (SanOpts.has(SanitizerKind::NullabilityArg) && !NNAttr && PVD) { + if (SanOpts.has(SanitizerKind::NullabilityArg) && !NNAttr && PVD && + !PVD->getType()->isRecordType()) { auto Nullability = PVD->getType()->getNullability(); CanCheckNullability = Nullability && *Nullability == NullabilityKind::NonNull && diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 1ad905078d349c..c492929f061798 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -974,7 +974,8 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy, // return value. Initialize the flag to 'true' and refine it in EmitParmDecl. if (SanOpts.has(SanitizerKind::NullabilityReturn)) { auto Nullability = FnRetTy->getNullability(); - if (Nullability && *Nullability == NullabilityKind::NonNull) { + if (Nullability && *Nullability == NullabilityKind::NonNull && + !FnRetTy->isRecordType()) { if (!(SanOpts.has(SanitizerKind::ReturnsNonnullAttribute) && CurCodeDecl && CurCodeDecl->getAttr<ReturnsNonNullAttr>())) RetValNullabilityPrecondition = diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index e8bfb215a5b4c5..1778f632c12aad 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -27,6 +27,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/AST/ExprOpenMP.h" #include "clang/AST/FormatString.h" +#include "clang/AST/IgnoreExpr.h" #include "clang/AST/NSAPI.h" #include "clang/AST/NonTrivialTypeVisitor.h" #include "clang/AST/OperationKinds.h" @@ -7154,6 +7155,14 @@ bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember, /// /// Returns true if the value evaluates to null. static bool CheckNonNullExpr(Sema &S, const Expr *Expr) { + // Treat (smart) pointers constructed from nullptr as null, whether we can + // const-evaluate them or not. + // This must happen first: the smart pointer expr might have _Nonnull type! + if (isa<CXXNullPtrLiteralExpr>( + IgnoreExprNodes(Expr, IgnoreImplicitAsWrittenSingleStep, + IgnoreElidableImplicitConstructorSingleStep))) + return true; + // If the expression has non-null type, it doesn't evaluate to null. if (auto nullability = Expr->IgnoreImplicit()->getType()->getNullability()) { if (*nullability == NullabilityKind::NonNull) diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 0fd458837163e5..eea63c2e577d42 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -7050,6 +7050,11 @@ PerformConstructorInitialization(Sema &S, hasCopyOrMoveCtorParam(S.Context, getConstructorInfo(Step.Function.FoundDecl)); + // A smart pointer constructed from a null pointer is almost certainly null. + if (NumArgs == 1 && !Kind.isExplicitCast()) + S.diagnoseNullableToNonnullConversion(Entity.getType(), Args.front()->getType(), + Kind.getLocation()); + // Determine the arguments required to actually perform the constructor // call. if (S.CompleteConstructorCall(Constructor, Step.Type, Args, Loc, diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index f7645422348b65..03ac17c5be4b04 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -14706,6 +14706,13 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc, } } + // Check for nonnull = nullable. + // This won't be caught in the arg's initialization: the parameter to + // the assignment operator is not marked nonnull. + if (Op == OO_Equal) + diagnoseNullableToNonnullConversion(Args[0]->getType(), + Args[1]->getType(), OpLoc); + // Convert the arguments. if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FnDecl)) { // Best->Access is only meaningful for class members. diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 1e43e36016a66f..517152397a0a68 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -4705,6 +4705,18 @@ static bool DiagnoseMultipleAddrSpaceAttributes(Sema &S, LangAS ASOld, return false; } +// Whether this is a type broadly expected to have nullability attached. +// These types are affected by `#pragma assume_nonnull`, and missing nullability +// will be diagnosed with -Wnullability-completeness. +static bool shouldHaveNullability(QualType T) { + return T->canHaveNullability(/*ResultIfUnknown=*/false) && + // For now, do not infer/require nullability on C++ smart pointers. + // It's unclear whether the pragma's behavior is useful for C++. + // e.g. treating type-aliases and template-type-parameters differently + // from types of declarations can be surprising. + !isa<RecordType>(T); +} + static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, QualType declSpecType, TypeSourceInfo *TInfo) { @@ -4823,8 +4835,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, // inner pointers. complainAboutMissingNullability = CAMN_InnerPointers; - if (T->canHaveNullability(/*ResultIfUnknown*/ false) && - !T->getNullability()) { + if (shouldHaveNullability(T) && !T->getNullability()) { // Note that we allow but don't require nullability on dependent types. ++NumPointersRemaining; } @@ -5047,8 +5058,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, // If the type itself could have nullability but does not, infer pointer // nullability and perform consistency checking. if (S.CodeSynthesisContexts.empty()) { - if (T->canHaveNullability(/*ResultIfUnknown*/ false) && - !T->getNullability()) { + if (shouldHaveNullability(T) && !T->getNullability()) { if (isVaList(T)) { // Record that we've seen a pointer, but do nothing else. if (NumPointersRemaining > 0) diff --git a/clang/test/SemaCXX/nullability.cpp b/clang/test/SemaCXX/nullability.cpp index 8d0c4dc195a6bd..4984b0eefabcdf 100644 --- a/clang/test/SemaCXX/nullability.cpp +++ b/clang/test/SemaCXX/nullability.cpp @@ -27,6 +27,7 @@ template<typename T> struct AddNonNull { typedef _Nonnull T type; // expected-error{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'int'}} // expected-error@-1{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'std::nullptr_t'}} + // expected-error@-2{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'NotPtr'}} }; typedef AddNonNull<int *>::type nonnull_int_ptr_1; @@ -35,6 +36,23 @@ typedef AddNonNull<nullptr_t>::type nonnull_int_ptr_3; // expected-note{{in inst typedef AddNonNull<int>::type nonnull_non_pointer_1; // expected-note{{in instantiation of template class 'AddNonNull<int>' requested here}} +// Nullability on C++ class types (smart pointers). +struct NotPtr{}; +typedef AddNonNull<NotPtr>::type nonnull_non_pointer_2; // expected-note{{in instantiation}} +struct [[gsl::Pointer]] SmartPtr{ + SmartPtr(); + SmartPtr(nullptr_t); + SmartPtr(const SmartPtr&); + SmartPtr(SmartPtr&&); + SmartPtr &operator=(const SmartPtr&); + SmartPtr &operator=(SmartPtr&&); +}; +typedef AddNonNull<SmartPtr>::type nonnull_smart_pointer_1; +template<class> struct [[gsl::Pointer]] SmartPtrTemplate{}; +typedef AddNonNull<SmartPtrTemplate<int>>::type nonnull_smart_pointer_2; +namespace std { inline namespace __1 { template <class> class unique_ptr {}; } } +typedef AddNonNull<std::unique_ptr<int>>::type nonnull_smart_pointer_3; + // Non-null checking within a template. template<typename T> struct AddNonNull2 { @@ -54,6 +72,7 @@ void (*& accepts_nonnull_2)(_Nonnull int *ptr) = accepts_nonnull_1; void (X::* accepts_nonnull_3)(_Nonnull int *ptr); void accepts_nonnull_4(_Nonnull int *ptr); void (&accepts_nonnull_5)(_Nonnull int *ptr) = accepts_nonnull_4; +void accepts_nonnull_6(SmartPtr _Nonnull); void test_accepts_nonnull_null_pointer_literal(X *x) { accepts_nonnull_1(0); // expected-warning{{null passed to a callee that requires a non-null argument}} @@ -61,6 +80,8 @@ void test_accepts_nonnull_null_pointer_literal(X *x) { (x->*accepts_nonnull_3)(0); // expected-warning{{null passed to a callee that requires a non-null argument}} accepts_nonnull_4(0); // expected-warning{{null passed to a callee that requires a non-null argument}} accepts_nonnull_5(0); // expected-warning{{null passed to a callee that requires a non-null argument}} + + accepts_nonnull_6(nullptr); // expected-warning{{null passed to a callee that requires a non-null argument}} } template<void FP(_Nonnull int*)> @@ -71,6 +92,7 @@ void test_accepts_nonnull_null_pointer_literal_template() { template void test_accepts_nonnull_null_pointer_literal_template<&accepts_nonnull_4>(); // expected-note{{instantiation of function template specialization}} void TakeNonnull(void *_Nonnull); +void TakeSmartNonnull(SmartPtr _Nonnull); // Check different forms of assignment to a nonull type from a nullable one. void AssignAndInitNonNull() { void *_Nullable nullable; @@ -81,12 +103,26 @@ void AssignAndInitNonNull() { void *_Nonnull nonnull; nonnull = nullable; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}} nonnull = {nullable}; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}} - TakeNonnull(nullable); //expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull}} TakeNonnull(nonnull); // OK + nonnull = (void *_Nonnull)nullable; // explicit cast OK + + SmartPtr _Nullable s_nullable; + SmartPtr _Nonnull s(s_nullable); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s2{s_nullable}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s3 = {s_nullable}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s4 = s_nullable; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s_nonnull; + s_nonnull = s_nullable; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + s_nonnull = {s_nullable}; // no warning here - might be nice? + TakeSmartNonnull(s_nullable); //expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull}} + TakeSmartNonnull(s_nonnull); // OK + s_nonnull = (SmartPtr _Nonnull)s_nullable; // explicit cast OK + s_nonnull = static_cast<SmartPtr _Nonnull>(s_nullable); // explicit cast OK } void *_Nullable ReturnNullable(); +SmartPtr _Nullable ReturnSmartNullable(); void AssignAndInitNonNullFromFn() { void *_Nonnull p(ReturnNullable()); // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}} @@ -96,8 +132,16 @@ void AssignAndInitNonNullFromFn() { void *_Nonnull nonnull; nonnull = ReturnNullable(); // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}} nonnull = {ReturnNullable()}; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}} - TakeNonnull(ReturnNullable()); //expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull}} + + SmartPtr _Nonnull s(ReturnSmartNullable()); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s2{ReturnSmartNullable()}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s3 = {ReturnSmartNullable()}; // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s4 = ReturnSmartNullable(); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + SmartPtr _Nonnull s_nonnull; + s_nonnull = ReturnSmartNullable(); // expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull'}} + s_nonnull = {ReturnSmartNullable()}; + TakeSmartNonnull(ReturnSmartNullable()); //expected-warning{{implicit conversion from nullable pointer 'SmartPtr _Nullable' to non-nullable pointer type 'SmartPtr _Nonnull}} } void ConditionalExpr(bool c) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits