[libcxx] [llvm] [libc++] Diagnose when nullptrs are passed to string APIs (PR #122790)
Nikolas Klauser via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 22 02:02:23 PST 2025
https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/122790
>From 2e6e4bbb01f45afb693bf331d38da2f8b7e69cf6 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 | 41 +++++++++++++++++++
libcxx/utils/libcxx/test/params.py | 3 ++
runtimes/cmake/Modules/WarningFlags.cmake | 1 +
5 files changed, 72 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 53900e40655ef..5ed36d210e1e5 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -1201,6 +1201,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
+
// TODO(LLVM 22): Remove this macro once LLVM19 support ends. __cpp_explicit_this_parameter has been set in LLVM20.
// 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)
diff --git a/libcxx/include/string b/libcxx/include/string
index e2968621bb314..2e03159bb6990 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1037,13 +1037,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));
@@ -1214,7 +1215,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
@@ -1332,7 +1334,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);
}
@@ -1373,7 +1376,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);
@@ -1531,7 +1534,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);
@@ -1711,7 +1714,7 @@ public:
}
_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_ASSERT_NON_NULL(__s != nullptr, "string::find(): received nullptr");
return std::__str_find<value_type, size_type, traits_type, npos>(
data(), size(), __s, __pos, traits_type::length(__s));
@@ -1742,7 +1745,7 @@ public:
}
_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_ASSERT_NON_NULL(__s != nullptr, "string::rfind(): received nullptr");
return std::__str_rfind<value_type, size_type, traits_type, npos>(
data(), size(), __s, __pos, traits_type::length(__s));
@@ -1775,7 +1778,7 @@ public:
}
_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_ASSERT_NON_NULL(__s != nullptr, "string::find_first_of(): received nullptr");
return std::__str_find_first_of<value_type, size_type, traits_type, npos>(
data(), size(), __s, __pos, traits_type::length(__s));
@@ -1809,7 +1812,7 @@ public:
}
_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_ASSERT_NON_NULL(__s != nullptr, "string::find_last_of(): received nullptr");
return std::__str_find_last_of<value_type, size_type, traits_type, npos>(
data(), size(), __s, __pos, traits_type::length(__s));
@@ -1843,7 +1846,7 @@ public:
}
_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_ASSERT_NON_NULL(__s != nullptr, "string::find_first_not_of(): received nullptr");
return std::__str_find_first_not_of<value_type, size_type, traits_type, npos>(
data(), size(), __s, __pos, traits_type::length(__s));
@@ -1877,7 +1880,7 @@ public:
}
_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_ASSERT_NON_NULL(__s != nullptr, "string::find_last_not_of(): received nullptr");
return std::__str_find_last_not_of<value_type, size_type, traits_type, npos>(
data(), size(), __s, __pos, traits_type::length(__s));
@@ -1925,12 +1928,13 @@ public:
return __self_view(*this).substr(__pos1, __n1).compare(__sv.substr(__pos2, __n2));
}
- _LIBCPP_CONSTEXPR_SINCE_CXX20 int compare(const value_type* __s) const _NOEXCEPT {
+ _LIBCPP_CONSTEXPR_SINCE_CXX20 int compare(const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) const _NOEXCEPT {
_LIBCPP_ASSERT_NON_NULL(__s != nullptr, "string::compare(): received nullptr");
return compare(0, npos, __s, traits_type::length(__s));
}
- _LIBCPP_CONSTEXPR_SINCE_CXX20 int compare(size_type __pos1, size_type __n1, const value_type* __s) const {
+ _LIBCPP_CONSTEXPR_SINCE_CXX20 int
+ compare(size_type __pos1, size_type __n1, const value_type* _LIBCPP_DIAGNOSE_NONNULL __s) const {
_LIBCPP_ASSERT_NON_NULL(__s != nullptr, "string::compare(): received nullptr");
return compare(__pos1, __n1, __s, traits_type::length(__s));
}
@@ -1949,7 +1953,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));
}
@@ -1963,7 +1967,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
@@ -1979,7 +1983,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 0000000000000..d61896277afd4
--- /dev/null
+++ b/libcxx/test/libcxx/strings/basic.string/nonnull.verify.cpp
@@ -0,0 +1,41 @@
+//===----------------------------------------------------------------------===//
+//
+// 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>
+
+#include "test_macros.h"
+
+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}}
+
+#if TEST_STD_VER >= 20
+ str2.starts_with(np); // expected-warning {{null passed}}
+ str2.ends_with(np); // expected-warning {{null passed}}
+#endif
+#if TEST_STD_VER >= 23
+ str2.contains(np); // expected-warning {{null passed}}
+#endif
+}
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 7dba39bc5db04..93dc3a8be3f08 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -72,6 +72,9 @@
# 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",
+
+ # We're not annotating all the APIs, since that's a lot of annotations compared to how many we actually care about
+ "-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 d17bf92389d0b..43ef76561cc54 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