"address not from malloc()" error using electric fence

I've been writing a test case program to demonstrate a problem with a larger program of mine, and the tes开发者_开发百科t case has a bug that the original program does not.

Here's the header file:

// compiled with g++ -I/usr/local/bin/boost_1_43_0 -Wall -std=c++0x -g test.cpp

#include <bitset>
#include <boost/shared_ptr.hpp>
#include <vector>

typedef std::vector< std::vector< std::bitset<11> > > FlagsVector;

namespace yarl
{
    namespace path
    {
        class Pathfinder;
    }

    namespace level
    {
        class LevelMap
        {
        // Member Variables
            private:
                int width, height;
                FlagsVector flags;

            public:
                boost::shared_ptr<path::Pathfinder> pathfinder;

        // Member Functions
            LevelMap(const int, const int);

            int getWidth() const {return width;}
            int getHeight() const {return height;}

            bool getFifthBit(const int x, const int y) const
            {
                return flags.at(x).at(y).test(5);
            }
        };



        class Level
        {
        // Member Variables
            public:
                LevelMap map;

        // Member Functions
            public:
                Level(const int w=50, const int h=50);
        };
    }


    namespace path
    {
        class Pathfinder
        {
        // Member Variables
            private:
                boost::shared_ptr<level::LevelMap> clientMap;

        // Member Functions
            public:
                Pathfinder() {}
                Pathfinder(level::LevelMap* cm)
                : clientMap(cm) {}

                void test() const;
        };
    }
}

and here's the implementation file:

#include <iostream>
#include "test.hpp"
using namespace std;

namespace yarl
{
    namespace level
    {
        LevelMap::LevelMap(const int w, const int h)
        : width(w), height(h), flags(w, vector< bitset<11> >(h, bitset<11>())),
          pathfinder(new path::Pathfinder(this)) 
        {}



        Level::Level(const int w, const int h)
        : map(w,h)
        {
            map.pathfinder->test();
        }
    }



    namespace path
    {
        void Pathfinder::test() const
        {
            int width = clientMap->getWidth();
            int height = clientMap->getHeight();
            cerr << endl;
            cerr << "clientMap->width: " << width << endl; 
            cerr << "clientMap->height: " << height << endl; 
            cerr << endl;
            for(int x=0; x<width; ++x)
            {
                for(int y=0; y<height; ++y)
                {
                    cerr << clientMap->getFifthBit(x,y);
                }
                cerr << "***" << endl; // marker for the end of a line in the output
            }
        }
    }
}

int main()
{
    yarl::level::Level l;
    l.map.pathfinder->test();
}

I link this program with electric fence, and when I run it it aborts with this error:

ElectricFence Aborting: free(bffff434): address not from malloc().

Program received signal SIGILL, Illegal instruction.
0x0012d422 in __kernel_vsyscall ()

backtracing from gdb shows that the illegal instruction is in the compiler-generated destructor of Pathfinder, which is having trouble destructing its shared_ptr. Anyone see why that is?

yarl::level::Level l;

You instantiate an automatic Level variable, which, in its constructor constructs its member pathfinder like so:

pathfinder(new path::Pathfinder(this))

Then in the Pathfinder constructor, it takes the Level pointer that you pass in and assigns that to a shared_ptr. The shared_ptr then takes ownership of this pointer.

This is incorrect for several reasons:

1. A shared_ptr should be used to manage dynamically allocated objects, not automatically allocated objects
2. If you want to use shared_ptr, then you should use it everywhere: as it is now, you pass raw pointers (e.g. to the constructor of Pathfinder, but then store them as shared_ptrs. This just opens a big can of ownership worms.
3. The correct way to assign this to a shared_ptr is to derive from enable_shared_from_this; note however that you cannot get a shared_ptr from this in a constructor.

When the shared_ptr is destroyed, it will try to delete the pointer it manages. In this case, however, that pointer is not to a dynamically allocated object (i.e., allocated with new), but to an automatically allocated object (i.e., on the stack). Hence, the error.

If you don't need something to take ownership of a resource, there is nothing wrong with using a raw pointer (or a reference, if you have that option).

You are constructing shared_ptr from a pointer which is not supposed to be managed by shared_ptr. (The this pointer)

When the last shared_ptr's copy is destroyed this memory is free'd - when in fact it should not - the this is on stack in this case.

There is a reason the constructor of shared_ptr is explicit - it is exactly to avoid such an unnoticed conversion from regular pointer, which is not to be managed by shared_ptr, to a shared_ptr - once you pass such a pointer into shared_ptr, your program is doomed - the only way out is by deleting the pointer you have not intended to delete.

In general it is advisable to construct shared pointer with new directly - such as ptr(new Somethings(x,y,z) - this way you don't risk an exception leaking your allocated, but unassigned to a shared_ptr memory.

The Level contains a LevelMap member variable. When the Level gets destroyed, it will also destroy its LevelMap.

On the other hand a pointer to this LevelMap member is passed to Pathfinder, which creates a shared_ptr<> from the passed pointer. This newly created shared_ptr<> thinks that it owns the object it points to and will try to destroy it once the Pathfinder gets destroyed.

So the LevelMap is destroyed several times.

In the example the LevelMap is created on the stack. Therefore the delete called by shared_ptr<> can see that the address is not from the heap and you get an error. If your real program also has this problem, but all those objects are dynamically allocated, the error will probably not be detected. You will just get silent memory corruption and weird crashes later on.