Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

std::function bad memory access when creating a temporary

I'm currently implementing few abstractions to represent level-set operations for 3D objects. Basically what is described in this amazing page for GLSL shaders.

To give a brief overview, a 3D object can be described by a function that maps the R^3 domain to a scalar called level-set (or signed-distance-function). For example, for a sphere, the level-set function is defined by phi(X) = X.Norm2() - R*R where Norm2 represents the squared Euclidean norm of a vector in R^3.

So I came out with a LevelSetObject class that represents such a concept:

 class LevelSetObject
    {

    using SDFFunction = std::function<double(double, double, double)>;

    protected:
        SDFFunction m_SDF;

    public:

        double SDF(double x, double y, double z) const {
            return m_SDF(x, y, z);
        }

Now I wanted to define few operators between LevelSetObjects. For example the union operator:

LevelSetObject LevelSetObject::operator+(const LevelSetObject& other) const {
        LevelSetObject outObj;
        outObj.m_SDF = [this, other]
            (double x, double y, double z) {
                return std::min(m_SDF(x, y, z), other.m_SDF(x, y, z));
            };
        return outObj;
    }

But I'm experiencing bad memory access when I create a temporary due to, for example, a triple sum (while if I sum the two objects separately as in the commented case, no memory leak is spotted using Valgrind and not SIGSEV). LevelSetSphere is a derived class of LevelSetObject where I define specifically the SDF of a sphere (given its center and its radius)

int main(int argc, char* argv[]) {

    // Generate the central sphere
    double radius = 1.0;
    SimpleVector center(2, 2, 2);
    LevelSetSphere sphere(radius, center);

    // Generate the ears spheres
    LevelSetSphere ear1(radius/2, SimpleVector(1, 1, 2));
    LevelSetSphere ear2(radius/2, SimpleVector(3, 1, 2));

    // Combine objects
    auto mickeyMouse = sphere + ear1 + ear2;
    //auto first = sphere + ear1;
    //auto mickeyMouse = first + ear2;

    // Materialize in the domain

    mickeyMouse.SDF(0.0, 0.0, 0.0);



}

What I suppose is that in the operator+ definition, the std::function keeps a reference to other that becomes a dangling reference when I actually call m_SDF because a temporary is created during the triple sum. I also tried to change the signature of operator+ to operator+(const LevelSetObject other), so passing by copy, but the outcome is the same.

Where am I failing? :)

like image 369
rdbisme Avatar asked Jan 24 '19 13:01

rdbisme


Video Answer


2 Answers

The lambda in the derived class captures this to the derived class, and shoves it into the base class's std::function.

This is a recipe for trouble.

What this means that, at the very least, the derived class must be fully Rule of 3 compliant, and implement, at the very least, a copy constructor and an assignment operator that meticulously reinstalls a new lambda, with a freshly-captured this that actually refers to the right instance of the derived class.

If you have a std::function member of a class, that captures its own this, and the class gets copied, the captured this does not get automatically updated to refer to the new instance of the class. C++ does not work this way. The new class's std::function's this still references the original instance of the class. And if an instance of a class is assigned from another instance of the class, guess what? The copied std::function's captured this still points to the copied-from instance of the class.

But I don't really see anything that the std::function does here that cannot be implemented by a garden-variety virtual function. Simply replace m_SDF by a virtual function, and this entire headache goes away.

like image 144
Sam Varshavchik Avatar answered Dec 29 '22 16:12

Sam Varshavchik


Your bad memory access isn't due to the other variable, it's the this pointer of the temp object going out of scope.

You can fix it by capturing the SDF variables explicitly, like this:

LevelSetObject LevelSetObject::operator+(const LevelSetObject& other) const {
        LevelSetObject outObj;
        auto& SDF=this->m_SDF;
        auto& other_SDF=other.m_SDF
        outObj.m_SDF = [SDF, other_SDF]
            (double x, double y, double z) {
                return std::min(SDF(x, y, z), other_SDF(x, y, z));
            };
        return outObj;
    }
like image 34
Joseph Ireland Avatar answered Dec 29 '22 17:12

Joseph Ireland