Skip to content

Commit 91a50d5

Browse files
sbenzaquencopybara-github
authored andcommitted
Fix memoization key collision for function pointers.
PiperOrigin-RevId: 941306300
1 parent 050bd07 commit 91a50d5

2 files changed

Lines changed: 39 additions & 5 deletions

File tree

src/google/protobuf/descriptor.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2727,11 +2727,17 @@ class PROTOBUF_EXPORT DescriptorPool {
27272727
static const auto& MemoizeProjection(const Desc* descriptor, Func func) {
27282728
using ResultT = std::decay_t<decltype(func(descriptor))>;
27292729
auto* pool = GetPool(descriptor);
2730-
static_assert(std::is_empty_v<Func> ||
2731-
std::is_function_v<std::remove_pointer_t<Func>>);
2732-
// This static bool is unique per-Func, so its address can be used as a key.
2733-
static bool type_key;
2734-
auto key = std::pair<const void*, const void*>(descriptor, &type_key);
2730+
const void* secondary_key;
2731+
if constexpr (std::is_function_v<std::remove_pointer_t<Func>>) {
2732+
secondary_key = reinterpret_cast<const void*>(func);
2733+
} else if constexpr (std::is_empty_v<Func>) {
2734+
static bool type_key;
2735+
secondary_key = &type_key;
2736+
} else {
2737+
static_assert(sizeof(Func) == 0,
2738+
"Func must be an empty functor or a function pointer.");
2739+
}
2740+
auto key = std::pair<const void*, const void*>(descriptor, secondary_key);
27352741
{
27362742
absl::ReaderMutexLock lock(&pool->field_memo_table_mutex_);
27372743
auto it = pool->field_memo_table_->find(key);

src/google/protobuf/descriptor_unittest.cc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14375,6 +14375,34 @@ TEST_F(DescriptorPoolMemoizationTest, SupportsDifferentDescriptorTypes) {
1437514375
EXPECT_EQ(counter, 3);
1437614376
}
1437714377

14378+
// We define two helper functions with the exact same type signature
14379+
// (absl::string_view(*)(const Descriptor*)) but different addresses.
14380+
// We use them to test that MemoizeProjection does not cause key collisions
14381+
// in the cache when passed different function pointers of the identical type.
14382+
namespace {
14383+
absl::string_view FunctionPointer1(const Descriptor* desc) {
14384+
return desc->name();
14385+
}
14386+
14387+
absl::string_view FunctionPointer2(const Descriptor* desc) {
14388+
return desc->full_name();
14389+
}
14390+
} // namespace
14391+
14392+
TEST_F(DescriptorPoolMemoizationTest, MemoizeProjectionFunctionPointers) {
14393+
const Descriptor* descriptor = proto2_unittest::TestAllTypes::descriptor();
14394+
14395+
const absl::string_view& res1 =
14396+
DescriptorPoolMemoizationTest::MemoizeProjection(descriptor,
14397+
FunctionPointer1);
14398+
const absl::string_view& res2 =
14399+
DescriptorPoolMemoizationTest::MemoizeProjection(descriptor,
14400+
FunctionPointer2);
14401+
14402+
EXPECT_EQ(res1, "TestAllTypes");
14403+
EXPECT_EQ(res2, "proto2_unittest.TestAllTypes");
14404+
}
14405+
1437814406
TEST_F(DescriptorPoolMemoizationTest, MemoizeProjectionMultithreaded) {
1437914407
auto name_lambda = [](const FieldDescriptor* field) {
1438014408
return field->full_name();

0 commit comments

Comments
 (0)