From 8dfe4ceb907f070a3d2f0a842129aea45cb105ac Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo@apple.com>
Date: Thu, 20 Feb 2025 11:27:23 -0800
Subject: [PATCH 1/7] [-Wunsafe-buffer-usage] Accept calls to some libc
 functions with annotated arguments

- `printf`, `fprintf` `snprintf` functions accept `__null_terminated`
- `snprintf` function accepts `__counted_by/__sized_by`
- functions consuming a single string pointer like `strlen` or `atoi`
  accept `__null_terminated`

Generalized `isCountAttributedPointerArgumentSafe` so that it is
shared by interoperation gadgets and the unsafe libc gadget.

(A follow-up change to rdar://138798346)
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 213 +++++++++++-------
 ...-usage-libc-functions-inline-namespace.cpp |   6 +-
 ...fe-buffer-usage-libc-functions-interop.cpp |  81 +++++++
 ...arn-unsafe-buffer-usage-libc-functions.cpp |  10 +-
 4 files changed, 216 insertions(+), 94 deletions(-)
 create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index e65c98e4765f6..a233859f34ceb 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -747,7 +747,7 @@ const Expr *extractExtentFromSubviewDataCall(ASTContext &Context,
 static bool hasIntegeralConstant(const Expr *E, uint64_t Val, ASTContext &Ctx) {
   Expr::EvalResult ER;
 
-  if (E->EvaluateAsConstantExpr(ER, Ctx)) {
+  if (E->EvaluateAsInt(ER, Ctx)) {
     APInt Eval = ER.Val.getInt();
 
     return APInt::isSameValue(Eval, APInt(Eval.getBitWidth(), Val));
@@ -769,17 +769,60 @@ static bool hasIntegeralConstant(const Expr *E, uint64_t Val, ASTContext &Ctx) {
 //    `constant-array`.
 // 7. `(T*) constant-array` if the size of `T` equals to one byte and the
 //    argument to dependent size is equivalent to `sizeof(constant-array)`.
-bool isCountAttributedPointerArgumentSafe(ASTContext &Context,
-                                          const CountAttributedType *CAT,
-                                          const CallExpr *Call,
-                                          const Expr *Arg) {
-  const Expr *ArgNoImp = Arg->IgnoreParenImpCasts();
+//
+// This function is generalized for two different cases:
+//  (a). `__counted_by(e)` attributes are annotated for the function being
+//       called;
+//  (b). The function being called has no annotations but this analysis
+//       hardcodes `__counted_by(n)/__sized_by(n)` relation for a pair of
+//       parameters--pointer `p` and count/size `n`.
+//       (E.g., the first two parameters of snprintf).
+//
+// \param PtrArg   the argument to the parameter representing a counted_by
+//                 pointer.
+// \param CountArg the argument to the parameter representing count/size. Or
+//                 NULL if the count/size cannot be represented by one argument.
+//                 `CountArg` is never NULL for case (b).
+// \param CountedByExpr
+//                 the counted_by expression in the annotated bounds attribute
+//                 for case (a); Or NULL for case (b).  This parameter indicates
+//                 whether it is case (a) or (b).
+// \param CountedByPointerTy
+//                 the pointer type of a counted_by parameter. Note
+//                 this type is NOT necessarily a `CountAttributedType` in case
+//                 (b) because the counted_by relation comes from hardcoded
+//                 knowledge.
+// \param isSizedBy
+//                 true iff the counted_by relation is bytewise.
+// \param isOrNull
+//                 true iff the counted_by attribute is __counted_by_or_null
+// \param DependentValueMap
+//                 A map from variables in the counted_by expression to actual
+//                 arguments in case (a); or NULL for case (b) or failure in
+//                 obtatining the info.
+static bool isCountAttributedPointerArgumentSafeImpl(
+    ASTContext &Context, const Expr *PtrArg,
+    const Expr * /* Nullable */ CountArg, const Type *CountedByPointerTy,
+    const Expr * /* Nullable */ CountedByExpr, bool isSizedBy, bool isOrNull,
+    const DependentValuesTy * /* Nullable */ DependentValueMap = nullptr) {
+  assert(CountedByExpr ||
+         CountArg &&
+             "Only one of them can be null. If the __counted_by information is "
+             "hardcoded, there must be an argument representing the actual "
+             "count.");
+  assert(CountedByExpr ||
+         !DependentValueMap &&
+             "If the __counted_by information is hardcoded, there is no "
+             "dependent value map.");
+  const Expr *PtrArgNoImp = PtrArg->IgnoreParenImpCasts();
 
   // check form 0:
-  if (ArgNoImp->getType()->isNullPtrType()) {
-    if (const auto *CountArg = findCountArg(CAT->getCountExpr(), Call))
+  if (PtrArgNoImp->getType()->isNullPtrType()) {
+    if (CountArg)
       return hasIntegeralConstant(CountArg, 0, Context);
-    return false;
+    // When there is no argument representing the count/size, it is safe iff
+    // the annotation is `__counted_by(0)`.
+    return hasIntegeralConstant(CountedByExpr, 0, Context);
   }
 
   // check form 1-2:
@@ -788,14 +831,17 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context,
       hasUnaryOperand(ignoringParenImpCasts(declRefExpr().bind("VarIdent")))));
 
   if (auto *DRE = selectFirst<const DeclRefExpr>(
-          "VarIdent", match(AddressofDRE, *ArgNoImp, Context))) {
-    if (const auto *CountArg = findCountArg(CAT->getCountExpr(), Call)) {
-      if (!CAT->isCountInBytes()) // form 1:
+          "VarIdent", match(AddressofDRE, *PtrArgNoImp, Context))) {
+    if (CountArg) {
+      if (!isSizedBy) // form 1:
         return hasIntegeralConstant(CountArg, 1, Context);
       // form 2:
       if (auto TySize = Context.getTypeSizeInCharsIfKnown(DRE->getType()))
         return hasIntegeralConstant(CountArg, TySize->getQuantity(), Context);
-    }
+    } else
+      // When there is no argument representing the count/size, it is safe iff
+      // the annotation is `__counted_by(1)`.
+      return !isSizedBy && hasIntegeralConstant(CountedByExpr, 1, Context);
     return false;
   }
 
@@ -806,45 +852,49 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context,
   };
 
   std::optional<CharUnits> ArgTypeSize = getTypeSize(
-      QualType(ArgNoImp->getType()->getPointeeOrArrayElementType(), 0));
-  std::optional<CharUnits> ParamTypeSize = getTypeSize(CAT->getPointeeType());
+      QualType(PtrArgNoImp->getType()->getPointeeOrArrayElementType(), 0));
+  std::optional<CharUnits> ParamTypeSize =
+      getTypeSize(CountedByPointerTy->getPointeeType());
 
   bool ArgInBytes = ArgTypeSize.has_value() && ArgTypeSize->isOne();
-  bool ParamInBytes = CAT->isCountInBytes() ||
-                      (ParamTypeSize.has_value() && ParamTypeSize->isOne());
+  bool ParamInBytes =
+      isSizedBy || (ParamTypeSize.has_value() && ParamTypeSize->isOne());
 
   // If there is only one dependent count, check for the form 3.
-  if (const auto *CountArg = findCountArg(CAT->getCountExpr(), Call)) {
-    if (isValidContainerRange(Context, Arg, CountArg, ArgInBytes, ParamInBytes))
-      return true;
-  }
+  if (CountArg && isValidContainerRange(Context, PtrArg, CountArg, ArgInBytes,
+                                        ParamInBytes))
+    return true;
 
   // Check forms 4-7.
 
   if (ArgInBytes != ParamInBytes)
     return false;
 
-  auto ValuesOpt = getDependentValuesFromCall(CAT, Call);
-  if (!ValuesOpt.has_value())
+  if (!DependentValueMap && CountedByExpr)
+    // Bail if the map is not available for case (a). Becuase
+    // we need the map to substitute parameters to arguments in case (a) but
+    // not case(b).
     return false;
 
-  const Expr *ArgCount = nullptr;
+  // the acutal count of the pointer inferred through patterns below:
+  const Expr *ActualCount = nullptr;
   const Expr *MemberBase = nullptr;
 
-  if (const auto *ME = dyn_cast<MemberExpr>(ArgNoImp))
+  if (const auto *ME = dyn_cast<MemberExpr>(PtrArgNoImp))
     MemberBase = ME->getBase();
 
-  if (const Expr *ExtentExpr = extractExtentFromSubviewDataCall(Context, Arg)) {
+  if (const Expr *ExtentExpr =
+          extractExtentFromSubviewDataCall(Context, PtrArg)) {
     // Form 4.
-    ArgCount = ExtentExpr;
+    ActualCount = ExtentExpr;
   } else if (const auto *ArgCAT =
-                 ArgNoImp->getType()->getAs<CountAttributedType>()) {
+                 PtrArgNoImp->getType()->getAs<CountAttributedType>()) {
     // Form 5.
-    if (ArgCAT->isOrNull() == CAT->isOrNull())
-      ArgCount = ArgCAT->getCountExpr();
+    if (ArgCAT->isOrNull() == isOrNull)
+      ActualCount = ArgCAT->getCountExpr();
   } else {
     // Form 6-7.
-    const Expr *ArrArg = ArgNoImp;
+    const Expr *ArrArg = PtrArgNoImp;
 
     if (ArgInBytes)
       if (auto *CE = dyn_cast<CastExpr>(ArrArg))
@@ -854,18 +904,47 @@ bool isCountAttributedPointerArgumentSafe(ASTContext &Context,
     if (const auto *ATy = Context.getAsConstantArrayType(ArrArg->getType())) {
       APInt TySize = ATy->getSize();
 
-      if (CAT->isCountInBytes())
+      if (isSizedBy)
         if (auto TySizeInChar = Context.getTypeSizeInCharsIfKnown(ATy))
           TySize = APInt(TySize.getBitWidth(), TySizeInChar->getQuantity());
-      ArgCount = IntegerLiteral::Create(Context, TySize, Context.getSizeType(),
-                                        SourceLocation());
+      ActualCount = IntegerLiteral::Create(
+          Context, TySize, Context.getSizeType(), SourceLocation());
     }
   }
-  if (!ArgCount)
+  if (!ActualCount)
     return false;
+  if (!CountedByExpr)
+    // In case (b), the actual count of the pointer must match the argument
+    // passed to the parameter representing the count:
+    CountedByExpr = CountArg;
+  return isCompatibleWithCountExpr(ActualCount, CountedByExpr, MemberBase,
+                                   DependentValueMap, Context);
+}
+
+// Checks if the argument passed to a count-attributed pointer is safe.  This
+// function is for the case where the pointer has bounds attributes. This is
+// case (a) in `isCountAttributedPointerArgumentSafeImpl`.
+static bool isCountAttributedPointerArgumentSafe(ASTContext &Context,
+                                                 const CountAttributedType *CAT,
+                                                 const CallExpr *Call,
+                                                 const Expr *Arg) {
+  const Expr * /* Nullable */ CountArg =
+      findCountArg(CAT->getCountExpr(), Call);
+  auto ValuesOpt = getDependentValuesFromCall(CAT, Call);
+
+  return isCountAttributedPointerArgumentSafeImpl(
+      Context, Arg, CountArg, CAT, CAT->getCountExpr(), CAT->isCountInBytes(),
+      CAT->isOrNull(), ValuesOpt ? &*ValuesOpt : nullptr);
+}
 
-  return isCompatibleWithCountExpr(ArgCount, CAT->getCountExpr(), MemberBase,
-                                   &*ValuesOpt, Context);
+// Checks if arguments passed to a function, which has no annotation but
+// hardcoded counted_by knowledge, is safe.  This is case (b) in
+// `isCountAttributedPointerArgumentSafeImpl`.
+static bool isHardcodedCountedByPointerArgumentSafe(
+    ASTContext &Context, const Expr *PtrArg, const Expr *CountArg,
+    const Type *PtrParmType, bool isSizedBy, bool isOrNull) {
+  return isCountAttributedPointerArgumentSafeImpl(
+      Context, PtrArg, CountArg, PtrParmType, nullptr, isSizedBy, isOrNull);
 }
 
 // Checks if the argument passed to __single pointer is one of the following
@@ -1189,6 +1268,10 @@ static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) {
   return false;
 }
 
+AST_MATCHER(Expr, isNullTermPointer) {
+  return isNullTermPointer(&Node, Finder->getASTContext());
+}
+
 // Return true iff at least one of following cases holds:
 //  1. Format string is a literal and there is an unsafe pointer argument
 //     corresponding to an `s` specifier;
@@ -1546,55 +1629,11 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
       !Size->getType()->isIntegerType())
     return false; // not an snprintf call
 
-  // Pattern 1:
-  static StringRef SizedObjs[] = {"span", "array", "vector",
-                                  "basic_string_view", "basic_string"};
   Buf = Buf->IgnoreParenImpCasts();
   Size = Size->IgnoreParenImpCasts();
-  if (auto *MCEPtr = dyn_cast<CXXMemberCallExpr>(Buf))
-    if (auto *MCESize = dyn_cast<CXXMemberCallExpr>(Size)) {
-      auto *DREOfPtr = dyn_cast<DeclRefExpr>(
-          MCEPtr->getImplicitObjectArgument()->IgnoreParenImpCasts());
-      auto *DREOfSize = dyn_cast<DeclRefExpr>(
-          MCESize->getImplicitObjectArgument()->IgnoreParenImpCasts());
-
-      if (!DREOfPtr || !DREOfSize)
-        return true; // not in safe pattern
-      if (DREOfPtr->getDecl() != DREOfSize->getDecl())
-        return true; // not in safe pattern
-      if (MCEPtr->getMethodDecl()->getName() != "data")
-        return true; // not in safe pattern
-
-      if (MCESize->getMethodDecl()->getName() == "size_bytes" ||
-          // Note here the pointer must be a pointer-to-char type unless there
-          // is explicit casting.  If there is explicit casting, this branch
-          // is unreachable. Thus, at this branch "size" and "size_bytes" are
-          // equivalent as the pointer is a char pointer:
-          MCESize->getMethodDecl()->getName() == "size")
-        for (StringRef SizedObj : SizedObjs)
-          if (MCEPtr->getRecordDecl()->isInStdNamespace() &&
-              MCEPtr->getRecordDecl()->getCanonicalDecl()->getName() ==
-                  SizedObj)
-            return false; // It is in fact safe
-    }
-
-  // Pattern 2:
-  if (auto *DRE = dyn_cast<DeclRefExpr>(Buf->IgnoreParenImpCasts())) {
-    ASTContext &Ctx = Finder->getASTContext();
-
-    if (auto *CAT = Ctx.getAsConstantArrayType(DRE->getType())) {
-      Expr::EvalResult ER;
-      // The array element type must be compatible with `char` otherwise an
-      // explicit cast will be needed, which will make this check unreachable.
-      // Therefore, the array extent is same as its' bytewise size.
-      if (Size->EvaluateAsInt(ER, Ctx)) {
-        APSInt EVal = ER.Val.getInt(); // Size must have integer type
-
-        return APSInt::compareValues(EVal, APSInt(CAT->getSize(), true)) != 0;
-      }
-    }
-  }
-  return true; // ptr and size are not in safe pattern
+  return !isHardcodedCountedByPointerArgumentSafe(
+      Finder->getASTContext(), Buf, Size, FirstParmTy.getTypePtr(), true,
+      false);
 }
 } // namespace libc_func_matchers
 
@@ -2303,9 +2342,11 @@ class UnsafeLibcFunctionCallGadget : public WarningGadget {
                 // safe:
                 functionDecl(libc_func_matchers::isUnsafeSprintfFunc())
                     .bind(UnsafeSprintfTag)))),
