what is causing this buffer overrun warning over 64 bit VS2012 compile environment?

Question

I have a working application compiled using VS2010 64 bit environment - while trying to migrate it to VS2012 compiler (C++11) i get a C6386 warning and can't seem to find the root cause:

here is the code snippet that yields the warning:

Packet::Packet(const Packet& oOther)
{
   m_vData.assign(oOther.m_vData.begin(),oOther.m_vData.end());
   if(NULL != oOther.m_pValueChecks)
   {
      m_pValueChecks = new set<string>*[oOther.m_vData.size()];
      for(size_t i = 0; i < oOther.m_vData.size(); ++i)
      {

        if(NULL == oOther.m_pValueChecks[i])
        {
             m_pValueChecks[i] = NULL;
        }
        else
        {
/// compiler warns on the below line
             m_pValueChecks[i] = new set<string>(oOther.m_pValueChecks[i]->begin(), oOther.m_pValueChecks[i]->end());
        }
      }
   }
}

Packet class definition:

class Packet
{
public:
    Packet();
    Packet(const Packet&);

    .....

    vector<pair<string,pair<FieldType,Field> > > m_vData;
    set<string> ** m_pValueChecks;
}

the Warning generated:

c:\<my_path_and_file>(3331): warning C6386: Buffer overrun while writing to 'm_pValueChecks': the writable size is 'oOther.m_vData.public: unsigned __int64 __cdecl std::vector<struct std::pair<string,struct std::pair<enum Packet::FieldType,class Field> >,class std::allocator<struct std::pair<string,struct std::pair<enum Packet::FieldType,class Field> > > >::size(void)const ()*8' bytes, but '16' bytes might be written.: Lines: 3312, 3313, 3314, 3315, 3316, 3317, 3318, 3319, 3320, 3322, 3323, 3325, 3331, 3323, 3325, 3331

as the compile environment is 64 bit, the oOther.m_vData.size() is of type size_t which is 64 bit unsigned integer on 64 bit environment, so the for loop is iterating correctly on the range, and m_pValueChecks contains enough allocated items to satisfy the assignment.

why the warning is generated?

Answer 1

Ok - continue digging into the scenario - i have found that the project contains /analyze switch which forces the Code Analysis option of VS2012 to always run.

when removing this switch, the above (nasty i agree) code compiles without any warning.

now, as this specific code compiles with no warning on VS2010 64 bit environment, then VS2010 vs. VS2012 code analysis rules differences are the primary suspect for the warning.

currently, i've passed this roadblock in my migration efforts, i'll try to continue understand the rules thing differences later on. if someone can shade some light on it it would be great to post it here.

EDIT: i have found out that Code Analyzer is not working on VS2010 - the compiler switch is ignored, which explains why this code is compiling. Anyhow a proper re-factor is required.

EDIT 2

Folks, after intensive thinking (almost 6 months) i finally found the root cause...

m_pValueChecks = new set<string>*[oOther.m_vData.size()];
for(size_t i = 0; i < oOther.m_vData.size(); ++i)
{

  if(NULL == oOther.m_pValueChecks[i])
  {
       m_pValueChecks[i] = NULL;
  }
  else
  {
      /// commpiler warns on the below line
      m_pValueChecks[i] = new set<string>(oOther.m_pValueChecks[i]->begin(), oOther.m_pValueChecks[i]->end());
  }
}

The reasons is because the for loop increments the i index using ++i, which means first the i variable is incremented, then being used.

This means there might be a chance the i variable will be more than oOther.m_vData.size() which is the size of the m_pValueChecks allocated array.

Changing the for floop to for(size_t i = 0; i < oOther.m_vData.size(); i++) removed the warning.

Answer 2

You seem to be mixing other.m_pValueChecks with other.m_vData in a few places. This could result in memory corruption due to mismatching array sizes.

Instead of this ghastly double-pointer approach, consider putting your sets inside another container type and let the language do all this for you, safely.