views:

189

answers:

4

So I'm making a snake game with teleports and the usual mice. I had a loop running like this:

while(snake.alive() && miceEaten < micePerLevel)
{
    displayInfo(lives, score, level, micePerLevel - miceEaten);
    //some code
    if(miceEaten())
    {
        //update score...
    }
    //more stuff...
}

The problem with the above code was that displayInfo gets called before the score gets updated, and so after eating a mouse, the user has to wait until the loop runs again to see his score updated. So I moved that one line of code to the bottom of the function:

while(snake.alive() && miceEaten < micePerLevel)
{
    //some code
    if(miceEaten())
    {
        //update score...
    }
    //more stuff...
    displayInfo(lives, score, level, micePerLevel - miceEaten);
}

and teleports stop working! The program crashes whenever the snake reaches a teleport. And displayInfo uses the following code:

stringstream s;
s << "LEVEL " << left << setw(12) << level << "LIVES: " << setw(12) << lives << "MICE LEFT: " << setw(12) << miceLeft
    << "SCORE: " << setw(13) << score;
printLine(0, s.str(), WHITEONBLUE);

Where printLine only has a color_set, mvprintw, and refresh(). Nothing to do with Teleports. Weird.

So I went to the snake function where the snake gets its next location from a teleport:

    body.push_back(teleports[overlap(next)]->teleportFrom(dir)); //next is a Location object

Where teleports[overlap(next)]->teleportFrom(dir) returns the location the snake is to be teleported to. In an attempt to see why it was crashing (perhaps Teleport was returning some location offscreen?), I added the following 3 lines before the above line:

    Location l = teleports[overlap(next)]->teleportFrom(dir);
    mvprintw(1, 0, "(%i, %i)", l.x, l.y);
    refresh();

And the problem disappears!

Not only that, but I HAVE to have those three lines. If I comment out mvprintw(1, 0, "(%i, %i)", l.x, l.y);, or refresh();, or both, the program crashes as before upon reaching a teleport.

Any ideas on what might be causing this behavior?

UPDATE: I tried removing all warnings (which were mostly warnings about comparisons of signed/unsigned numbers), but only 1 remains so far:

warning: reference to local variable 'other' returned

And the code:

Location& Location::operator = (Location other)
{
    if(this == &other)
        return other;
    x = other.x;
    y = other.y;
    return *this;
}

What do I do to fix this warning?

+3  A: 

Have you checked your code that is causing this anomaly? The Heisenbug phenomena quoted here:

One common example is a bug that occurs in a program that was compiled with an optimizing compiler, but not in the same program when compiled without optimization (e.g., for generating a debug-mode version)

Here are a few guidelines:

  • Race condition? are you using threads?
  • Pointer overflow boundary somewhere?
  • Run your code through valgrind to monitor for any unusual/erratic changes in memory buffers somewhere

Another quote:

One common reason for heisenbug-like behaviour is that executing a program in debug mode often cleans memory before the program starts, and forces variables onto stack locations, instead of keeping them in registers. These differences in execution can alter the effect of bugs involving out-of-bounds member access or incorrect assumptions about the initial contents of memory. Another reason is that debuggers commonly provide watches or other user interfaces that cause additional code (such as property accessors) to be executed, which can, in turn, change the state of the program. Yet another reason is a fandango on core, the effect of a pointer running out of bounds. In C++, many heisenbugs are caused by uninitialized variables.

Ensure that the switches are turned off - no optimization, full debug information, clear any existing builds, restart the IDE and recompile again....

tommieb75
+1 for valgrind. (Beat me to it too.)
Donal Fellows
No threads--I haven't learned to use those yet. Will check out valgrind, but how will I know if there are any unusual changes? And what's a pointer overflow? I have enabled all warnings on the compiler and do not see any optimization boxes checked. Thanks!
wrongusername
@wrongusername: what compiler are you using?
tommieb75
@tommieb75: GNU GCC compiler
wrongusername
+4  A: 
Location& Location::operator = (Location other)
{
    if(this == &other)
        return other;
    x = other.x;
    y = other.y;
    return *this;
}

This returns a reference. When the function returns, what happens to other? (It dies, and you're referring to nothing.) Since this is the class you're dealing with around the problem area, this is probably the cause. Re-arranging surrounding code leaves the stack in a certain condition where referring to the dead variable "works".

Change it to return *this, or just remove the check altogether. (Assigning two variables without a branch will probably always run faster than adding a branch, on a modern CPU.)

(You should also generally take the parameter by reference, instead of by-value.)

GMan
Chris Dodd
@Chris: Ah, good point.
GMan
So the problem was the OP's assignment operator was returning a reference to the local variable (call by value parameter). Is that correct?
Chubsdad
GMan
+5  A: 

Build your assignment operator like this:
You should always return *this (even if they were equal). But they never would since you were creating a local copy (so this was not your error).

Location& Location::operator = (Location const& other)
{
    // Does it really matter if you assign to self?
    x = other.x;
    y = other.y;
    return *this;
}

The standard copy and swap seemed a bit overkill for such a simple class.

PS. You should fix all warnings (even if they are as simple as the unsigned mismatch). If you do not fix them you will become immune to their potency and will not spot a real problem because it is surrounded by warning that you are ignoring. So fix them all (aI always turn on the flag that makes the compiler treat all warnings as errors so that the code does not compile if there are any warnings).

The correct way to implement assignment operator (or the most commonly accepted good way). Is to use the copy and swap idiom:

// notice the parameter is passed by value (i.e. a copy).
// So the copy part is aromatically taken care of here.
// So now you just need tom implement the swap() part of the idiom.
Location& Location::operator = (Location other)
{
    this->swap(other);
    return *this;
}

void Location::swap(Location& other)
{
    std::swap(x, other.x);
    std::swap(y, other.y);
}
Martin York
Hehe, my professor wrote that in class :) But with what you wrote, there's 0 warnings, and the code runs fine. Thanks!
wrongusername
You can't always treat all warnings as errors. Too often, there are system headers you need (especially the more obscure ones) which always produce warnings when used, despite working fine. The best way forward in such situations is to quarantine the use of the offending header to a single source file so that at least the rest of the code can be warning-free.
Donal Fellows
@wrongusername @Martin: The only time you use the copy-and-swap idiom is when you need to write the Big Three. Does `Location` actually need a custom assignment operator at all? (Since you're not managing anything, I doubt it!)
GMan
@GMan Location has no copy constructor and no destructor, so I guess this structure does not need a copy assignment operator
wrongusername
@gman Absolutely, no argument there, just being a smart arse (me) obviously failing in this situation
Martin York
So the problem was the OP's assignment operator was returning a reference to the local variable (call by value parameter). Is that correct?
Chubsdad
@Chubsdad: No if you read the comments on the main question it shows the problem was solved by fixing all the warnings ( this was just the warning that the original postercould not fix)
Martin York
@Martin: "aromatically" ? smells like a typo :-)
Matthieu M.
+1  A: 

First of all, your Location::operator= should be like this instead:

Location& Location::operator = (const Location &other)
{
    if(this == &other)
        return *this;
    x = other.x;
    y = other.y;
    return *this;
}

However, that probably doesn't explain the crash. Bad pointers on stack here don't crash on most architectures (assuming x and y are int).

Now then, this is a mandelbug, not a heisenbug. You have somebody else somewhere corrupting memory. Good luck.

Joshua