From 963b2c9b7ab8643abbb5391ccc089fc4f299a4d3 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 8 Sep 2026 15:34:43 +0900 Subject: [PATCH 1/3] Avoid null memcpy arguments for empty byte strings Skip empty copies in ByteString::SetSmall so default-constructed string views do not trigger nonnull-attribute diagnostics from libc memcpy declarations. Checking at the copy also covers borrowed and external string construction. Add a regression test using the existing heap and arena allocator cases. C2y permits zero-length operations on null pointers, but existing libc headers can still annotate memcpy arguments as unconditionally nonnull. Signed-off-by: Anuraag Agrawal --- common/internal/byte_string.cc | 5 ++++- common/internal/byte_string_test.cc | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/common/internal/byte_string.cc b/common/internal/byte_string.cc index d35c0efa2..d44cc16ca 100644 --- a/common/internal/byte_string.cc +++ b/common/internal/byte_string.cc @@ -946,7 +946,10 @@ void ByteString::SetSmall(google::protobuf::Arena* absl_nullable arena, rep_.header.kind = ByteStringKind::kSmall; rep_.small.size = string.size(); rep_.small.arena = arena; - std::memcpy(rep_.small.data, string.data(), rep_.small.size); + // Some libc declarations require non-null pointers even for zero-byte copies. + if (!string.empty()) { + std::memcpy(rep_.small.data, string.data(), rep_.small.size); + } } void ByteString::SetSmall(google::protobuf::Arena* absl_nullable arena, diff --git a/common/internal/byte_string_test.cc b/common/internal/byte_string_test.cc index 902a2e877..c4ae0eae6 100644 --- a/common/internal/byte_string_test.cc +++ b/common/internal/byte_string_test.cc @@ -124,6 +124,12 @@ TEST_P(ByteStringTest, Default) { EXPECT_EQ(GetKind(byte_string), ByteStringKind::kSmall); } +TEST_P(ByteStringTest, ConstructNullDataStringView) { + ByteString byte_string(GetAllocator(), absl::string_view()); + EXPECT_THAT(byte_string, IsEmpty()); + EXPECT_EQ(byte_string.GetArena(), GetAllocator().arena()); +} + TEST_P(ByteStringTest, ConstructSmallCString) { ByteString byte_string = ByteString(GetAllocator(), GetSmallString().c_str()); EXPECT_THAT(byte_string, SizeIs(GetSmallStringView().size())); From a87f547d50d4bc057950e81998760339c220b903 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 8 Sep 2026 15:55:22 +0900 Subject: [PATCH 2/3] Check for empty qualifiers without a signed comparison Use the non-empty precondition directly instead of comparing size_t with an int literal through Abseil's templated comparison helper. This avoids -Wsign-compare in debug builds without narrowing the qualifier count. Signed-off-by: Anuraag Agrawal --- common/value.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/value.h b/common/value.h index 34b4714a7..c2ca1c608 100644 --- a/common/value.h +++ b/common/value.h @@ -2802,7 +2802,7 @@ absl::StatusOr> StructValueMixin::Qualify( const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena) const { - ABSL_DCHECK_GT(qualifiers.size(), 0); + ABSL_DCHECK(!qualifiers.empty()); ABSL_DCHECK(descriptor_pool != nullptr); ABSL_DCHECK(message_factory != nullptr); ABSL_DCHECK(arena != nullptr); From aa480020cd9e262d3b413593af858727ac495731 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 8 Sep 2026 16:04:54 +0900 Subject: [PATCH 3/3] Cleanup --- common/internal/byte_string.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/common/internal/byte_string.cc b/common/internal/byte_string.cc index d44cc16ca..201842a10 100644 --- a/common/internal/byte_string.cc +++ b/common/internal/byte_string.cc @@ -946,7 +946,6 @@ void ByteString::SetSmall(google::protobuf::Arena* absl_nullable arena, rep_.header.kind = ByteStringKind::kSmall; rep_.small.size = string.size(); rep_.small.arena = arena; - // Some libc declarations require non-null pointers even for zero-byte copies. if (!string.empty()) { std::memcpy(rep_.small.data, string.data(), rep_.small.size); }