-            //  (unless the call has a sole string literal argument):
+            //  (unless the call has a sole null-terminated argument, e.g., strlen, printf, atoi):
             unless(
-                allOf(hasArgument(0, expr(stringLiteral())), hasNumArgs(1)))),
+                allOf(hasArgument(0,
+                expr(libc_func_matchers::isNullTermPointer())),
+                hasNumArgs(1)))),
 
         // The following two cases require checking against actual
         // arguments of the call:
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp
index 2bd12db93fd52..9ca1e6261de57 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-inline-namespace.cpp
@@ -23,7 +23,7 @@ namespace std {
     T* p;
     T *c_str();
     T *data();
-    unsigned size_bytes();
+    unsigned size();
   };
 
   typedef basic_string<char> string;
@@ -50,8 +50,8 @@ void f(char * p, char * q, std::span<char> s) {
 }
 
 void v(std::string s1) {
-  std::snprintf(s1.data(), s1.size_bytes(), "%s%d", s1.c_str(), 0); // no warn
-  std::__1::snprintf(s1.data(), s1.size_bytes(), "%s%d", s1.c_str(), 0); // no warn
+  std::snprintf(s1.data(), s1.size(), "%s%d", s1.c_str(), 0); // no warn
+  std::__1::snprintf(s1.data(), s1.size(), "%s%d", s1.c_str(), 0); // no warn
 }
 
 void g(char *begin, char *end, char *p, std::span<char> s) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
