Valgrind reports Invalid Read in protobuf & dumps core during SerializationToString()

[CSuganthi]

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

What operating system (Linux, Windows, ...) and version? Linux

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

What did you do? I defined map filed in my proto. I use reflection api to update the fields of map. During a call to SerializationToString(), protobuf cores. This happens only under load and mine is single threaded application. When running under valgrind, it reports Invalid Read at protobuf librarya which i attached. https://groups.google.com/forum/#!topic/protobuf/OwOw1jEnAV4 Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Here is my proto file message Hi2IriStatsData { message PerCFStats { string cfid = 1; // The CFID uint32 ifid = 2; // The IFID string cftype = 3; // The CF TYPE (as configured in the CF MO table). string destip = 4;
string destport = 5;
uint32 total_lisco_rcvd = 10;
uint32 curr_lisco_rcvd = 11; uint32 total_iri_dlvd = 30;
uint32 curr_iri_dlvd = 31;
uint32 total_iri_dropped = 40;
uint32 curr_iri_dropped = 41;
uint32 total_calls_dlvd = 50;
uint32 curr_calls_dlvd = 51;

    int32  hi2_connections   = 90;   
    google.protobuf.Timestamp last_iri_dlvd_time = 110; 
}

message PerTargetStats
{
    string coid     = 1;   
    uint32 tid      = 2;   
    string caseid   = 3;  
    uint32 curr_iri_dlvd    = 40;  
    uint32 curr_iri_dropped = 41;  
    uint32 curr_calls_dlvd  = 50; 
}

/* These are the stats */
map<string, PerCFStats>      cf_stats  = 1;  // Key is <CFID IFID>  (i.e. cfid space and ifid)
map<string, PerTargetStats>  tgt_stats = 5;  // Key is <coid tid>  (i.e. coid space and tid)

}

What did you expect to see No cores What did you see instead? Core dump

Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs). Here is my core dump Process terminating with default action of signal 6 (SIGABRT): dumping core ==13113== at 0xC0D2207: raise (in /usr/lib64/libc-2.17.so) ==13113== by 0xC0D38F7: abort (in /usr/lib64/libc-2.17.so) ==13113== by 0xB8DD7D4: __gnu_cxx::verbose_terminate_handler() (in /usr/lib64/libstdc++.so.6.0.19) ==13113== by 0xB8DB745: ??? (in /usr/lib64/libstdc++.so.6.0.19) ==13113== by 0xB8DB772: std::terminate() (in /usr/lib64/libstdc++.so.6.0.19) ==13113== by 0xB8DC2DE: cxa_pure_virtual (in /usr/lib64/libstdc++.so.6.0.19) ==13113== by 0x54F9DE: GetCachedSize (map_type_handler.h:324) ==13113== by 0x54F9DE: google::protobuf::internal::MapEntryImpl<hi2iristats::common::Hi2IriStatsData_CfStatsEntry_DoNotUse, google::protobuf::Message, std::string, hi2iristats::common::Hi2IriStatsData_PerCFStats, (google::protobuf::internal::WireFormatLite::FieldType)9, (google::protobuf::internal::WireFormatLite::FieldType)11, 0>::GetCachedSize() const (map_entry_lite.h:257) ==13113== by 0x5441C1: InternalWriteMessageNoVirtualToArray (wire_format_lite_inl.h:979) ==13113== by 0x5441C1: hi2iristats::common::Hi2IriStatsData::InternalSerializeWithCachedSizesToArray(bool, unsigned char) const (kpi_hi2_iridata.pb.cc:1968) ==13113== by 0x73797A4: google::protobuf::MessageLite::AppendPartialToString(std::string) const (message_lite.cc:296) ==13113== by 0x5188C9: KpiData::buildKpi(std::string&) (KpiData.C:384) ==13113== by 0x50F0F3: KpiClientMgr::buildKpi(std::string&) (KpiClientMgr.C:731) ==13113== by 0x50F1E3: KpiClientMgr::buildPayload(std::string&) (KpiClientMgr.C:695)

Here is my valgrind report.

