[-Wunsafe-buffer-usage] Relax passing to __counted_by_or_null()

Relax restrictions when passing to
__counted_by_or_null()/__sized_by_or_null() parameters:
- Allow the dependent count var to be anything.
- Allow passing from non-null variant to *_or_null(), but do warn for
  passing *_or_null() to non-null variant.

rdar://156006635

Author: patrykstefanski

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -14228,6 +14228,11 @@ def note_unsafe_count_attributed_pointer_argument : Note<
   "consider using %select{|a safe container and passing '.data()' to the "
   "parameter%select{| '%3'}2 and '.size()' to its dependent parameter '%4' or }0"
   "'std::span' and passing '.first(...).data()' to the parameter%select{| '%3'}2">;
+def note_unsafe_count_attributed_pointer_argument_null_to_nonnull : Note<
+  "passing '%select{__counted_by_or_null()|__sized_by_or_null()}0' to "
+  "'%select{__counted_by()|__sized_by()}1' while "
+  "'%select{__counted_by_or_null()|__sized_by_or_null()}0' can be null with an "
+  "arbirary %select{count|size}0">;
 def warn_unsafe_single_pointer_argument : Warning<
   "unsafe assignment to function parameter of __single pointer type">,
   InGroup<UnsafeBufferUsage>, DefaultIgnore;

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -958,7 +958,9 @@ static const DeclRefExpr *tryGetAddressofDRE(const Expr *E) {
 
 // Checks if the argument passed to count-attributed pointer is one of the
 // following forms:
-// 0. `NULL/nullptr`, if the argument to dependent count/size is `0`.
+// 0. `NULL/nullptr`, if the argument to dependent count/size is `0`, or
+//     anything if the type of the pointer is
+//     __counted_by_or_null()/__sized_by_or_null().
 // 1. `&var`, if `var` is a variable identifier and (1.a.) the dependent count
 //     is `1`; or (1.b.) the counted_by expression is constant `1`
 // 2. `&var`, if `var` is a variable identifier and the dependent size is
@@ -1021,6 +1023,8 @@ static bool isCountAttributedPointerArgumentSafeImpl(
 
   // check form 0:
   if (PtrArgNoImp->getType()->isNullPtrType()) {
+    if (isOrNull)
+      return true;
     if (CountArg)
       return hasIntegeralConstant(CountArg, 0, Context);
     // When there is no argument representing the count/size, it is safe iff
@@ -1095,7 +1099,7 @@ static bool isCountAttributedPointerArgumentSafeImpl(
     bool areBoundsAttributesCompatible =
         (ArgCAT->isCountInBytes() == isSizedBy || (ArgInBytes && ParamInBytes));
 
-    if (ArgCAT->isOrNull() == isOrNull && areBoundsAttributesCompatible)
+    if ((isOrNull || !ArgCAT->isOrNull()) && areBoundsAttributesCompatible)
       ActualCount = ArgCAT->getCountExpr();
     // In case `PtrArgNoImp` is a function call expression of counted_by type
     // (i.e., return type is a CAT), create a map for the call:

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2590,6 +2590,15 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
     SourceLocation DiagLoc =
         Arg->isDefaultArgument() ? Call->getRParenLoc() : Arg->getBeginLoc();
     S.Diag(DiagLoc, diag::warn_unsafe_count_attributed_pointer_argument);
+
+    if (const auto *ArgCATy = Arg->getType()->getAs<CountAttributedType>();
+        ArgCATy && ArgCATy->isOrNull() && !CATy->isOrNull()) {
+      S.Diag(
+          DiagLoc,
+          diag::note_unsafe_count_attributed_pointer_argument_null_to_nonnull)
+          << ArgCATy->isCountInBytes() << CATy->isCountInBytes();
+    }
+
     S.Diag(PVD->getBeginLoc(),
            diag::note_unsafe_count_attributed_pointer_argument)
         << IsSimpleCount << QualType(CATy, 0) << !PtrParamName.empty()

clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-argument.cpp

@@ -69,7 +69,7 @@ void cb_cchar(const char *__counted_by(len) s, size_t len);
 // expected-note@+1 3{{consider using 'std::span' and passing '.first(...).data()' to the parameter 's'}}
 void cb_cchar_42(const char *__counted_by(42) s);
 
-// expected-note@+1 31{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
+// expected-note@+1 +{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
 void cb_int(int *__counted_by(count) p, size_t count);
 
 // expected-note@+1 34{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
@@ -81,6 +81,9 @@ void cb_cint_42(const int *__counted_by(42) p);
 // expected-note@+1 6{{consider using 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
 void cb_cint_multi(const int *__counted_by((a + b) * (c - d)) p, int a, int b, int c, int d);
 
+// expected-note@+1 3{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
+void sb_void(void *__sized_by(size) p, size_t size);
+
 // expected-note@+1 13{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
 void sb_cvoid(const void *__sized_by(size) p, size_t size);
 
@@ -98,6 +101,12 @@ void sb_cint_42(const int *__sized_by(42) p);
 
 void cb_cint_array(const int (* __counted_by(size) p)[10], size_t size);
 
+// expected-note@+1 +{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'count' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
+void cbn_int(int *__counted_by_or_null(count) p, size_t count);
+
+// expected-note@+1 +{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
+void sbn_void(void *__sized_by_or_null(size) p, size_t size);
+
 }  // extern "C"
 
 // Check allowed classes and method.
@@ -389,9 +398,22 @@ void from_cb_int_multi(int *__counted_by((a + b) * (c - d)) p, int a, int b, int
   cb_int(p, 42);                 // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
 }
 
-void nullptr_as_arg(void * __sized_by(size) p, unsigned size) { //expected-note{{consider using a safe container and passing '.data()' to the parameter 'p' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'p'}}
-  nullptr_as_arg(nullptr, 0);
-  nullptr_as_arg(nullptr, size); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+void nullptr_as_arg(size_t n) {
+  cb_int(nullptr, 0);
+  cb_int(nullptr, 42); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+  cb_int(nullptr, n);  // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+  sb_void(nullptr, 0);
+  sb_void(nullptr, 42); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+  sb_void(nullptr, n);  // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+  cbn_int(nullptr, 0);
+  cbn_int(nullptr, 42);
+  cbn_int(nullptr, n);
+
+  sbn_void(nullptr, 0);
+  sbn_void(nullptr, 42);
+  sbn_void(nullptr, n);
 }
 
 void single_variable() {
@@ -708,6 +730,42 @@ static void previous_infinite_loop3(int * __counted_by(n + n * m) p, size_t n,
   previous_infinite_loop3(p, n, q, r, m, o + 1); // expected-warning 2{{unsafe assignment to function parameter of count-attributed type}}
 }
 
+// Check nullable variants.
+
+void nullable(std::span<int> sp, size_t n) {
+  cbn_int(sp.data(), sp.size());
+  cbn_int(sp.data(), sp.size_bytes()); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+  cbn_int(sp.first(42).data(), 42);
+  cbn_int(sp.first(n).data(), n);
+  cbn_int(sp.first(42).data(), n); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+
+  sbn_void(sp.data(), sp.size_bytes());
+  sbn_void(sp.data(), sp.size());   // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+  sbn_void(sp.first(42).data(), n); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
+}
+
+void nonnull_tofrom_nullable(size_t n,
+                             int *__counted_by(n) cb,
+                             int *__counted_by_or_null(n) cbn,
+                             void *__sized_by(n) sb,
+                             void *__sized_by_or_null(n) sbn) {
+  cb_int(cb, n);
+  // expected-note@+2{{passing '__counted_by_or_null()' to '__counted_by()' while '__counted_by_or_null()' can be null with an arbirary count}}
+  // expected-warning@+1{{unsafe assignment to function parameter of count-attributed type}}
+  cb_int(cbn, n);
+
+  cbn_int(cb, n);
+  cbn_int(cbn, n);
+
+  sb_void(sb, n);
+  // expected-note@+2{{passing '__sized_by_or_null()' to '__sized_by()' while '__sized_by_or_null()' can be null with an arbirary size}}
+  // expected-warning@+1{{unsafe assignment to function parameter of count-attributed type}}
+  sb_void(sbn, n);
+
+  sbn_void(sb, n);
+  sbn_void(sbn, n);
+}
+
 // Check default args.
 
 void cb_def_arg_null_0(int *__counted_by(count) p = nullptr, size_t count = 0);

clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp

@@ -176,3 +176,15 @@ namespace test_fam {
   }
 
 } // namespace test_fam
+
+void test_nullable(int *__counted_by_or_null(count) cbn_p, size_t count,
+                   char *__sized_by_or_null(size) sbn_p, size_t size) {
+  // __counted_by_or_null(count) pointer can be null with an arbitrary count.
+  // We rely here on the dynamic check performed by std::span constructor, and
+  // allow those patterns.
+  std::span<int>{cbn_p, count};
+  std::span<char>{sbn_p, size};
+
+  std::span<int>{cbn_p, 42};  // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+  std::span<char>{sbn_p, 42}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+}

clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop-annotated.cpp

@@ -39,9 +39,9 @@ namespace std {
 // expected-note@+1{{consider using a safe container and passing '.data()' to the parameter 'src' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'src'}}
 void *memcpy(void * __sized_by(size) dst, const void * __sized_by(size) src, size_t size);
 size_t strlen( const char* __null_terminated str );
-// expected-note@+1{{consider using a safe container and passing '.data()' to the parameter 'buffer' and '.size()' to its dependent parameter 'buf_size' or 'std::span' and passing '.first(...).data()' to the parameter 'buffer'}}
-int snprintf( char* __counted_by(buf_size) buffer, size_t buf_size, const char* format, ... );
 // expected-note@+1 2{{consider using a safe container and passing '.data()' to the parameter 'buffer' and '.size()' to its dependent parameter 'buf_size' or 'std::span' and passing '.first(...).data()' to the parameter 'buffer'}}
+int snprintf( char* __counted_by(buf_size) buffer, size_t buf_size, const char* format, ... );
+// expected-note@+1 +{{consider using a safe container and passing '.data()' to the parameter 'buffer' and '.size()' to its dependent parameter 'buf_size' or 'std::span' and passing '.first(...).data()' to the parameter 'buffer'}}
 int snwprintf( wchar_t* __counted_by(buf_size) buffer, size_t buf_size, const wchar_t* format, ... );
 int vsnprintf( char* __counted_by(buf_size) buffer, size_t buf_size, const char* format, va_list va_args);
 
@@ -55,7 +55,8 @@ void test(char * p, char * q, const char * str,
 	  const char * __null_terminated safe_str,
 	  char * __counted_by(n) safe_p,
 	  size_t n,
-	  char * __counted_by(10) safe_ten) {
+	  char * __counted_by(10) safe_ten,
+	  char * __counted_by_or_null(n) null_p) {
   memcpy(p, q, 10);                  // expected-warning2{{unsafe assignment to function parameter of count-attributed type}}
   snprintf(p, 10, "%s", "hlo");      // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
 
@@ -73,13 +74,18 @@ void test(char * p, char * q, const char * str,
   vsnprintf(safe_p, n, "%s", vlist); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}}
   sprintf(safe_ten, "%s", safe_str);    // expected-warning{{function 'sprintf' is unsafe}} expected-note{{change to 'snprintf' for explicit bounds checking}}
 
+  // expected-warning@+2{{unsafe assignment to function parameter of count-attributed type}}
+  // expected-note@+1{{passing '__counted_by_or_null()' to '__counted_by()' while '__counted_by_or_null()' can be null with an arbirary count}}
+  snprintf(null_p, n, "%s", "hlo");
 }
 
 void test_wchar(wchar_t * p, wchar_t * q, const wchar_t * wstr,
 	  const wchar_t * __null_terminated safe_wstr,
 	  wchar_t * __null_terminated nt_wstr,
 	  wchar_t * __counted_by(n) safe_p,
 	  wchar_t * __sized_by(n) sizedby_p,
+	  wchar_t * __counted_by_or_null(n) cbn,
+	  wchar_t * __sized_by_or_null(n) sbn,
 	  size_t n) {
   std::wstring cxx_wstr;
   std::span<wchar_t> cxx_wspan;
@@ -90,5 +96,11 @@ void test_wchar(wchar_t * p, wchar_t * q, const wchar_t * wstr,
   snwprintf(p, n, L"%ls", safe_wstr); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
   snwprintf(sizedby_p, n, L"%ls", safe_wstr); // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
   snwprintf(safe_p, n, L"%ls", wstr); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
+  // expected-warning@+2{{unsafe assignment to function parameter of count-attributed type}}
+  // expected-note@+1{{passing '__sized_by_or_null()' to '__counted_by()' while '__sized_by_or_null()' can be null with an arbirary size}}
+  snwprintf(sbn, n, L"%ls", safe_wstr);
+  // expected-warning@+2{{unsafe assignment to function parameter of count-attributed type}}
+  // expected-note@+1{{passing '__counted_by_or_null()' to '__counted_by()' while '__counted_by_or_null()' can be null with an arbirary count}}
+  snwprintf(cbn, n, L"%ls", wstr);
 }