Skip to content

Conversation

@uzairnawaz
Copy link
Contributor

Added CRASH_ON_NULLPTR macro to wcspbrk function and related test

@llvmbot llvmbot added the libc label May 30, 2025
@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-libc

Author: Uzair Nawaz (uzairnawaz)

Changes

Added CRASH_ON_NULLPTR macro to wcspbrk function and related test


Full diff: https://github.com/llvm/llvm-project/pull/142216.diff

3 Files Affected:

  • (modified) libc/src/wchar/CMakeLists.txt (+1)
  • (modified) libc/src/wchar/wcspbrk.cpp (+4)
  • (modified) libc/test/src/wchar/wcspbrk_test.cpp (+10)
diff --git a/libc/src/wchar/CMakeLists.txt b/libc/src/wchar/CMakeLists.txt
index 50b17c617258e..f00e283e68201 100644
--- a/libc/src/wchar/CMakeLists.txt
+++ b/libc/src/wchar/CMakeLists.txt
@@ -66,6 +66,7 @@ add_entrypoint_object(
   DEPENDS
     libc.hdr.wchar_macros
     libc.src.__support.wctype_utils
+    libc.src.__support.macros.null_check
 )
 
 add_entrypoint_object(
diff --git a/libc/src/wchar/wcspbrk.cpp b/libc/src/wchar/wcspbrk.cpp
index bf305a5dbe125..a00ba9979a489 100644
--- a/libc/src/wchar/wcspbrk.cpp
+++ b/libc/src/wchar/wcspbrk.cpp
@@ -10,6 +10,7 @@
 
 #include "hdr/types/wchar_t.h"
 #include "src/__support/common.h"
+#include "src/__support/macros/null_check.h"
 
 namespace LIBC_NAMESPACE_DECL {
 
@@ -23,6 +24,9 @@ bool contains_char(const wchar_t *str, wchar_t target) {
 
 LLVM_LIBC_FUNCTION(const wchar_t *, wcspbrk,
                    (const wchar_t *src, const wchar_t *breakset)) {
+  LIBC_CRASH_ON_NULLPTR(src);
+  LIBC_CRASH_ON_NULLPTR(breakset);
+
   // currently O(n * m), can be further optimized to O(n + m) with a hash set
   for (int src_idx = 0; src[src_idx] != 0; src_idx++)
     if (contains_char(breakset, src[src_idx]))
diff --git a/libc/test/src/wchar/wcspbrk_test.cpp b/libc/test/src/wchar/wcspbrk_test.cpp
index f7754c0b324e9..bca9bffcf85f0 100644
--- a/libc/test/src/wchar/wcspbrk_test.cpp
+++ b/libc/test/src/wchar/wcspbrk_test.cpp
@@ -60,3 +60,13 @@ TEST(LlvmLibcWCSPBrkTest, FindsFirstInBreakset) {
   EXPECT_EQ(LIBC_NAMESPACE::wcspbrk(src, L"34"), src + 2);
   EXPECT_EQ(LIBC_NAMESPACE::wcspbrk(src, L"43"), src + 2);
 }
+
+#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
+TEST(LlvmLibcWCSPBrkTest, NullptrCrash) {
+  // Passing in a nullptr should crash the program.
+  EXPECT_DEATH([] { LIBC_NAMESPACE::wcspbrk(L"aaaaaaaaaaaaaa", nullptr); },
+               WITH_SIGNAL(-1));
+  EXPECT_DEATH([] { LIBC_NAMESPACE::wcspbrk(nullptr, L"aaaaaaaaaaaaaa"); },
+               WITH_SIGNAL(-1));
+}
+#endif // LIBC_HAS_ADDRESS_SANITIZER

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, cc: @lntue

@michaelrj-google michaelrj-google merged commit 45c2974 into llvm:main Jun 2, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants