[libcxx] [llvm] [libc++] Diagnose when nullptrs are passed to string APIs (PR #122790)

Nikolas Klauser via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 13 12:53:23 PST 2025


https://github.com/philnik777 created https://github.com/llvm/llvm-project/pull/122790

This allows catching misuses of APIs that take a pointer to a null-terminated string.

CC @thakis


>From ab8a7bc823bbc478344870d9947fcfac8f7a15cb Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Mon, 13 Jan 2025 21:46:42 +0100
Subject: [PATCH] [libc++] Diagnose when nullptrs are passed to string APIs

---
 libcxx/include/__config                       |  6 +++
 libcxx/include/string                         | 38 ++++++++++---------
 .../strings/basic.string/nonnull.verify.cpp   | 34 +++++++++++++++++
 libcxx/utils/libcxx/test/params.py            |  2 +
 runtimes/cmake/Modules/WarningFlags.cmake     |  1 +
 5 files changed, 64 insertions(+), 17 deletions(-)
 create mode 100644 libcxx/test/libcxx/strings/basic.string/nonnull.verify.cpp

diff --git a/libcxx/include/__config b/libcxx/include/__config
index 658a7e16fae916..751eb41a896fe2 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -1222,6 +1222,12 @@ typedef __char32_t char32_t;
 #    define _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK
 #  endif
 
+#  if __has_feature(nullability)
+#    define _LIBCPP_DIAGNOSE_NONNULL _Nonnull
+#  else
+#    define _LIBCPP_DIAGNOSE_NONNULL
+#  endif
+
 // Clang-18 has support for deducing this, but it does not set the FTM.
 #  if defined(__cpp_explicit_this_parameter) || (defined(_LIBCPP_CLANG_VER) && _LIBCPP_CLANG_VER >= 1800)
 #    define _LIBCPP_HAS_EXPLICIT_THIS_PARAMETER 1
diff --git a/libcxx/include/string b/libcxx/include/string
index 39982d5670bdbb..40887fbdc5dfce 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1079,13 +1079,14 @@ public:
 #  endif // _LIBCPP_CXX03_LANG
 
   template <__enable_if_t<__is_allocator<_Allocator>::value, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string(const _CharT* __s) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string(const _CharT* _LIBCPP_DIAGNOSE_NONNULL __s) {
     _LIBCPP_ASSERT_NON_NULL(__s != nullptr, "basic_string(const char*) detected nullptr");
     __init(__s, traits_type::length(__s));
   }
 
   template <__enable_if_t<__is_allocator<_Allocator>::value, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string(const _CharT* __s, const _Allocator& __a)
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
+  basic_string(const _CharT* _LIBCPP_DIAGNOSE_NONNULL __s, const _Allocator& __a)
       : __alloc_(__a) {
     _LIBCPP_ASSERT_NON_NULL(__s != nullptr, "basic_string(const char*, allocator) detected nullptr");
     __init(__s, traits_type::length(__s));
@@ -1256,7 +1257,8 @@ public:
     return assign(__il.begin(), __il.size());
   }
 #  endif
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& operator=(const value_type* __s) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string&
+  operator=(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) {
     return assign(__s);
   }
 #  if _LIBCPP_STD_VER >= 23
@@ -1374,7 +1376,8 @@ public:
     return append(__sv);
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& operator+=(const value_type* __s) {
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string&
+  operator+=(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) {
     return append(__s);
   }
 
@@ -1415,7 +1418,7 @@ public:
   append(const _Tp& __t, size_type __pos, size_type __n = npos);
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& append(const value_type* __s, size_type __n);
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& append(const value_type* __s);
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& append(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& append(size_type __n, value_type __c);
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __append_default_init(size_type __n);
@@ -1573,7 +1576,7 @@ public:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string&
   insert(size_type __pos1, const basic_string& __str, size_type __pos2, size_type __n = npos);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& insert(size_type __pos, const value_type* __s, size_type __n);
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& insert(size_type __pos, const value_type* __s);
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& insert(size_type __pos, const value_type* _LIBCPP_DIAGNOSE_NONNULL __s);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& insert(size_type __pos, size_type __n, value_type __c);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 iterator insert(const_iterator __pos, value_type __c);
 
@@ -1739,7 +1742,7 @@ public:
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type find(const value_type* __s, size_type __pos, size_type __n) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
-  find(const value_type* __s, size_type __pos = 0) const _NOEXCEPT;
+  find(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s, size_type __pos = 0) const _NOEXCEPT;
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type find(value_type __c, size_type __pos = 0) const _NOEXCEPT;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
@@ -1751,7 +1754,7 @@ public:
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type rfind(const value_type* __s, size_type __pos, size_type __n) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
-  rfind(const value_type* __s, size_type __pos = npos) const _NOEXCEPT;
+  rfind(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s, size_type __pos = npos) const _NOEXCEPT;
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type rfind(value_type __c, size_type __pos = npos) const _NOEXCEPT;
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
@@ -1764,7 +1767,7 @@ public:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_first_of(const value_type* __s, size_type __pos, size_type __n) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
-  find_first_of(const value_type* __s, size_type __pos = 0) const _NOEXCEPT;
+  find_first_of(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s, size_type __pos = 0) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_first_of(value_type __c, size_type __pos = 0) const _NOEXCEPT;
 
@@ -1778,7 +1781,7 @@ public:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_last_of(const value_type* __s, size_type __pos, size_type __n) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
-  find_last_of(const value_type* __s, size_type __pos = npos) const _NOEXCEPT;
+  find_last_of(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s, size_type __pos = npos) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_last_of(value_type __c, size_type __pos = npos) const _NOEXCEPT;
 
@@ -1792,7 +1795,7 @@ public:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_first_not_of(const value_type* __s, size_type __pos, size_type __n) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
-  find_first_not_of(const value_type* __s, size_type __pos = 0) const _NOEXCEPT;
+  find_first_not_of(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s, size_type __pos = 0) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_first_not_of(value_type __c, size_type __pos = 0) const _NOEXCEPT;
 
@@ -1806,7 +1809,7 @@ public:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_last_not_of(const value_type* __s, size_type __pos, size_type __n) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
-  find_last_not_of(const value_type* __s, size_type __pos = npos) const _NOEXCEPT;
+  find_last_not_of(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s, size_type __pos = npos) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
   find_last_not_of(value_type __c, size_type __pos = npos) const _NOEXCEPT;
 
@@ -1832,8 +1835,9 @@ public:
   inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 int
   compare(size_type __pos1, size_type __n1, const _Tp& __t, size_type __pos2, size_type __n2 = npos) const;
 
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 int compare(const value_type* __s) const _NOEXCEPT;
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 int compare(size_type __pos1, size_type __n1, const value_type* __s) const;
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 int compare(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) const _NOEXCEPT;
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 int
+  compare(size_type __pos1, size_type __n1, const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) const;
   _LIBCPP_CONSTEXPR_SINCE_CXX20 int
   compare(size_type __pos1, size_type __n1, const value_type* __s, size_type __n2) const;
 
@@ -1846,7 +1850,7 @@ public:
     return !empty() && _Traits::eq(front(), __c);
   }
 
-  constexpr _LIBCPP_HIDE_FROM_ABI bool starts_with(const value_type* __s) const noexcept {
+  constexpr _LIBCPP_HIDE_FROM_ABI bool starts_with(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) const noexcept {
     return starts_with(__self_view(__s));
   }
 
@@ -1858,7 +1862,7 @@ public:
     return !empty() && _Traits::eq(back(), __c);
   }
 
-  constexpr _LIBCPP_HIDE_FROM_ABI bool ends_with(const value_type* __s) const noexcept {
+  constexpr _LIBCPP_HIDE_FROM_ABI bool ends_with(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) const noexcept {
     return ends_with(__self_view(__s));
   }
 #  endif
@@ -1872,7 +1876,7 @@ public:
     return __self_view(typename __self_view::__assume_valid(), data(), size()).contains(__c);
   }
 
-  constexpr _LIBCPP_HIDE_FROM_ABI bool contains(const value_type* __s) const {
+  constexpr _LIBCPP_HIDE_FROM_ABI bool contains(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) const {
     return __self_view(typename __self_view::__assume_valid(), data(), size()).contains(__s);
   }
 #  endif
diff --git a/libcxx/test/libcxx/strings/basic.string/nonnull.verify.cpp b/libcxx/test/libcxx/strings/basic.string/nonnull.verify.cpp
new file mode 100644
index 00000000000000..64a46aadcd0ef2
--- /dev/null
+++ b/libcxx/test/libcxx/strings/basic.string/nonnull.verify.cpp
@@ -0,0 +1,34 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// Ensure that APIs which take a CharT* (and no size for it) are diagnosing passing a nullptr to them
+
+#include <string>
+
+void func() {
+  const char* const np = nullptr;
+  std::string str1(np);                         // expected-warning {{null passed}}
+  std::string str2(np, std::allocator<char>{}); // expected-warning {{null passed}}
+  str2 = np;                                    // expected-warning {{null passed}}
+  str2 += np;                                   // expected-warning {{null passed}}
+  str2.append(np);                              // expected-warning {{null passed}}
+  str2.insert(0, np);                           // expected-warning {{null passed}}
+  str2.find(np);                                // expected-warning {{null passed}}
+  str2.rfind(np);                               // expected-warning {{null passed}}
+  str2.find_first_of(np);                       // expected-warning {{null passed}}
+  str2.find_last_of(np);                        // expected-warning {{null passed}}
+  str2.find_first_not_of(np);                   // expected-warning {{null passed}}
+  str2.find_last_not_of(np);                    // expected-warning {{null passed}}
+  str2.compare(np);                             // expected-warning {{null passed}}
+  str2.compare(0, 0, np);                       // expected-warning {{null passed}}
+  str2.starts_with(np);                         // expected-warning {{null passed}}
+  str2.ends_with(np);                           // expected-warning {{null passed}}
+  str2.contains(np);                            // expected-warning {{null passed}}
+}
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 947cfd26513643..17268a29778252 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -71,6 +71,8 @@
 
     # This doesn't make sense in real code, but we have to test it because the standard requires us to not break
     "-Wno-self-move",
+
+    "-Wno-nullability-completeness",
 ]
 
 _allStandards = ["c++03", "c++11", "c++14", "c++17", "c++20", "c++23", "c++26"]
diff --git a/runtimes/cmake/Modules/WarningFlags.cmake b/runtimes/cmake/Modules/WarningFlags.cmake
index d17bf92389d0b0..43ef76561cc54a 100644
--- a/runtimes/cmake/Modules/WarningFlags.cmake
+++ b/runtimes/cmake/Modules/WarningFlags.cmake
@@ -25,6 +25,7 @@ function(cxx_add_warning_flags target enable_werror enable_pedantic)
       -Wformat-nonliteral
       -Wzero-length-array
       -Wdeprecated-redundant-constexpr-static-def
+      -Wno-nullability-completeness
       )
 
   if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")



More information about the llvm-commits mailing list