Eliminating an odd bug

[AvDav]

Hello gurus,
I'm trying to build a project I have written a couple of decades ago under Visual Studio 6 environment, and currently trying to adopt it for a new dev. env., and I'm having crash related to access to a NULL iterator. Actually, I have the following code:

struct SomeStruct
{
.
.
.
};
std::vector<SomeStruct> BracketExpressionArray;

While iterating through the container the program accesses to a not valid memory, NULL iterator, which causes the fault.
So, in order to avoid this, I need to check somehow the iterator, could you please help me to overcome this issue?
Thanks in advance.

[Marc G]

Based on that piece of code, there is not much we can help you with.
That piece of code is perfectly fine.
Maybe there is some dynamic memory happening inside the struct?
Maybe that's not properly handled when you are copying instances of SomeStruct.
A vector might perform copies of its elements.

Did you try to run it in a debugger to step through the execution?

[AvDav]

I have already checked that the underlying memory of iterator at the crash point is NULL, using debugger, now I'm trying to overcome this issue, asking a solution how can I check that the iterator object is pointing to a valid writable portion of my vector container? Is it possible?

[Marc G]

How are you iterating? Are you maybe iterating one element too far in your container?
Can you post your iteration loop?

[AvDav]

void FParser::MakeExpression(Token* begin, Token* end, BracketExpArray& bracks) {
	Operation op;
	BracketExp tmp;
	BracketExpArray::iterator i;

	tmp.Nat = false;
	tmp.iOpBrack = begin->index;
	tmp.iCloseBrack = end->index;
	

	for(int priority = 0; priority < 3; priority++) {
		for(Token *iter = begin; iter < end; iter++) {
			if((priority == 0 && (iter->COP == POW)) ||
				(priority == 1 && (iter->COP == MUL   || iter->COP == DIV)) || 
				(priority == 2 && (iter->COP == ADD   || iter->COP == SUB))) {
				if(iter->Nat) continue;

				bool doBrake1 = false;
				bool doBrake2 = false;
				bool takeLeftOperand = true;
				bool takeRightOperand = true;
  

				if(!m_opers.empty()) {
					for(OperArray::iterator it = m_opers.end() - 1; it >= m_opers.begin(); it--) {
						if(doBrake1 && doBrake2) break;
						Token* iter1 = iter - 1;

						while(iter1 > begin && !doBrake1 && iter1->Nat) {
							if(!iter1->isCloseBrack && !iter1->isOpenBrack && iter1->COP == ERROR) {
								iter1--;
								continue;
							}
							if(iter1->isCloseBrack) {
								bool doInsert = false;

								for(i = bracks.end() - 1; i >= bracks.begin(); i--)
									if(i->iCloseBrack == iter1->index) {
										if(!i->Nat)
										doInsert = true;
										break;
									}
								if(doInsert) {
									op.leftIndex = i->index;
									i->Nat = true;
									takeLeftOperand = false;
									doBrake1 = true;
									break;
								}
								else {
									while(iter1-- > begin && (iter1->index != i->iOpBrack));
									iter1--;
									if(iter1 == begin) {
										doBrake1 = true;
										break;
									}
									continue;
								}
							}
							else if(iter1->index==it->Index) {
								 op.leftIndex = it->Index;
								 takeLeftOperand = false;
								 doBrake1 = true;
								 break;
							}
							iter1--;
						}

						iter1 = iter + 1;

						while(iter1 < end && !doBrake2 && iter1->Nat) {
							if(!iter1->isCloseBrack && !iter1->isOpenBrack && iter1->COP == ERROR) {
								iter1++;
								continue;
							}
							if(iter1->isOpenBrack) {
								bool doInsert = false;

								for(BracketExpArray::iterator ii = bracks.end() - 1; ii >= bracks.begin(); ii--)
									if(ii->iOpBrack == iter1->index) {
										if(!ii->Nat) doInsert = true; 
										break;
									}
								if(doInsert) {
									op.rightIndex = i->index; // here comes a crash while accessing the i iterator.
									i->Nat = true;
									takeRightOperand = false;
									doBrake2 = true;
									break;
								}
								else {
									while(iter1++ < end && (iter1->index != i->iCloseBrack));
									iter1--;
									if(iter1 == end) {
										doBrake2 = true;
										break;
									}
									continue;
								}
							}
							else if(iter1->index==it->Index) {
								op.rightIndex = it->Index;
								takeRightOperand = false;
								doBrake2 = true;
								break;
							}
							iter1++;
						}
					}
				}
  
				tmp.index = (op.Index = iter->index);
				op.COP = iter->COP;
				op.resultNeg = iter->resultNeg;
				iter->Nat = true;

				if(takeLeftOperand) {
					op.left = (iter - 1)->argument;
					op.leftIndex = -1;
				}

				if(takeRightOperand) {
					op.right = (iter + 1)->argument;
					op.rightIndex = -1;
				}

				op.isFunc = false;
				m_opers.push_back(op);
			}
		}    
	}
	if(begin->isFunc) {
		Operation* fnc = &m_opers.back();
		fnc->isFunc = true;
		fnc->fnc = begin->fnc;
	}
	bracks.push_back(tmp);
}

