Link to home
Start Free TrialLog in
Avatar of Christopher Schene
Christopher ScheneFlag for United States of America

asked on

typedef std::deque<BYTE> ByteVector is broken in vs2012

I have a definition of a byte vector as follows. I have verified that there is data in "rawConfigBytes" and that it is a valid memory address and contains valid data and I have also verified that bytes points to that same data.

I moved this code from VS2005 to VS2012 and it seems the iterator is broken in VS2012 and VS2012 indicates that it points to unreadable memory. Any idea why and what the fix is?  This worked fine in vs2005.

According to VS2012 it points to unreadable memory (see screen User generated imagesnapshot from Visual Studio)

typedef std::deque<BYTE> ByteVector;

CString ChipConfigData::GetConfigOutputCCode(ByteVector& rawConfigBytes,
                                             int headerBytes, int trailerBytes) const
{
 
  const ByteVector& bytes = rawConfigBytes;
   for (ByteVector::const_iterator it = bytes.begin(); it != bytes.end(); it++)
   {
   byte i = *it;
Avatar of Subrat (C++ windows/Linux)
Subrat (C++ windows/Linux)
Flag of India image

I checked it using Ultimate and works fine.
do you have issues with the iteration (does it crash when you were dereferencing the iterator?) or only with the info 'unreadable memory'?

if only the latter, i would assume that the debugger cannot show the iterator member since the deque::const_iterator is too complex because of the various template types, the default implementations and the fact that iterators are derived from template classes.  in vs2005 iterators often were simple pointers what made it easy for the debugger to evaluate the internals.

you might debug into the statement byte i = *it; to see the difference.

Sara
Avatar of Christopher Schene

ASKER

If I step into this I crash at the code below.

It seems like the broken code is this:  bytes.begin();

 #if _ITERATOR_DEBUG_LEVEL == 2
            if (_Mycont == 0
                  || this->_Myoff < _Mycont->_Myoff
                  || _Mycont->_Myoff + _Mycont->_Mysize <= this->_Myoff)
            {      // report error
            _DEBUG_ERROR("deque iterator not dereferencable");
            _SCL_SECURE_OUT_OF_RANGE;
            }
I checked it with vs2010 Enterprise:

#include <deque>

typedef std::deque<BYTE> ByteVector;

class ChipConfigData
{
public:
    CString ChipConfigData::GetConfigOutputCCode(ByteVector& rawConfigBytes,
                                              int headerBytes, int trailerBytes) const;
};

 CString ChipConfigData::GetConfigOutputCCode(ByteVector& rawConfigBytes,
                                              int headerBytes, int trailerBytes) const
 {
  
    CString str;
    const ByteVector& bytes = rawConfigBytes;
    for (ByteVector::const_iterator it = bytes.begin(); it != bytes.end(); it++)
    {
       byte i = *it;
       str += (TCHAR)i;
    }
    return str;
 }

Open in new window


and called it from a test class:

// CTestDlgView construction/destruction

CTestDlgView::CTestDlgView()
	: CFormView(CTestDlgView::IDD)
    , m_num(0)
{
	// TODO: add construction code here
    ChipConfigData ccd;
    ByteVector rd;
    rd.push_back(33);
    rd.push_back(34);
    rd.push_back(48);
    rd.push_back(49);
    rd.push_back(65);
    rd.push_back(66);
    CString s = ccd.GetConfigOutputCCode(rd, 0, 0);
    OutputDebugString(s);
    OutputDebugString("\n");
}

Open in new window


it worked fine. the debugger showed all 6 element nodes of 'bytes' without issues.

can you show the code where the rawConfigBytes was defined and filled with data? what happens if you iterate the rawConfigBytes with a non-const iterator?

Sara
Can you provide the complete code. Bcz I am also not getting error and i believe code is correct. So without looking into your code , it is difficult to tell the resolution.

If not able to share code, please check, are you doing any delete operation withing the loop and then continuing the loop. Just my guess!!!
SOLUTION
Avatar of sarabande
sarabande
Flag of Luxembourg image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of phoffric
phoffric

For the best resolution (and normal debugging practice), write the smallest driver that can reproduce the problem. Then, if it is still unclear why you have this problem, post the complete minimal program.
Hmmmm. This is indeed puzzling. Thanks for the responses.

I tried a minimal piece of code to test this and it seems to be OK.  I am still trying to figure why mine in an issue

static void testDebug() {
ByteVector test1;
test1.push_back(1);
test1.push_back(2);
 const ByteVector& bytes = test1;
 ByteVector::const_iterator it1 = bytes.begin();
 
}
I extended this a little and tried this code and it works fine.

The memory area that I am having a problem with is  part of a standard class that has not been deallocated: my only guess at this point is that something has over wriiten the memory. As a diagnostic test I am going to move it to a different area of memory.

static void testDebug() {
ByteVector test1;
test1.push_back(1);
test1.push_back(2);
 const ByteVector& bytes = test1;

  for (ByteVector::const_iterator it = bytes.begin(); it != bytes.end(); it++)
   {
      // Output the byte
      BYTE byte = *it;
      BYTE byte1 = *it;
   }

}
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
>> This worked fine in vs2005.
The possible problem with the vs2005 code is that it may have erroneous code in it. This means code with potential side effects that has been hammered to work flawlessly for years, but now does not. For example, if a stray pointer is pointing to the middle of an array which doesn't happen to be used at the moment, then even if the pointer operations goes out of bounds, it may be clobbering other variables which also may not have been used at the moment, or whose values are not longer going to be used.

When you upgrade, the memory layouts are likely to become different. If you were using a flag that was not set and happened to have garbage in it, then that flag is TRUE, but with another compiler, the garbage value may happen to be zero - another example of erroneous code.
Hi  phoffric

regarding your comment D: 41709495: I absolutely agree.  90% of the maintenance and work in upgrading this code from its original VS6 implementation have been issues related

(1) Uninitialized memory where by some stroke of luck either the "fall though error" resulted in the right result or the memory as somehow set to zero by default
(2) The other area where I have spent most of my time was related to "new" features related to the templates, COM and the ATL (Automation Template Library) that Microsoft later changed or "tweaked" in ways that invalidated the code in very, very subtle ways.

Having worked on software since 1977 and actually worked as the first group to use the UNIX Programmers' Work Bench at Bell laboratories we interacted with the team working on UNIX all the time. One take-away lesson is that I do not like using new language or IDE features until they have been flushed out over a few iterations.

The tool I am working on is a CAD tool that we wrote in the year 2000-01 using Visual Studio 6.0.      

One way I dealt with the C++ memory issues is that for every  class I designed and implemented I add an "ID" code that is some unique value we are not likely to see accidentally (0, ff, etc) and then have a #ifdef CHECKIDON that will validate the ID in every method for the class. This is not so much an issue in .net and Java but in C++ and C it is.
I used the suggestion from 2016-07-13 at 22:28:28ID: 41709493
and implemented in the initinstance  code the following statement to enable heap verification

afxMemDF |= checkAlwaysMemDF;

An application startup and file load that previously took around 2.5 minutes took 8 hours to complete.

By, Alas, it never identified a memory error and my code crashed in same location it did before. So the memory check did not help me.

I am thinking of writing my own  ByteVector class that does not use the template library.
ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Sara,

Your test seems like a good idea. I will try it.
I actually used this code for the test and I invoke it like this

getByteVector(__LINE__,__FILE__,m_oOutputData);

    void getByteVector(int line, CString filename, const ByteVector &myByteVector);
    void getByteVector(int line, CString filename, const ByteVector &myByteVector) {
    if (myByteVector.empty() == false)
        {
        const ByteVector& bytes = myByteVector;
        char buf[200];
        ByteVector::const_iterator it = bytes.begin();
        BYTE byte = *it;
        std::string s(1, (char)byte);
        //sprintf(buf, "ok at %s(%d)");
        sprintf(buf, "getByteVector: %s(%d)\n",filename,line);
        s += buf;
        OutputDebugString(s.c_str());
        }
    }      

Yes---I know sprintf ios old school: being lazy!
I have tracked this down and I think I am getting close to an answer. It seems that the map pointer is null in the case where I see a failure. This snapshot shows a correctly structured ByteVector.

User generated image
And here is one that fails

User generated image
Still zeroing in on the issue. very, very tedious but i am getting close.
I believe I have tracked this down to a locally defined variable that has been destructed but is still being used.
a locally defined variable that has been destructed but is still being used

you could avoid such errors by setting all pointers deleted to NULL. this might not help to avoid crash but surely is easier to debug.

second thing is to not using pointers to local variables or return local variables by reference. always return by value or by output argument (by reference). don't return pointers beside in 'create' functions. otherwise all pointers allocated with new should be deleted in the same function where they were allocated (of course that doesn't apply for self-written container classes).

Sara
I don't have an answer yet but I think you experts have given me sufficient information that I am on the right track. I think I have enough to award points.

 I only work on this job part-time (I also have a full-time job)  so the majority of the time I can work on this is on the weekends. So it may appear that I am abandoning the question but I am still working it. This is probably the longest time I have ever spent tracking a bug down.

This is not my code: it was written 15 years ago by a new grad and I'll bet he forgot all about it
I write so much code that I forget about what I've done after a few weeks.
Good luck!
Ok experts: I have my answer as to what is causing this problem. It turns out that Sara was correct".

Sara's comment:
"i would guess that the rawConfigBytes is invalid as well. that could happen if you used a pointer to a local ByteVector which had run out of scope."

I am going to award points but want to think about how to allocate them as all the expert comments helped me to some degree.



 if (data.size() > 0)
         {
         
            OutputBlock block;
            TRACE("File = %s(%d), block.id = %d\n",__FILE__,__LINE__,block.getId());
           
            block.InitializeTransition(
               subState->GetDisplayName(),
               configBytes,
               subState->GetState()->GetAddress1(),
               &differences);

            transitionBlocks.Add(block);
         }

and then later on it references the "block": that was defined here but it gets deallocated when it goes out of scope and so do all its contained classes. It is a pretty complicated class and it contains another complicated class.

I am just going to change this to pointers

 if (data.size() > 0)
         {
         
            OutputBlock *pBlock = new OutputBlock();
            TRACE("File = %s(%d), block.id = %d\n",__FILE__,__LINE__,pBlock.getId());
           
            block.InitializeTransition(
               subState->GetDisplayName(),
               configBytes,
               subState->GetState()->GetAddress1(),
               &differences);

            transitionBlocks.Add(pBlock);
         }