Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++: Valgrind: map.InitializationErrorString(): jump or move depends on uninitialised value #20024

Open
chrisse74 opened this issue Jan 17, 2025 · 2 comments

Comments

@chrisse74
Copy link
Contributor

Calling InitializationErrorString() on a Protobuf map leads to Valgrind reporting a 'jump or move depends on uninitialised value'.

Note: We get the same error when calling DebugString() instead of InitializationErrorString().

What version of protobuf and what language are you using?
Version: v29.2
Language: C++

What operating system (Linux, Windows, ...) and version?
Red Hat Enterprise Linux 9.5

What runtime / compiler are you using (e.g., python version or gcc version)
clang 19.1.6

What did you do?
Steps to reproduce the behavior:
We added debug symbols to the cmake release build to come up with the corresponding line numbers.

Sample Proto File:

syntax = "proto3";

option java_multiple_files = true;

package my.idl;

// This is a Map type documentation for StringMap
message StringMap
{
    map<string,string> string_map = 1;
}

Sample source file:

#include "MyTypesProto.grpc.pb.h"

int main(void)
{
    my::idl::StringMap stringMap;
    const std::string stringValue("test1234");
    stringMap.mutable_string_map()->insert({ stringValue, stringValue });

    stringMap.InitializationErrorString();

    google::protobuf::ShutdownProtobufLibrary();
}

What did you expect to see
No errors detected

What did you see instead?
See

Likely candidate is the is_repeated() call which is reported first in