[AvDav]

In fact, I decided not to use the project with this obsolete code, as lots of things have been changed during the years of existence of development environments, it's better to compile them in a native dev env. And rewritten the project according to the new specs.

[S_M_A]

Oh my... frankly, I think this code should be completely re-written. It's terribly hard to follow.
Anyway, I'm guessing a bit here but it seem like this is a regular 'cut & paste but forgot to change everything' bug
To me it seems like i should be ii where the crash occur?

[AvDav]

To me it seems like i should be ii where the crash occur?

I have the same conclusion, but making that correction leads to another unsolvable crash, that's why the best way to overcome is to leave it under 6-th Studio, and start learning the new C++ development topics.
Have a nice weekend,
Cheers.

[AvDav]

The bug has been successfully eliminated using VS 6 environment.
Thanks for supporting me.

[VictorN]

The bug has been successfully eliminated using VS 6 environment.
Thanks for supporting me.

And what and where was the bug?
And how did you fix it?

[AvDav]

As I already mentioned in the code above, the crash occurs when accessing vector's element with i iterator, which is pointing to a NULL memory at the run-time.
I just replaced the following conditional code

if(doInsert) {
	op.rightIndex = i->index; // here comes a crash while accessing the i iterator.
	i->Nat = true;
	takeRightOperand = false;
        doBrake2 = true;
	break;
}

To this one:

if(doInsert && i) {
	op.rightIndex = i->index; 
	i->Nat = true;
	takeRightOperand = false;
        doBrake2 = true;
	break;
}

But then again, I need to re-write my application for conformity of new codding standards of VS 2017 environment.
That's it.

[2kaud]

This is just bad c++ code - even for c++98! I suspect the issue is that it's possible to reach that section of code without i having been set and hence has its initial value from its definition.

[AvDav]

This portion of the project I have worked on during my intention of starting career in local branch U.S. based company, Heuristic Physics Laboratory.
They've rejected my candidacy, stating that the projct did not passed the initial requirements. I was learning C programming language basics and then switched to C++.

[2kaud]

if (doInsert && i) {

I'm not convinced that'll compile/is correct for later versions of MSVC as it involves casting i (which is an instantiation of class iterator and so has a default) to bool (which isn't part of the iterator standard but I think was allowed in VC6). Without a re-write I would look at something like this:

BracketExpArray::iterator i, iInit;
....
if (doInsert && i != iInit) {

So rather than casting i to a bool, compare it to the default init value for class iterator. Obviously, if this is going to re-written then these sort of 'tricks' wouldn't be needed.