==13113== Invalid read of size 8 ==13113== at 0x54F9D6: GetCachedSize (map_type_handler.h:324) ==13113== by 0x54F9D6: google::protobuf::internal::MapEntryImpl<hi2iristats::common::Hi2IriStatsData_CfStatsEntry_DoNotUse, google::protobuf::Message, std::string, hi2iristats::common::Hi2IriStatsData_PerCFStats, (google::protobuf::internal::WireFormatLite::FieldType)9, (google::protobuf::internal::WireFormatLite::FieldType)11, 0>::GetCachedSize() const (map_entry_lite.h:257) ==13113== by 0x5441C1: InternalWriteMessageNoVirtualToArray (wire_format_lite_inl.h:979) ==13113== by 0x5441C1: hi2iristats::common::Hi2IriStatsData::InternalSerializeWithCachedSizesToArray(bool, unsigned char) const (kpi_hi2_iridata.pb.cc:1968) ==13113== by 0x73797A4: google::protobuf::MessageLite::AppendPartialToString(std::string) const (message_lite.cc:296) ==13113== by 0x5188C9: KpiData::buildKpi(std::string&) (KpiData.C:384) ==13113== by 0x50F0F3: KpiClientMgr::buildKpi(std::string&) (KpiClientMgr.C:731) ==13113== by 0x50F1E3: KpiClientMgr::buildPayload(std::string&) (KpiClientMgr.C:695) ==13113== by 0x51009A: KpiClientMgr::buildAndSendKpi() (KpiClientMgr.C:294) ==13113== by 0x511287: KpiClientMgr::publishKpi() (KpiClientMgr.C:636) ==13113== by 0x512962: KpiClientMgr::run() (KpiClientMgr.C:235) ==13113== by 0x512D78: KpiClientMgrThreadCbk (KpiClientMgr.C:47) ==13113== by 0x9A23DD4: start_thread (in /usr/lib64/libpthread-2.17.so) ==13113== by 0xC199EAC: clone (in /usr/lib64/libc-2.17.so) ==13113== Address 0xe2ddfa8 is 8 bytes inside a block of size 112 free'd ==13113== at 0x4C2B16D: operator delete(void) (vg_replace_malloc.c:576) ==13113== by 0x576B58: erase (map.h:1128) ==13113== by 0x576B58: erase (map.h:1135) ==13113== by 0x576B58: google::protobuf::Map<std::string, hi2iristats::common::Hi2IriStatsData_PerCFStats>::clear() (map.h:1138) ==13113== by 0x589596: google::protobuf::internal::MapField<hi2iristats::common::Hi2IriStatsData_CfStatsEntry_DoNotUse, std::string, hi2iristats::common::Hi2IriStatsData_PerCFStats, (google::protobuf::internal::WireFormatLite::FieldType)9, (google::protobuf::internal::WireFormatLite::FieldType)11, 0>::SyncMapWithRepeatedFieldNoLock() const (map_field_inl.h:307) ==13113== by 0x73FAE20: google::protobuf::internal::MapFieldBase::SyncMapWithRepeatedField() const (map_field.cc:116) ==13113== by 0x544177: GetMap (map_field.h:250) ==13113== by 0x544177: cf_stats (kpi_hi2_iridata.pb.h:1230) ==13113== by 0x544177: hi2iristats::common::Hi2IriStatsData::InternalSerializeWithCachedSizesToArray(bool, unsigned char) const (kpi_hi2_iridata.pb.cc:1963) ==13113== by 0x73797A4: google::protobuf::MessageLite::AppendPartialToString(std::string) const (message_lite.cc:296) ==13113== by 0x5188C9: KpiData::buildKpi(std::string&) (KpiData.C:384) ==13113== by 0x50F0F3: KpiClientMgr::buildKpi(std::string&) (KpiClientMgr.C:731) ==13113== by 0x50F1E3: KpiClientMgr::buildPayload(std::string&) (KpiClientMgr.C:695) ==13113== by 0x51009A: KpiClientMgr::buildAndSendKpi() (KpiClientMgr.C:294) ==13113== by 0x511287: KpiClientMgr::publishKpi() (KpiClientMgr.C:636) ==13113== by 0x512962: KpiClientMgr::run() (KpiClientMgr.C:235) ==13113== Block was alloc'd at ==13113== at 0x4C2A1E3: operator new(unsigned long) (vg_replace_malloc.c:334) ==13113== by 0x589399: CreateValueTypeInternal (map.h:1175) ==13113== by 0x589399: google::protobuf::Map<std::string, hi2iristats::common::Hi2IriStatsData_PerCFStats>::operator[](std::string const&) (map.h:1050) ==13113== by 0x5895DA: google::protobuf::internal::MapField<hi2iristats::common::Hi2IriStatsData_CfStatsEntry_DoNotUse, std::string, hi2iristats::common::Hi2IriStatsData_PerCFStats, (google::protobuf::internal::WireFormatLite::FieldType)9, (google::protobuf::internal::WireFormatLite::FieldType)11, 0>::SyncMapWithRepeatedFieldNoLock() const (map_field_inl.h:315) ==13113== by 0x73FAE20: google::protobuf::internal::MapFieldBase::SyncMapWithRepeatedField() const (map_field.cc:116) ==13113== by 0x54410B: GetMap (map_field.h:250) ==13113== by 0x54410B: cf_stats (kpi_hi2_iridata.pb.h:1230) ==13113== by 0x54410B: hi2iristats::common::Hi2IriStatsData::InternalSerializeWithCachedSizesToArray(bool, unsigned char) const (kpi_hi2_iridata.pb.cc:1923) ==13113== by 0x73797A4: google::protobuf::MessageLite::AppendPartialToString(std::string*) const (message_lite.cc:296) ==13113== by 0x5188C9: KpiData::buildKpi(std::string&) (KpiData.C:384) ==13113== by 0x50F0F3: KpiClientMgr::buildKpi(std::string&) (KpiClientMgr.C:731) ==13113== by 0x50F1E3: KpiClientMgr::buildPayload(std::string&) (KpiClientMgr.C:695) ==13113== by 0x51009A: KpiClientMgr::buildAndSendKpi() (KpiClientMgr.C:294) ==13113== by 0x511287: KpiClientMgr::publishKpi() (KpiClientMgr.C:636) ==13113== by 0x512962: KpiClientMgr::run() (KpiClientMgr.C:235) ==13113==