if (proto.has_default_value() && result->is_repeated()) {

==1000== Conditional jump or move depends on uninitialised value(s)
==1000==    at 0x1BC369: google::protobuf::DescriptorBuilder::BuildFieldOrExtension(google::protobuf::FieldDescriptorProto const&, google::protobuf::Descriptor*, google::protobuf::FieldDescriptor*, bool, google::protobuf::internal::FlatAllocator&) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:6646)
==1000==    by 0x1B6D0E: BuildField (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:4420)
==1000==    by 0x1B6D0E: google::protobuf::DescriptorBuilder::BuildMessage(google::protobuf::DescriptorProto const&, google::protobuf::Descriptor const*, google::protobuf::Descriptor*, google::protobuf::internal::FlatAllocator&) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:6349)
==1000==    by 0x1B51CB: google::protobuf::DescriptorBuilder::BuildFileImpl(google::protobuf::FileDescriptorProto const&, google::protobuf::internal::FlatAllocator&) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:6129)
==1000==    by 0x1AFBF8: google::protobuf::DescriptorBuilder::BuildFile(google::protobuf::FileDescriptorProto const&) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:5893)
==1000==    by 0x1A585D: operator() (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:4750)
==1000==    by 0x1A585D: google::protobuf::DescriptorPool::BuildFileFromDatabase(google::protobuf::FileDescriptorProto const&, google::protobuf::DescriptorPool::DeferredValidation&) const (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:4755)
==1000==    by 0x1A3CC1: google::protobuf::DescriptorPool::TryFindFileInFallbackDatabase(std::__1::basic_string_view<char, std::__1::char_traits<char> >, google::protobuf::DescriptorPool::DeferredValidation&) const (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:2717)
==1000==    by 0x1A3BC5: google::protobuf::DescriptorPool::FindFileByName(std::__1::basic_string_view<char, std::__1::char_traits<char> >) const (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:2274)
==1000==    by 0x23520D: google::protobuf::(anonymous namespace)::AssignDescriptorsImpl(google::protobuf::internal::DescriptorTable const*, bool) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/generated_message_reflection.cc:3666)
==1000==    by 0x275DCB: operator() (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:168)
==1000==    by 0x275DCB: __invoke<(lambda at grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167:35)> (sharecompiler.HEAD/linux/clang19el9/bin/../include/c++/v1/__type_traits/invoke.h:149)
==1000==    by 0x275DCB: invoke<(lambda at grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167:35)> (sharecompiler.HEAD/linux/clang19el9/bin/../include/c++/v1/__functional/invoke.h:28)
==1000==    by 0x275DCB: void absl::lts_20240722::base_internal::CallOnceImpl<google::protobuf::Message::GetMetadataImpl(google::protobuf::internal::ClassDataFull const&)::$_0>(std::__1::atomic<unsigned int>*, absl::lts_20240722::base_internal::SchedulingMode, google::protobuf::Message::GetMetadataImpl(google::protobuf::internal::ClassDataFull const&)::$_0&&) (grpc-1.69.0/third_party/abseil-cpp/absl/base/call_once.h:182)
==1000==    by 0x2740EF: call_once<(lambda at grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167:35)> (grpc-1.69.0/third_party/abseil-cpp/absl/base/call_once.h:216)
==1000==    by 0x2740EF: google::protobuf::Message::GetMetadataImpl(google::protobuf::internal::ClassDataFull const&) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167)
==1000==    by 0x1A391E: GetDescriptor (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.pb.h:10890)
==1000==    by 0x1A391E: descriptor (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.pb.h:10887)
==1000==    by 0x1A391E: google::protobuf::DescriptorPool::generated_pool() (grpc-1.69.0/third_party/protobuf/src/google/protobuf/descriptor.cc:2217)
==1000==    by 0x235126: MaybeInitializeLazyDescriptors (grpc-1.69.0/third_party/protobuf/src/google/protobuf/generated_message_reflection.cc:3693)
==1000==    by 0x235126: google::protobuf::internal::AssignDescriptorsOnceInnerCall(google::protobuf::internal::DescriptorTable const*) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/generated_message_reflection.cc:3731)
==1000==    by 0x275DCB: operator() (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:168)
==1000==    by 0x275DCB: __invoke<(lambda at grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167:35)> (sharecompiler.HEAD/linux/clang19el9/bin/../include/c++/v1/__type_traits/invoke.h:149)
==1000==    by 0x275DCB: invoke<(lambda at grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167:35)> (sharecompiler.HEAD/linux/clang19el9/bin/../include/c++/v1/__functional/invoke.h:28)
==1000==    by 0x275DCB: void absl::lts_20240722::base_internal::CallOnceImpl<google::protobuf::Message::GetMetadataImpl(google::protobuf::internal::ClassDataFull const&)::$_0>(std::__1::atomic<unsigned int>*, absl::lts_20240722::base_internal::SchedulingMode, google::protobuf::Message::GetMetadataImpl(google::protobuf::internal::ClassDataFull const&)::$_0&&) (grpc-1.69.0/third_party/abseil-cpp/absl/base/call_once.h:182)
==1000==    by 0x274092: call_once<(lambda at grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167:35)> (grpc-1.69.0/third_party/abseil-cpp/absl/base/call_once.h:216)
==1000==    by 0x274092: GetMetadataImpl (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:167)
==1000==    by 0x274092: google::protobuf::Message::GetMetadata() const (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:156)
==1000==    by 0x282FE0: GetDescriptor (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.h:362)
==1000==    by 0x282FE0: google::protobuf::internal::ReflectionOps::FindInitializationErrors(google::protobuf::Message const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >*) (grpc-1.69.0/third_party/protobuf/src/google/protobuf/reflection_ops.cc:382)
==1000==    by 0x273E65: FindInitializationErrors (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:136)
==1000==    by 0x273E65: google::protobuf::Message::InitializationErrorString() const (grpc-1.69.0/third_party/protobuf/src/google/protobuf/message.cc:141)
==1000==    by 0x19BF94: main (main.cpp:9)

Anything else we should know about your project / environment
It affects us as gRPC 1.69.0 calls InitializationErrorString() in grpcpp/impl/generic_serialize.h (line 84)

@chrisse74 chrisse74 added the untriaged auto added to all issues by default when created. label Jan 17, 2025
@esrauchg
Copy link

esrauchg commented Jan 17, 2025

I believe this must be a valgrind false positive.

is_repeated() is defined as:

inline bool FieldDescriptor::is_repeated() const {
  ABSL_DCHECK_EQ(is_repeated_, label() == LABEL_REPEATED);
  return is_repeated_;
}

Both label and is_repeated_ are always assigned above this line in the same function.

I suspect this is a false positive based on is_repeated_ is a bit field (the :1 syntax on the definition site), valgrind is confused because other adjacent bit fields are uninitialized, and then the valgrind path doesn't understand that uninitialized adjacent bits are not (in the C++ semantics) part of the comparison that is happening. But, I can't immediately find a reference to valgrind having this false positive case, so we may want to double check that.

@esrauchg esrauchg removed the untriaged auto added to all issues by default when created. label Jan 17, 2025
@chrisse74
Copy link
Contributor Author

The error goes away when a libprotobuf which was compiled without optimizations is used. (O1 does trigger it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants