[-Wunsafe-buffer-usage][BoundsSafety] support __null_terminated in the interop mode

For a __null_terminated pointer 'p', only a __null_terminated pointer,
a string literal or a call to `std::string::c_str` can be assigned to
(/passed to/used to initialize) 'p'.  Otherwise, in the interop mode,
an -Wunsafe-buffer-usage warning will be reported.

Similar to how __counted_by is handled, incompatibility in nested
__null_terminated pointers (e.g., input/output pointer type arguments)
is reported as Bounds Safety type checking errors.

(rdar://138798346)

Author: ziqingluo-90

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -13603,6 +13603,19 @@ def note_unsafe_count_attributed_pointer_argument : Note<
 def warn_unsafe_single_pointer_argument : Warning<
   "unsafe assignment to function parameter of __single pointer type">,
   InGroup<UnsafeBufferUsage>, DefaultIgnore;
+def warn_unsafe_assign_to_null_terminated_pointer : Warning<
+  "unsafe %select{assignment to a pointer of|"
+                  "assignment to a parameter of|"
+                  "return of|"
+                  "conversion to|"
+                  "initialization of a pointer of|"
+                  "assignment to a pointer of|"
+                  "casting to}2 '__null_terminated' type; "
+  "only '__null_terminated' pointers, string literals, and 'std::string::c_str' calls are compatible with '__null_terminated' pointers">,
+  InGroup<UnsafeBufferUsage>, DefaultIgnore;
+def warn_unsafe_named_cast_to_null_terminated_pointer : Warning<
+  "C++ named cast to '__null_terminated' type is unsafe">,
+  InGroup<UnsafeBufferUsage>, DefaultIgnore;
 #ifndef NDEBUG
 // Not a user-facing diagnostic. Useful for debugging false negatives in
 // -fsafe-buffer-usage-suggestions (i.e. lack of -Wunsafe-buffer-usage fixits).

clang/lib/Sema/SemaCast.cpp

@@ -205,7 +205,7 @@ namespace {
       /// int *__bidi_indexable p1;
       /// int *__indexable p2 = (int *__indexable)p1;
       /// \endcode
-      if (Self.getLangOpts().BoundsSafety) {
+      if (Self.getLangOpts().hasBoundsSafetyAttributes()) {
         unsigned DiagKind = 0;
         bool isInvalid = false;
         // The type error may be nested, so any pointer can result in VT errors
@@ -238,6 +238,17 @@ namespace {
           const auto *DstPointerType = DestType->getAs<ValueTerminatedType>();
           IsDstNullTerm =
               DstPointerType->getTerminatorValue(Self.Context).isZero();
+          if (Self.LangOpts.isBoundsSafetyAttributeOnlyMode() &&
+              Self.LangOpts.CPlusPlus &&
+              !Self.Diags.isIgnored(
+                  diag::warn_unsafe_assign_to_null_terminated_pointer,
+                  SrcExpr.get()->getExprLoc())) {
+            // In C++ Safe Buffers/Bounds Safety interop mode, the compiler
+            // reports a warning under -Wunsafe-buffer-usage:
+            DiagKind = diag::warn_unsafe_assign_to_null_terminated_pointer;
+            isInvalid = true;
+            break;
+          }
           if (Self.getLangOpts().BoundsSafetyRelaxedSystemHeaders) {
             DiagKind = diag::
                 warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by;
@@ -576,6 +587,17 @@ Sema::BuildCXXNamedCast(SourceLocation OpLoc, tok::TokenKind Kind,
 
   Op.checkQualifiedDestType();
 
+  // In C++ Safe Buffers/Bounds Safety interop mode, forbidding  for now all
+  // named cast to __null_terminated pointer type.
+  if (DestType->isPointerType() && LangOpts.hasBoundsSafetyAttributes() &&
+      !Diags.isIgnored(diag::warn_unsafe_named_cast_to_null_terminated_pointer,
+                       E->getBeginLoc())) {
+    if (DestType->getAs<ValueTerminatedType>()) {
+      Diag(E->getBeginLoc(),
+           diag::warn_unsafe_named_cast_to_null_terminated_pointer);
+      return ExprError();
+    }
+  }
   switch (Kind) {
   default: llvm_unreachable("Unknown C++ cast!");
 

clang/lib/Sema/SemaExpr.cpp

@@ -12186,13 +12186,39 @@ Sema::CheckValueTerminatedAssignmentConstraints(QualType LHSType,
              Context.hasSameUnqualifiedType(
                  Context.getCorrespondingSignedType(LPointee),
                  Context.getCorrespondingSignedType(RPointee)));
-        if (CompatiblePointees &&
+        if (CompatiblePointees && RHSExpr->isPRValue() &&
             RHSExpr->tryEvaluateTerminatorElement(Evald, Context) &&
             Evald.Val.isInt() &&
             llvm::APSInt::isSameValue(LVTT->getTerminatorValue(Context),
                                       Evald.Val.getInt())) {
           return Sema::Compatible;
         }
+        // If in C++ Safe Buffers/Bounds Safety interoperation mode, the RHS can
+        // be 'std::string::c_str':
+        bool SafeBuffersEnabled = !Diags.isIgnored(
+            diag::warn_unsafe_assign_to_null_terminated_pointer,
+            RHSExpr->getBeginLoc());
+
+        if (LangOpts.CPlusPlus && SafeBuffersEnabled) {
+          if (const auto *MCE =
+                  dyn_cast<CXXMemberCallExpr>(RHSExpr->IgnoreParenImpCasts())) {
+            IdentifierInfo *MethodDeclII = nullptr, *ObjTypeII = nullptr;
+
+            if (MCE->getMethodDecl())
+              MethodDeclII = MCE->getMethodDecl()->getIdentifier();
+            if (const auto *RecordDecl =
+                    MCE->getObjectType()->getAsRecordDecl()) {
+              // If not in std namespace, let `ObjTypeII` be null so that the
+              // match fails:
+              if (RecordDecl->isInStdNamespace())
+                ObjTypeII = RecordDecl->getIdentifier();
+            }
+            if (MethodDeclII && ObjTypeII &&
+                MethodDeclII->getName() == "c_str" &&
+                ObjTypeII->getName() == "basic_string")
+              return Sema::Compatible;
+          }
+        }
       }
       return Nested
                  ? Sema::
@@ -20783,6 +20809,16 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType ConvTy,
     const auto *DstPointerType = DstType->getAs<ValueTerminatedType>();
     IsDstNullTerm =
         DstPointerType->getTerminatorValue(getASTContext()).isZero();
+    if (getLangOpts().isBoundsSafetyAttributeOnlyMode() &&
+        getLangOpts().CPlusPlus &&
+        !Diags.isIgnored(diag::warn_unsafe_assign_to_null_terminated_pointer,
+                         Loc)) {
+      // In C++ Safe Buffers/Bounds Safety interop mode, the compiler reports a
+      // warning under -Wunsafe-buffer-usage:
+      DiagKind = diag::warn_unsafe_assign_to_null_terminated_pointer;
+      isInvalid = false;
+      break;
+    }
     if (getLangOpts().BoundsSafetyRelaxedSystemHeaders) {
       DiagKind =
           diag::warn_bounds_safety_incompatible_non_terminated_by_to_terminated_by;

clang/lib/Sema/SemaExprCXX.cpp

@@ -4225,6 +4225,17 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
                                 const ImplicitConversionSequence &ICS,
                                 AssignmentAction Action,
                                 CheckedConversionKind CCK) {
+  if (ToType->isPointerType() && LangOpts.hasBoundsSafetyAttributes() &&
+      !Diags.isIgnored(diag::warn_unsafe_assign_to_null_terminated_pointer,
+                       From->getBeginLoc())) {
+    // In C++ Safe Buffers/Bounds Safety interop mode, warn about implicit
+    // conversions from non-`__null_terminated` to `__null_terminated`:
+    AssignConvertType ConvTy =
+        CheckValueTerminatedAssignmentConstraints(ToType, From);
+    if (DiagnoseAssignmentResult(ConvTy, From->getExprLoc(), ToType,
+                                 From->getType(), From, Action))
+      return ExprError();
+  }
   // C++ [over.match.oper]p7: [...] operands of class type are converted [...]
   if (CCK == CheckedConversionKind::ForBuiltinOverloadedOp &&
       !From->getType()->isRecordType())

clang/lib/Sema/SemaInit.cpp

@@ -9046,7 +9046,7 @@ ExprResult InitializationSequence::Perform(Sema &S,
                           Entity.getKind() == InitializedEntity::EK_Result);
 
   /* TO_UPSTREAM(BoundsSafety) ON*/
-  if (S.LangOpts.BoundsSafety) {
+  if (S.LangOpts.hasBoundsSafetyAttributes()) {
     auto *Init = CurInit.get();
     const auto K = Entity.getKind();
     if (Init && Entity.getParent() == nullptr &&

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

@@ -4,7 +4,10 @@
 // RUN:            -verify %s -x objective-c++
 // RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage-in-libc-call \
 // RUN:            -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN:            -verify -fexperimental-bounds-safety-attributes %s -DINTEROP_MODE
 
+#ifndef INTEROP_MODE
 typedef struct {} FILE;
 void memcpy();
 void __asan_memcpy();
@@ -155,3 +158,47 @@ void ff(char * p, char * q, std::span<char> s, std::span<char> s2) {
   wcscpy_s();
 #pragma clang diagnostic pop
 }
+
+#else
+
+#include <ptrcheck.h>
+typedef unsigned size_t;
+
+// 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}}
+
+}
+
+#endif //#ifdef INTEROP_MODE