Anything else we should know about your project / environment

[acozzette]

Could you post some example code that triggers the problem? If possible it would also be nice if you could try with protobuf 3.8 and see if the crash still occurs.

[CSuganthi]

//This part of code gets field descriptor for specific key const Reflection* refl = mKpiData->GetReflection();

RepeatedPtrField<google::protobuf::Message> *mmf_afstats_msg = 
    refl->MutableRepeatedPtrField<google::protobuf::Message>(mKpiData, amapFd);

int map_size = mmf_afstats_msg->size();
bool is_exist = false;
bool ret = true;
for (int i = 0;i< map_size; i++)
{
    const google::protobuf::Message& afstats_msg = mmf_afstats_msg->Get(i);
    is_exist = findEntry(&afstats_msg, akey);

    if (is_exist)
    {
        google::protobuf::Message* value_perafstats = getValueMessageFromMap(&afstats_msg);
        const FieldDescriptor* name_desc = 
            value_perafstats->GetDescriptor()->FindFieldByName(aname);

        if (!name_desc)
        {
            InfErr_IntL2_w_Chk <<"KpiData::update_entry_into_afstatmap - Field Name not exists : " <<aname.c_str()<<end;
            return false;
        }

        if (aisIncrement)
            ret = increment_and_set(value_perafstats, name_desc, avalue);
        else 
            ret = decrement_and_set(value_perafstats, name_desc, avalue);

        if (false == ret )
        {
            InfErr_IntL2_w_Chk <<"KpiData::update_entry_into_afstatmap - \
                Increment failed for Field Name : " <<aname.c_str()<<end;
        }

        break;
    }
}
if (!is_exist)
{
    // Insert Entry
    ret = insert_map_entry(mKpiData, amapFd, akey, aname, avalue);
} 

//This part of code inserts entry to map const FieldDescriptor fd_map_key = amapFd->message_type()->FindFieldByName("key"); const FieldDescriptor fd_map_value = amapFd->message_type()->FindFieldByName("value");

// Create Entry object
Message* entry_afstats_message = amessage->GetReflection()->AddMessage(mKpiData, amapFd);

// Set key
entry_afstats_message->GetReflection()->SetString(
        entry_afstats_message,
        fd_map_key, akey.c_str());

// Create PerAFStats Object
Message* value_perafstats = entry_afstats_message->
    GetReflection()->MutableMessage(entry_afstats_message,
            fd_map_value);

const FieldDescriptor* name_desc = 
    value_perafstats->GetDescriptor()->FindFieldByName(aname);

FieldDescriptor::Type field_type = name_desc ->type(); if (field_type == FieldDescriptor::TYPE_STRING) { value_perafstats ->GetReflection()->SetString(value_perafstats , name_desc , value); } else if (field_type == FieldDescriptor::TYPE_UINT32) { uint32 t_value = 0; ret = StringToNumber(value, t_value); if (ret) value_perafstats ->GetReflection()->SetUInt32(value_perafstats , name_desc , t_value); } // i have for other data types too...

//This part of the code get the value of the field, increment & set it back.

