Lewis
Lewis

Reputation: 1350

Segfault when using *this

I'm getting a segfault when I try to access a member of a class from within a method of that same class, which does not make sense to me at all.

I have the Tree class:

class Tree
{

public:

Coord* root;

Tree(int x, int y)
{
    root = new Coord(x, y);
    populateTree();
}

void populateTree()
{
    queue<Coord*> nodes;
    nodes.push(root);

    while (nodes.size() > 0)
    {
        Coord* currnode = nodes.front();
        nodes.pop();

        if ( !(currnode->getValidMoves()) )
        {
            return;
        }

        else
        {
            for (int i = 0; i < MAX_CHILDREN_PER_COORD; i++)
            {
                if (currnode->children[i] != NULL)
                {
                    nodes.push(currnode->children[i]);
                }
            }
        }
    }
}

...and the Coord class...

class Coord : public Loc
{
    public:

    Coord(int xPos, int yPos);

    Coord* children[MAX_CHILDREN_PER_COORD];


    bool getValidMoves();


    bool operator==(Coord coord);
    bool operator==(Loc loc);

};

Coord::Coord(int xPos, int yPos) : Loc(xPos, yPos) {}


bool Coord::getValidMoves()
{
    //This line segfaults
    Coord test = *this;

    //Global boolean method. Checks found
    if (!foundTrue())
    {
        for (int i = 0; i < MAX_CHILDREN_PER_COORD; i++)
        {
            //If the above segfaulting line is commented out, this is the first place that segfaults
            int newX = x + knightPositions[i].x;
            int newY = y + knightPositions[i].y;

            if ( !(newX > GRID_X || newX < 0 || newY > GRID_Y || newY < 0) )
            {
                //knightPositions is a Loc array of length MAX_CHILDREN_PER_COORD
                children[i] = new Coord(x + knightPositions[i].x, y + knightPositions[i].y);
                //Global 2d array of ints. Gets checked by foundTrue()
                found[x + knightPositions[i].x][y + knightPositions[i].y] = true;
            }
        }

        return true;
    }

    else
    {
        return false;
    }

    //Otherwise, just leave it as a blank array
}


bool Coord::operator==(Coord coord)
{
    return coord.x == x && coord.y == y;
}

bool Coord::operator==(Loc loc)
{
    return loc.x == x && loc.y == y;
}

... and the Loc class, from which Coord inheirits...

class Loc
{
    public:
        int x, y;

        //Constructor
        Loc(int xPos, int yPos) : x(xPos), y(yPos) {}
};

The segfault occurs in Coord::getValidMoves() as indicated by the comments. If step through the code to that point, then make a watch for *this or x or this->x, I get a "Cannot access memory at 0xbaadf00d"

Why is this happening? Where have I messed up? I just don't understand how trying to access *this in a method could possibly result in a segfault.

Upvotes: 1

Views: 167

Answers (3)

Peter Alexander
Peter Alexander

Reputation: 54300

You need to initialise the elements of Coord::children. They aren't guaranteed to be NULL, so in populateTree(), when you do the null test on each child, you will get non-null children although they will not point to a valid Coord. When they are popped off the queue, and you call getValidMoves() on the invalid Coord you'll get the seg-fault.

Change the Coord constructor to:

Coord::Coord(int xPos, int yPos) : Loc(xPos, yPos)
{
    std::fill( children, children + MAX_CHILDREN_PER_COORD, NULL );
}

(you'll need to #include <algorithm> for std::fill.

Note that the segfault occurs on the attempt to dereference this because that's the first time you try to access the invalid memory.

Upvotes: 4

Omnifarious
Omnifarious

Reputation: 56088

If you make your constructor look like this:

Coord::Coord(int xPos, int yPos) : Loc(xPos, yPos)
{
    for (int i = 0; i < MAX_CHILDREN_PER_COORD; ++i) {
        children[i] = NULL;
    }
}

your problem will go away. You should initialize all your data members in the constructor.

The problem is that children will contain random data. It won't be initialized to NULL, and so the test currnode->children[i] != NULL will not be true even for non-existent children. You will then call member functions on these non-existent children and when those member functions run they will have an invalid this pointer.

Upvotes: 0

A segfault when accessing data members is a common problem when a method is called on an invalid (or invalidated) pointer. While the language provides the abstraction of objects and methods, the underlying implementation still has functions and data, where methods are functions (code) that applies to an data (implicit *this).

Check that the pointers are valid (not null, not freed) before calling the method, as that is surely the problem:

struct test {
   int x;
   void foo( int y ) {
      x = y;       // [1]
   }
};
int main() {
   test *p = 0;
   //p->foo();     // segfault in x=y above: this == 0
   p = new test;
   p->foo();
   delete p;
   // p->foo();    // undefined behavior, can be a segfault or not
   p = reinterpret_cast<test*>( "This is a literal" );
   p->foo();       // undefined behavior, probably segfault
                   // writing to read-only memory (this depends on compiler/environment)
}

In the code above, all errors will most be detected at the line marked as [1]

Upvotes: 1

Related Questions