new file mode 100644
index 0000000000000..279cc173ff0c8
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN:            -verify -fexperimental-bounds-safety-attributes %s
+#include <ptrcheck.h>
+typedef unsigned size_t;
+typedef struct {} FILE;
+
+namespace annotated_libc {
+  // For libc functions that have annotations,
+  // `-Wunsafe-buffer-usage-in-libc-call` yields to the interoperation
+  // warnings.
+
+// expected-note@+2{{consider using a safe container and passing '.data()' to the parameter 'dst' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'dst'}}
+// 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, unsigned size);
+unsigned 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, unsigned buf_size, const char* format, ... );
+int snwprintf( char* __counted_by(buf_size) buffer, unsigned buf_size, const char* format, ... );
+int vsnprintf( char* __counted_by(buf_size) buffer, unsigned buf_size, const char* format, ... );
+int sprintf( char* __counted_by(10) buffer, const char* format, ... );
+
+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) {
+  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}}
+
+  // We still warn about unsafe string pointer arguments to printfs:
+
+  snprintf(safe_p, n, "%s", str);  // expected-warning{{function 'snprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
+  snwprintf(safe_p, n, "%s", str); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
+
+  memcpy(safe_p, safe_p, n);               // no warn
+  strlen(str);                             // expected-warning{{unsafe assignment to a parameter of '__null_terminated' type; only '__null_terminated' pointers, string literals, and 'std::string::c_str' calls are compatible with '__null_terminated' pointers}}
+  snprintf(safe_p, n, "%s", "hlo");        // no warn
+  snprintf(safe_p, n, "%s", safe_str);     // no warn
+  snwprintf(safe_p, n, "%s", safe_str);    // no warn
+
+  // v-printf functions and sprintf are still warned about because
+  // they cannot be fully safe:
+
+  vsnprintf(safe_p, n, "%s", safe_str); // 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}}
+
+}
+} // namespace annotated_libc
+
+namespace unannotated_libc {
+  // The -Wunsafe-buffer-usage analysis considers some printf
+  // functions safe, arguments are correctly annotated. Because these
+  // functions are harder to be changed to C++ equivalents.
+int printf(const char*, ... );
+int fprintf (FILE*, const char*, ... );
+int snprintf( char*, unsigned, const char*, ... );
+int snwprintf( char*, unsigned, const char*, ... );
+int vsnprintf( char*, unsigned, const char*, ... );
+  // It is convenient to accept functions like `strlen` or `atoi` when
+  // they take a __null_termianted argument.
+unsigned strlen( const char* );
+int atoi( const char* );
+
+void test(const char * __null_terminated safe_str,
+         char * __counted_by(n) safe_p,
+         size_t n) {
+  FILE *file;
+  printf("%s", safe_str);
+  fprintf(file, "%s", safe_str);
+  snprintf(safe_p, n, "%s", safe_str);
+  snwprintf(safe_p, n, "%s", safe_str);
+  strlen(safe_str);
+  atoi(safe_str);
+  printf(safe_str);
+  strlen(safe_p); // safe_p is not null-terminated  expected-warning{{function 'strlen' is unsafe}}
+
+  // v-printf functions and sprintf are still warned about because
+  // they cannot be fully safe:
+  vsnprintf(safe_p, n, "%s", safe_str); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}}
+}
+} // namespace unannotated_libc
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
index 98a4c7bc6943e..15a430b85885a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp
@@ -46,7 +46,7 @@ namespace std {
     T* p;
     T *c_str();
     T *data();
-    unsigned size_bytes();
+    unsigned size();
   };
 
   typedef basic_string<char> string;