`bool KpiData::increment_and_set(const Message aMessage, const FieldDescriptor anameDesc,const string& astepValue) { if (!aMessage || !anameDesc) return false;

const Reflection* refl = aMessage->GetReflection();

// Get Field Type from FieldDesc
FieldDescriptor::Type field_type = anameDesc->type();

// Check for Integer Data type
if (false == isTypeInt(field_type))
{
    InfErr_IntL2_w_Chk<<" KpiData::increment_and_set - Increment can't be done"<<
        " on this data type : "<<anameDesc->type_name()
        <<" for field: "<<anameDesc->name().c_str()<<end;
    return false;
}

bool ret = true;
//Get field value using Reflection APIS according to 
//field type
if (field_type == FieldDescriptor::TYPE_UINT32)
{
    uint32 t_value;
    t_value = refl->GetUInt32(*aMessage, anameDesc);

    uint32 step_value = 0;
    ret = StringToNumber<uint32>(astepValue, step_value);

    if (ret)
    {
        t_value += step_value;
        refl->SetUInt32((Message*)aMessage, anameDesc, t_value);
    }
} else if (field_type == FieldDescriptor::TYPE_UINT64)
{
    uint64 t_value;
    t_value = refl->GetUInt64(*aMessage, anameDesc);

    uint32 step_value =0;
    ret =  StringToNumber<uint32>(astepValue, step_value);
    if (ret)
    {
        t_value += step_value;
        refl->SetUInt64((Message*)aMessage, anameDesc, t_value);
    }
} else if (field_type == FieldDescriptor::TYPE_INT32)
{
    int32 t_value;
    t_value = refl->GetInt32(*aMessage, anameDesc);

    int32 step_value =0;
    ret =  StringToNumber<int32>(astepValue, step_value);
    if (ret)
    {
        t_value += step_value;
        refl->SetInt32((Message*)aMessage, anameDesc, t_value);
    }
} else if (field_type == FieldDescriptor::TYPE_INT64)
{
    int64 t_value;
    t_value = refl->GetInt64(*aMessage, anameDesc);

    int64 step_value =0;
    ret =  StringToNumber<int64>(astepValue, step_value);
    if (ret)
    {
        t_value += step_value;
        refl->SetInt64((Message*)aMessage, anameDesc, t_value);
    }
} 
return ret;

}`

// This is serialization to String string aoutputmsg; aoutputMsg.clear(); bool ret = mKpiData->SerializeToString(&aoutputMsg);

[acozzette]

@CSuganthi Thank you for posting that code. Before we dig into it, though, it would help a lot if you could narrow it down to a smaller reproducible example and also let us know if it still happens with version 3.8.

[t-rybarski]

@CSuganthi you have a bug in your code, you are using incorrect type conversion for the UINT_64 field:

` else if (field_type == FieldDescriptor::TYPE_UINT64) { uint64 t_value; t_value = refl->GetUInt64(*aMessage, anameDesc);

uint32 step_value =0;
ret =  StringToNumber<uint32>(astepValue, step_value);
if (ret)
{
    t_value += step_value;
    refl->SetUInt64((Message*)aMessage, anameDesc, t_value);
}

}`

Of course should be:

` else if (field_type == FieldDescriptor::TYPE_UINT64) { uint64 t_value; t_value = refl->GetUInt64(*aMessage, anameDesc);

uint64 step_value =0;
ret =  StringToNumber<uint64>(astepValue, step_value);
if (ret)
{
    t_value += step_value;
    refl->SetUInt64((Message*)aMessage, anameDesc, t_value);
}

}`

[harendra247]

I am using protobuf version 3.9.2 and I am also facing the same issue. Any Help would be really appreciated.

CMakeFiles/object_detection.dir/get_prediction.cpp.o: In function google::protobuf::internal::MapField<tensorflow::MetaGraphDef_SignatureDefEntry_DoNotUse, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, tensorflow::SignatureDef, (google::protobuf::internal::WireFormatLite::FieldType)9, (google::protobuf::internal::WireFormatLite::FieldType)11, 0>::GetMap() const': get_prediction.cpp:(.text._ZNK6google8protobuf8internal8MapFieldIN10tensorflow39MetaGraphDef_SignatureDefEntry_DoNotUseENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS3_12SignatureDefELNS1_14WireFormatLite9FieldTypeE9ELSD_11ELi0EE6GetMapEv[_ZNK6google8protobuf8internal8MapFieldIN10tensorflow39MetaGraphDef_SignatureDefEntry_DoNotUseENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS3_12SignatureDefELNS1_14WireFormatLite9FieldTypeE9ELSD_11ELi0EE6GetMapEv]+0x10): undefined reference togoogle::protobuf::internal::MapFieldBase::SyncMapWithRepeatedField() const' collect2: error: ld returned 1 exit status

[fowles]

There error you have posted is a link error which is not at all a valgrind crash.

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.