[clang] [clang][nullability] allow _Nonnull etc on gsl::Pointer types (PR #82705)

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 22 15:39:22 PST 2024


https://github.com/sam-mccall updated https://github.com/llvm/llvm-project/pull/82705

>From 090c0b01d1b02fd68c15971a1f3d9548aba24670 Mon Sep 17 00:00:00 2001
From: Sam McCall <sam.mccall at 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                | 43 ++++++++++++++++++------
 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, 122 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..b8a8ccfe9b0673 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,15 @@ 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 it's a known class template, we can already check if it's a pointer.
+    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 +4642,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 +4663,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 at -1{{nullability specifier '_Nonnull' cannot be applied to non-pointer type 'std::nullptr_t'}}
+  // expected-error at -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) {



More information about the cfe-commits mailing list