@@ -84,7 +84,6 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
   snprintf(q, 10, "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
   snprintf(cp, 10, "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
   snprintf(s.data(), s2.size(), "%s%d", "hello", *p); // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
-  snprintf(nullptr, 0, "");                // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
   printf(nullptr);                         // expected-warning{{function 'printf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
   printf("%s", nullptr);                   // expected-warning{{function 'printf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
   snwprintf(s.data(), s2.size(), "%s%d", "hello", *p); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
@@ -105,7 +104,8 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
   std::wstring WS;
 
   snprintf(a, sizeof(b), "%s", __PRETTY_FUNCTION__);         // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
-  snprintf((char*)c, sizeof(c), "%s", __PRETTY_FUNCTION__);  // expected-warning{{function 'snprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
+  snprintf((char*)c, sizeof(c), "%s", __PRETTY_FUNCTION__);
+  snprintf(nullptr, 0, "");  // no warn
   fprintf((FILE*)p, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn
   fprintf(fp, "%P%d%p%i hello world %32s", *p, *p, p, *p, "hello"); // no warn
   printf("%s%d", "hello", *p); // no warn
@@ -120,8 +120,8 @@ void f(char * p, char * q, std::span<char> s, std::span<char> s2) {
 }
 
 void safe_examples(std::string s1, int *p) {
-  snprintf(s1.data(), s1.size_bytes(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
-  snprintf(s1.data(), s1.size_bytes(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str());      // no warn
+  snprintf(s1.data(), s1.size(), "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str()); // no warn
+  snprintf(s1.data(), s1.size(), s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str());      // no warn
   printf("%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str());              // no warn
   printf(s1.c_str(), __PRETTY_FUNCTION__, *p, "hello", s1.c_str());                   // no warn
   fprintf((FILE*)0, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", p, s1.c_str());   // no warn

From a33e41ac0319e57b43d87fc4b41833524e8175b1 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo@apple.com>
Date: Fri, 21 Feb 2025 16:22:08 -0800
Subject: [PATCH 2/7] Mark code in UnsafeBufferUsage.cpp that needs upstream

---
 clang/lib/Analysis/UnsafeBufferUsage.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a233859f34ceb..b47b6fdbb4314 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -366,6 +366,7 @@ isInUnspecifiedUntypedContext(internal::Matcher<Stmt> InnerMatcher) {
 
 namespace {
 
+/* TO_UPSTREAM(BoundsSafetyInterop) ON */
 // Finds the argument that is passed as dependent count.
 const Expr *findCountArg(const Expr *Count, const CallExpr *Call) {
   const auto *DRE = dyn_cast<DeclRefExpr>(Count->IgnoreParenImpCasts());
@@ -998,7 +999,7 @@ bool isSinglePointerArgumentSafe(ASTContext &Context, const Expr *Arg) {
 
   return false;
 }
-
+/* TO_UPSTREAM(BoundsSafetyInterop) OFF */
 } // namespace
 
 // Given a two-param std::span construct call, matches iff the call has the

From 2678bd8f3bdb86abcb4735887c96e921270883c5 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo@apple.com>
Date: Tue, 25 Feb 2025 16:35:03 -0800
Subject: [PATCH 3/7] Address comments

---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 59 ++++++++-----
 ...pan-construct-count-attributed-pointer.cpp |  8 ++
 ...fe-buffer-usage-libc-functions-interop.cpp | 86 +++++++++++++++----
 3 files changed, 116 insertions(+), 37 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index b47b6fdbb4314..98205b80bb3ba 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -806,15 +806,13 @@ static bool isCountAttributedPointerArgumentSafeImpl(
     const Expr * /* Nullable */ CountArg, const Type *CountedByPointerTy,
     const Expr * /* Nullable */ CountedByExpr, bool isSizedBy, bool isOrNull,
     const DependentValuesTy * /* Nullable */ DependentValueMap = nullptr) {
-  assert(CountedByExpr ||
-         CountArg &&
-             "Only one of them can be null. If the __counted_by information is "
-             "hardcoded, there must be an argument representing the actual "
-             "count.");
-  assert(CountedByExpr ||
-         !DependentValueMap &&
-             "If the __counted_by information is hardcoded, there is no "
-             "dependent value map.");
+  assert((CountedByExpr || CountArg) &&
+         "Only one of them can be null. If the __counted_by information is "
+         "hardcoded, there must be an argument representing the actual "
+         "count.");
+  assert((CountedByExpr || !DependentValueMap) &&
+         "If the __counted_by information is hardcoded, there is no "
+         "dependent value map.");
   const Expr *PtrArgNoImp = PtrArg->IgnoreParenImpCasts();
 
   // check form 0:
@@ -872,12 +870,12 @@ static bool isCountAttributedPointerArgumentSafeImpl(
     return false;
 
   if (!DependentValueMap && CountedByExpr)
-    // Bail if the map is not available for case (a). Becuase
+    // Bail if the map is not available for case (a). Because
     // we need the map to substitute parameters to arguments in case (a) but
     // not case(b).
     return false;
 
-  // the acutal count of the pointer inferred through patterns below:
+  // the actual count of the pointer inferred through patterns below:
   const Expr *ActualCount = nullptr;
   const Expr *MemberBase = nullptr;
 
@@ -891,7 +889,10 @@ static bool isCountAttributedPointerArgumentSafeImpl(
   } else if (const auto *ArgCAT =
                  PtrArgNoImp->getType()->getAs<CountAttributedType>()) {
     // Form 5.
-    if (ArgCAT->isOrNull() == isOrNull)
+    bool areBoundsAttributesCompatible =
+        (ArgCAT->isCountInBytes() == isSizedBy || (ArgInBytes && ParamInBytes));
+
+    if (ArgCAT->isOrNull() == isOrNull && areBoundsAttributesCompatible)
       ActualCount = ArgCAT->getCountExpr();
   } else {
     // Form 6-7.
@@ -935,7 +936,7 @@ static bool isCountAttributedPointerArgumentSafe(ASTContext &Context,
 
   return isCountAttributedPointerArgumentSafeImpl(
       Context, Arg, CountArg, CAT, CAT->getCountExpr(), CAT->isCountInBytes(),
-      CAT->isOrNull(), ValuesOpt ? &*ValuesOpt : nullptr);
+      CAT->isOrNull(), ValuesOpt.has_value() ? &*ValuesOpt : nullptr);
 }
 
 // Checks if arguments passed to a function, which has no annotation but
@@ -1013,7 +1014,7 @@ bool isSinglePointerArgumentSafe(ASTContext &Context, const Expr *Arg) {
 //   6. `std::span<T>{p, n}`, where `p` is a __counted_by(`n`)/__sized_by(`n`)
 //   pointer OR `std::span<char>{(char*)p, n}`, where `p` is a __sized_by(`n`)
 //   pointer.
-
+//   7. `std::span<char>{p, strlen(p)}` or `std::span<wchar_t>{p, wcslen(p)}`
 AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
   assert(Node.getNumArgs() == 2 &&
          "expecting a two-parameter std::span constructor");
@@ -1071,7 +1072,7 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
     break;
   }
 
-  QualType Arg0Ty = Arg0->IgnoreImplicit()->getType();
+  QualType Arg0Ty = Arg0->getType();
 
   if (auto *ConstArrTy =
           Finder->getASTContext().getAsConstantArrayType(Arg0Ty)) {
@@ -1127,6 +1128,20 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
                                      /*DependentValues=*/nullptr,
                                      Finder->getASTContext());
   }
+  // Check form 7:
+  if (const auto *Call = dyn_cast<CallExpr>(Arg1);
+      Call && Call->getNumArgs() == 1) {
+    if (const FunctionDecl *FD = Call->getDirectCallee();
+        FD && FD->getIdentifier()) {
+      StringRef FName = FD->getName();
+      const Expr *CallArg = Call->getArg(0)->IgnoreParenImpCasts();
+
+      // TODO: we can re-use `LibcFunNamePrefixSuffixParser` to support more
+      // variants, e.g., `strlen_s`
+      return (FName == "strlen" || FName == "wcslen") &&
+             AreSameDRE(Arg0, CallArg);
+    }
+  }
 
   return false;
 }
@@ -1630,11 +1645,15 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
       !Size->getType()->isIntegerType())
     return false; // not an snprintf call
 
-  Buf = Buf->IgnoreParenImpCasts();
-  Size = Size->IgnoreParenImpCasts();
-  return !isHardcodedCountedByPointerArgumentSafe(
-      Finder->getASTContext(), Buf, Size, FirstParmTy.getTypePtr(), true,
-      false);
+  if (auto NumChars =
+          Finder->getASTContext().getTypeSizeInCharsIfKnown(FirstPteTy)) {
+    Buf = Buf->IgnoreParenImpCasts();
+    Size = Size->IgnoreParenImpCasts();
+    return !isHardcodedCountedByPointerArgumentSafe(
+        Finder->getASTContext(), Buf, Size, FirstParmTy.getTypePtr(),
+        NumChars->isOne(), false);
+  }
+  return false;
 }
 } // namespace libc_func_matchers
 
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp
index e32223a69dcfd..d7fdc841907ee 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp
@@ -142,3 +142,11 @@ void span_from_output_parm(int * __counted_by(n)  *cb_p, size_t n,
   std::span<char>(sb_z, *l);  // no warn
   std::span<int>(*cb_q, n);   // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
 }
+
+size_t strlen( const char* __null_terminated str );
+size_t wcslen( const wchar_t* __null_terminated str );
+void test_span_ctor(const char * __null_terminated p,
+		    const wchar_t * __null_terminated wp) {
+  std::span S{p, strlen(p)};
+  std::span S2{wp, wcslen(wp)};
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
index 279cc173ff0c8..ff672ed39657f 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
@@ -1,9 +1,30 @@
-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -Wno-error=bounds-safety-strict-terminated-by-cast\
 // RUN:            -verify -fexperimental-bounds-safety-attributes %s
 #include <ptrcheck.h>
 typedef unsigned size_t;
 typedef struct {} FILE;
 
+namespace std {
+  template<typename T>
+  struct span {
+    T * ptr;
+    T * data();
+    unsigned size_bytes();
+    unsigned size();
+  };
+
+  template<typename T>
+  struct basic_string {
+    T* p;
+    T *c_str();
+    T *data();
+    unsigned size();
+  };
+
+  typedef basic_string<char> string;
+  typedef basic_string<wchar_t> wstring;
+}
+
 namespace annotated_libc {
   // For libc functions that have annotations,
   // `-Wunsafe-buffer-usage-in-libc-call` yields to the interoperation
@@ -11,12 +32,13 @@ namespace annotated_libc {
 
 // expected-note@+2{{consider using a safe container and passing '.data()' to the parameter 'dst' and '.size()' to its dependent parameter 'size' or 'std::span' and passing '.first(...).data()' to the parameter 'dst'}}
 // 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, unsigned size);
-unsigned strlen( const char* __null_terminated str );
+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, unsigned buf_size, const char* format, ... );
-int snwprintf( char* __counted_by(buf_size) buffer, unsigned buf_size, const char* format, ... );
-int vsnprintf( char* __counted_by(buf_size) buffer, unsigned buf_size, const char* format, ... );
+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 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, ... );
 int sprintf( char* __counted_by(10) buffer, const char* format, ... );
 
 void test(char * p, char * q, const char * str,
@@ -28,15 +50,12 @@ void test(char * p, char * q, const char * str,
   snprintf(p, 10, "%s", "hlo");      // expected-warning{{unsafe assignment to function parameter of count-attributed type}}
 
   // We still warn about unsafe string pointer arguments to printfs:
-
   snprintf(safe_p, n, "%s", str);  // expected-warning{{function 'snprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
-  snwprintf(safe_p, n, "%s", str); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
 
   memcpy(safe_p, safe_p, n);               // no warn
-  strlen(str);                             // expected-warning{{unsafe assignment to a parameter of '__null_terminated' type; only '__null_terminated' pointers, string literals, and 'std::string::c_str' calls are compatible with '__null_terminated' pointers}}
+  strlen(str);                             // expected-warning{{passing 'const char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
   snprintf(safe_p, n, "%s", "hlo");        // no warn
   snprintf(safe_p, n, "%s", safe_str);     // no warn
-  snwprintf(safe_p, n, "%s", safe_str);    // no warn
 
   // v-printf functions and sprintf are still warned about because
   // they cannot be fully safe:
@@ -45,6 +64,23 @@ void test(char * p, char * q, const char * str,
   sprintf(safe_ten, "%s", safe_str);    // expected-warning{{function 'sprintf' is unsafe}} expected-note{{change to 'snprintf' for explicit bounds checking}}
 
 }
+
+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,
+	  size_t n) {
+  std::wstring cxx_wstr;
+  std::span<wchar_t> cxx_wspan;
+
+  snwprintf(safe_p, n, L"%ls", safe_wstr);
+  snwprintf(cxx_wstr.data(), cxx_wstr.size(), cxx_wstr.c_str());
+  snwprintf(cxx_wspan.data(), cxx_wspan.size(), cxx_wspan.data()); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
+  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}}
+}
 } // namespace annotated_libc
 
 namespace unannotated_libc {
@@ -53,22 +89,21 @@ namespace unannotated_libc {
   // functions are harder to be changed to C++ equivalents.
 int printf(const char*, ... );
 int fprintf (FILE*, const char*, ... );
-int snprintf( char*, unsigned, const char*, ... );
-int snwprintf( char*, unsigned, const char*, ... );
-int vsnprintf( char*, unsigned, const char*, ... );
+int snprintf( char*, size_t, const char*, ... );
+int snwprintf( wchar_t*, size_t, const wchar_t*, ... );
+int vsnprintf( char*, size_t, const char*, ... );
   // It is convenient to accept functions like `strlen` or `atoi` when
   // they take a __null_termianted argument.
-unsigned strlen( const char* );
+size_t strlen( const char* );
 int atoi( const char* );
 
 void test(const char * __null_terminated safe_str,
-         char * __counted_by(n) safe_p,
-         size_t n) {
+	  char * __sized_by(n) safe_p,
+	  size_t n) {
   FILE *file;
   printf("%s", safe_str);
   fprintf(file, "%s", safe_str);
   snprintf(safe_p, n, "%s", safe_str);
-  snwprintf(safe_p, n, "%s", safe_str);
   strlen(safe_str);
   atoi(safe_str);
   printf(safe_str);
@@ -78,4 +113,21 @@ void test(const char * __null_terminated safe_str,
   // they cannot be fully safe:
   vsnprintf(safe_p, n, "%s", safe_str); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}}
 }
+
+void test_wchar(const wchar_t * unsafe_wstr,
+		const wchar_t * __null_terminated safe_wstr,
+	        wchar_t * __null_terminated nt_wstr,
+		wchar_t * __sized_by(n) sizedby_wp, // a 'wchar_t' is larger than a `char`
+		wchar_t * __counted_by(n) safe_wp,
+		size_t n) {
+  std::wstring cxx_wstr;
+  std::span<wchar_t> cxx_wspan;
+
+  snwprintf(cxx_wstr.data(), cxx_wstr.size(), cxx_wstr.c_str());
+  snwprintf(safe_wp, n, safe_wstr);
+  snwprintf(cxx_wspan.data(), cxx_wspan.size(), cxx_wspan.data()); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
+  snwprintf(sizedby_wp, n, safe_wstr); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{buffer pointer and size may not match}}
+  snwprintf(safe_wp, n, unsafe_wstr); // expected-warning{{function 'snwprintf' is unsafe}} expected-note{{string argument is not guaranteed to be null-terminated}}
+
+}
 } // namespace unannotated_libc

From db61bca828eaab5f8df97704ad23806844b3d982 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo@apple.com>
Date: Tue, 25 Feb 2025 16:56:17 -0800
Subject: [PATCH 4/7] add comments for sprintf test

---
 .../warn-unsafe-buffer-usage-libc-functions-interop.cpp      | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
index ff672ed39657f..47eb5f22ccf82 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
@@ -39,6 +39,11 @@ int snprintf( char* __counted_by(buf_size) buffer, size_t buf_size, const char*
 // 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 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, ... );
+
+// The '__counted_by(10)' is not a correct bounds annotation for
+// 'sprintf'. It is used to test that even if 'sprintf' has bounds
+// annotations, the function will still be warned against as 'sprintf'
+// can't be safe.
 int sprintf( char* __counted_by(10) buffer, const char* format, ... );
 
 void test(char * p, char * q, const char * str,

From 4cce6a805ec12066a53c4c34abbf5a725bdca084 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo@apple.com>
Date: Fri, 28 Feb 2025 11:01:53 -0800
Subject: [PATCH 5/7] address comments

---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 11 +++++----
 ...pan-construct-count-attributed-pointer.cpp | 14 ++++++++++-
 ...fe-buffer-usage-libc-functions-interop.cpp | 24 +++++++++++--------
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 98205b80bb3ba..3f12fdfc197e4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -759,7 +759,8 @@ static bool hasIntegeralConstant(const Expr *E, uint64_t Val, ASTContext &Ctx) {
 // 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`.
-// 1. `&var`, if `var` is a variable identifier and the dependent count is `1`.
+// 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
 //     equivalent to `sizeof(var)`.
 // 3. `sp.data()` if the argument to dependent count is `sp.size()`.
@@ -832,14 +833,14 @@ static bool isCountAttributedPointerArgumentSafeImpl(
   if (auto *DRE = selectFirst<const DeclRefExpr>(
           "VarIdent", match(AddressofDRE, *PtrArgNoImp, Context))) {
     if (CountArg) {
-      if (!isSizedBy) // form 1:
+      if (!isSizedBy) // form 1.a.:
         return hasIntegeralConstant(CountArg, 1, Context);
       // form 2:
       if (auto TySize = Context.getTypeSizeInCharsIfKnown(DRE->getType()))
         return hasIntegeralConstant(CountArg, TySize->getQuantity(), Context);
     } else
-      // When there is no argument representing the count/size, it is safe iff
-      // the annotation is `__counted_by(1)`.
+      // form 1.b.: when there is no argument representing the count/size, it is
+      // safe iff the annotation is `__counted_by(1)`.
       return !isSizedBy && hasIntegeralConstant(CountedByExpr, 1, Context);
     return false;
   }
@@ -1645,7 +1646,7 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {
       !Size->getType()->isIntegerType())
     return false; // not an snprintf call
 
-  if (auto NumChars =
+  if (std::optional<CharUnits> NumChars =
           Finder->getASTContext().getTypeSizeInCharsIfKnown(FirstPteTy)) {
     Buf = Buf->IgnoreParenImpCasts();
     Size = Size->IgnoreParenImpCasts();
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp
index d7fdc841907ee..0e6d0c9941d3b 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct-count-attributed-pointer.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-container -fexperimental-bounds-safety-attributes -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-container\
+// RUN: -Wno-error=bounds-safety-strict-terminated-by-cast -fexperimental-bounds-safety-attributes -verify %s
 
 #include <ptrcheck.h>
 #include <stddef.h>
@@ -145,8 +146,19 @@ void span_from_output_parm(int * __counted_by(n)  *cb_p, size_t n,
 
 size_t strlen( const char* __null_terminated str );
 size_t wcslen( const wchar_t* __null_terminated str );
+
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wunsafe-buffer-usage"
+
 void test_span_ctor(const char * __null_terminated p,
 		    const wchar_t * __null_terminated wp) {
   std::span S{p, strlen(p)};
   std::span S2{wp, wcslen(wp)};
 }
+
+void test_span_ctor_warn(const char * p,
+			 const wchar_t * wp) {
+  std::span S{p, strlen(p)}; // expected-warning{{passing 'const char *' to parameter of incompatible type 'const char * __terminated_by(0)' (aka 'const char *') is an unsafe operation}}
+  std::span S2{wp, wcslen(wp)}; // expected-warning{{passing 'const wchar_t *' to parameter of incompatible type 'const wchar_t * __terminated_by(0)' (aka 'const wchar_t *') is an unsafe operation}}
+}
+#pragma clang diagnostic pop
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
index 47eb5f22ccf82..3c2e0cc2c9a76 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
@@ -5,20 +5,24 @@ typedef unsigned size_t;
 typedef struct {} FILE;
 
 namespace std {
-  template<typename T>
+  template <typename T>
   struct span {
-    T * ptr;
-    T * data();
-    unsigned size_bytes();
-    unsigned size();
+    T *data() const noexcept;
+    size_t size() const noexcept;
+    size_t size_bytes() const noexcept;
+    span<T> first(size_t count) const noexcept;
+    span<T> last(size_t count) const noexcept;
+    span<T> subspan(size_t offset, size_t count) const noexcept;
+    const T &operator[](const size_t idx) const;
   };
 
-  template<typename T>
+  template <typename CharT>
   struct basic_string {
-    T* p;
-    T *c_str();
-    T *data();
-    unsigned size();
+    const CharT *data() const noexcept;
+    CharT *data() noexcept;
+    const CharT *c_str() const noexcept;
+    size_t size() const noexcept;
+    size_t length() const noexcept;
   };
 
   typedef basic_string<char> string;

From bad0c198caa6260ba30115e598418cdaedc02bcb Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo@apple.com>
Date: Fri, 28 Feb 2025 12:44:32 -0800
Subject: [PATCH 6/7] Improve the fidelity of vsnprintf declarations in a test

---
 ...rn-unsafe-buffer-usage-libc-functions-interop.cpp | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
index 3c2e0cc2c9a76..b24948b9eebc2 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
@@ -3,6 +3,7 @@
 #include <ptrcheck.h>
 typedef unsigned size_t;
 typedef struct {} FILE;
+typedef struct {} va_list;
 
 namespace std {
   template <typename T>
@@ -42,7 +43,7 @@ size_t strlen( const char* __null_terminated str );
 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 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, ... );
+int vsnprintf( char* __counted_by(buf_size) buffer, size_t buf_size, const char* format, va_list va_args);
 
 // The '__counted_by(10)' is not a correct bounds annotation for
 // 'sprintf'. It is used to test that even if 'sprintf' has bounds
@@ -68,8 +69,8 @@ void test(char * p, char * q, const char * str,
 
   // v-printf functions and sprintf are still warned about because
   // they cannot be fully safe:
-
-  vsnprintf(safe_p, n, "%s", safe_str); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}}
+  va_list vlist;
+  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}}
 
 }
@@ -100,7 +101,7 @@ int printf(const char*, ... );
 int fprintf (FILE*, const char*, ... );
 int snprintf( char*, size_t, const char*, ... );
 int snwprintf( wchar_t*, size_t, const wchar_t*, ... );
-int vsnprintf( char*, size_t, const char*, ... );
+int vsnprintf( char*, size_t, const char*, va_list va_args );
   // It is convenient to accept functions like `strlen` or `atoi` when
   // they take a __null_termianted argument.
 size_t strlen( const char* );
@@ -120,7 +121,8 @@ void test(const char * __null_terminated safe_str,
 
   // v-printf functions and sprintf are still warned about because
   // they cannot be fully safe:
-  vsnprintf(safe_p, n, "%s", safe_str); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}}
+  va_list vlist;
+  vsnprintf(safe_p, n, "%s", vlist); // expected-warning{{function 'vsnprintf' is unsafe}} expected-note{{'va_list' is unsafe}}
 }
 
 void test_wchar(const wchar_t * unsafe_wstr,

From ddce0718f286507a84e2c40576be3b64db2df3a9 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo@apple.com>
Date: Mon, 3 Mar 2025 21:20:15 -0800
Subject: [PATCH 7/7] add stddef.h to a test

---
 .../warn-unsafe-buffer-usage-libc-functions-interop.cpp        | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
index b24948b9eebc2..d600ae1d8ddb3 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions-interop.cpp
@@ -1,7 +1,8 @@
 // RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -Wno-error=bounds-safety-strict-terminated-by-cast\
 // RUN:            -verify -fexperimental-bounds-safety-attributes %s
 #include <ptrcheck.h>
-typedef unsigned size_t;
+#include <stddef.h>
+
 typedef struct {} FILE;
 typedef struct {} va_list;