diff --git a/absl/strings/ascii.cc b/absl/strings/ascii.cc index 2af13a6d..a38a612a 100644 --- a/absl/strings/ascii.cc +++ b/absl/strings/ascii.cc @@ -178,9 +178,10 @@ constexpr bool AsciiInAZRange(unsigned char c) { } // Force-inline so the compiler won't merge the short and long implementations. +// `src` may be null iff `size` is zero. template ABSL_ATTRIBUTE_ALWAYS_INLINE inline constexpr void AsciiStrCaseFoldImpl( - absl::Nonnull dst, absl::Nonnull src, size_t size) { + absl::Nonnull dst, absl::Nullable src, size_t size) { // The upper- and lowercase versions of ASCII characters differ by only 1 bit. // When we need to flip the case, we can xor with this bit to achieve the // desired result. Note that the choice of 'a' and 'A' here is arbitrary. We @@ -199,28 +200,30 @@ ABSL_ATTRIBUTE_ALWAYS_INLINE inline constexpr void AsciiStrCaseFoldImpl( constexpr size_t kCaseFoldThreshold = 16; // No-inline so the compiler won't merge the short and long implementations. +// `src` may be null iff `size` is zero. template ABSL_ATTRIBUTE_NOINLINE constexpr void AsciiStrCaseFoldLong( - absl::Nonnull dst, absl::Nonnull src, size_t size) { + absl::Nonnull dst, absl::Nullable src, size_t size) { ABSL_ASSUME(size >= kCaseFoldThreshold); AsciiStrCaseFoldImpl(dst, src, size); } // Splitting to short and long strings to allow vectorization decisions // to be made separately in the long and short cases. +// `src` may be null iff `size` is zero. template constexpr void AsciiStrCaseFold(absl::Nonnull dst, - absl::Nonnull src, size_t size) { + absl::Nullable src, size_t size) { size < kCaseFoldThreshold ? AsciiStrCaseFoldImpl(dst, src, size) : AsciiStrCaseFoldLong(dst, src, size); } -void AsciiStrToLower(absl::Nonnull dst, absl::Nonnull src, +void AsciiStrToLower(absl::Nonnull dst, absl::Nullable src, size_t n) { return AsciiStrCaseFold(dst, src, n); } -void AsciiStrToUpper(absl::Nonnull dst, absl::Nonnull src, +void AsciiStrToUpper(absl::Nonnull dst, absl::Nullable src, size_t n) { return AsciiStrCaseFold(dst, src, n); } diff --git a/absl/strings/ascii.h b/absl/strings/ascii.h index c8c40e85..44a6911e 100644 --- a/absl/strings/ascii.h +++ b/absl/strings/ascii.h @@ -76,10 +76,10 @@ ABSL_DLL extern const char kToUpper[256]; // Declaration for the array of characters to lower-case characters. ABSL_DLL extern const char kToLower[256]; -void AsciiStrToLower(absl::Nonnull dst, absl::Nonnull src, +void AsciiStrToLower(absl::Nonnull dst, absl::Nullable src, size_t n); -void AsciiStrToUpper(absl::Nonnull dst, absl::Nonnull src, +void AsciiStrToUpper(absl::Nonnull dst, absl::Nullable src, size_t n); } // namespace ascii_internal diff --git a/absl/strings/ascii_test.cc b/absl/strings/ascii_test.cc index 896ffb7d..fe1083a5 100644 --- a/absl/strings/ascii_test.cc +++ b/absl/strings/ascii_test.cc @@ -200,6 +200,10 @@ TEST(AsciiStrTo, Lower) { EXPECT_EQ("abcdefghijklmnopqrstuvwxyz1!a", absl::AsciiStrToLower(long_str)); EXPECT_EQ("pqrstu", absl::AsciiStrToLower(fun())); + // An empty `string_view` specifically exercises the case where a null data + // pointer is passed to internal functions. + EXPECT_EQ("", absl::AsciiStrToLower(absl::string_view())); + absl::AsciiStrToLower(&mutable_str); EXPECT_EQ("_`?@[{amnopqrstuvwxyz", mutable_str); @@ -223,6 +227,10 @@ TEST(AsciiStrTo, Upper) { EXPECT_EQ("ABCDEFGHIJKLMNOPQRSTUVWXYZ1!A", absl::AsciiStrToUpper(long_str)); EXPECT_EQ("PQRSTU", absl::AsciiStrToUpper(fun())); + // An empty `string_view` specifically exercises the case where a null data + // pointer is passed to internal functions. + EXPECT_EQ("", absl::AsciiStrToUpper(absl::string_view())); + char mutable_buf[] = "Mutable"; std::transform(mutable_buf, mutable_buf + strlen(mutable_buf), mutable_buf, absl::ascii_toupper); diff --git a/absl/strings/str_format_test.cc b/absl/strings/str_format_test.cc index 3c52be1e..969e1f9e 100644 --- a/absl/strings/str_format_test.cc +++ b/absl/strings/str_format_test.cc @@ -516,7 +516,13 @@ TEST_F(FormatEntryPointTest, SNPrintF) { EXPECT_EQ(result, 17); EXPECT_EQ(std::string(buffer), "NUMBER: 1234567"); - result = SNPrintF(nullptr, 0, "Just checking the %s of the output.", "size"); + // The `output` parameter is annotated nonnull, but we want to test that + // it is never written to if the size is zero. + // Use a variable instead of passing nullptr directly to avoid a `-Wnonnull` + // warning. + char* null_output = nullptr; + result = + SNPrintF(null_output, 0, "Just checking the %s of the output.", "size"); EXPECT_EQ(result, 37); } @@ -545,7 +551,13 @@ TEST_F(FormatEntryPointTest, SNPrintFWithV) { std::string size = "size"; - result = SNPrintF(nullptr, 0, "Just checking the %v of the output.", size); + // The `output` parameter is annotated nonnull, but we want to test that + // it is never written to if the size is zero. + // Use a variable instead of passing nullptr directly to avoid a `-Wnonnull` + // warning. + char* null_output = nullptr; + result = + SNPrintF(null_output, 0, "Just checking the %v of the output.", size); EXPECT_EQ(result, 37); } diff --git a/absl/strings/string_view_test.cc b/absl/strings/string_view_test.cc index e978fc3f..e810b25f 100644 --- a/absl/strings/string_view_test.cc +++ b/absl/strings/string_view_test.cc @@ -894,7 +894,11 @@ TEST(StringViewTest, NULLInput) { EXPECT_EQ(s.size(), 0u); #ifdef ABSL_HAVE_STRING_VIEW_FROM_NULLPTR - s = absl::string_view(nullptr); + // The `str` parameter is annotated nonnull, but we want to test the defensive + // null check. Use a variable instead of passing nullptr directly to avoid a + // `-Wnonnull` warning. + char* null_str = nullptr; + s = absl::string_view(null_str); EXPECT_EQ(s.data(), nullptr); EXPECT_EQ(s.size(), 0u); @@ -1076,8 +1080,21 @@ TEST(StringViewTest, ConstexprNullSafeStringView) { TEST(StringViewTest, ConstexprCompiles) { constexpr absl::string_view sp; + // With `-Wnonnull` turned on, there is no way to test the defensive null + // check in the `string_view(const char*)` constructor in a constexpr context, + // as the argument needs to be constexpr. The compiler will therefore always + // know at compile time that the argument is nullptr and complain because the + // parameter is annotated nonnull. We hence turn the warning off for this + // test. +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wnonnull" +#endif #ifdef ABSL_HAVE_STRING_VIEW_FROM_NULLPTR constexpr absl::string_view cstr(nullptr); +#endif +#if defined(__clang__) +#pragma clang diagnostic pop #endif constexpr absl::string_view cstr_len("cstr", 4);