Search code examples
c++c++11gccclangrvalue-reference

Segmentation fault when specifying rvalue as return value


I have a factory class which constructs new objects. The new objects should not be copied but may be moved. So I figured I would delete the copy constructor and copy assignment operator while providing a move constructor and move assignment operator.

I figured that, to help convey the idea that the object must be moved into place rather than copied, I would return an rvalue reference. However, upon doing this, it seems that the compiler always generates code which destroys the "expiring" object being returned and then provides that same (destroyed!) object to the move constructor or move assignment operator.

So I'm wondering; is there something in the standard which I am violating? If so, is there a warning (error, preferably) I can enable to prevent me from continuing to do something so stupid? Or else, if I'm not violating any standard, then I assume this is a compiler bug?

// g++ (Ubuntu 4.9.2-0ubuntu1~14.04) 4.9.2
// g++ test.cpp -std=c++11

#include <iostream>
#include <memory>

struct PTR {
    char * blah = nullptr;

    PTR(char* blah) : blah(blah)
    {
        std::cout << "\tctor  @" << (void*)this << std::endl;
    }

    PTR(const PTR & copy_ctor) : blah(new char)
    {
        *blah = *copy_ctor.blah;
        std::cout << "\tcopy @@" << (void*)this << "\t<-" << (void*)&copy_ctor << std::endl;
    }

    PTR(PTR && move_ctor) : blah(move_ctor.blah)
    {
        move_ctor.blah = nullptr;
        std::cout << "\tctor&&@" << (void*)this << "\t<@" << (void*)&move_ctor << std::endl;
    }

    PTR & operator=(const PTR & copy_assign)
    { delete blah;
        blah = new char;
        *blah = *copy_assign.blah;
        std::cout << "copyas@" << (void*)this << "\t<-" << (void*)&copy_assign << std::endl;
        return *this;
    }

    PTR & operator=(PTR && move_assign)
    {
        delete blah;
        blah = move_assign.blah;
        move_assign.blah = nullptr;
        std::cout << "\tmove&&@" << (void*)this << "\t<@" << (void*)&move_assign << std::endl;
        return *this;
    }

    ~PTR()
    {
        std::cout << "\tdtor~~@" << (void*)this << std::endl;
        delete blah;
    }
};

PTR make_ptr_l() {
    PTR ptr(new char());
    return std::move(ptr); // Without std::move, compiler *may* opt to copy the class, which is undesired
}

PTR && make_ptr_r() {
    PTR ptr(new char());
    return std::move(ptr); // Requires std::move to turn ptr into rvalue, otherwise compiler error
}

int main() {
    std::cout << "lvalue: \n" << std::flush;
    PTR ptr = make_ptr_l();
    std::cout << "successful\nrvalue new: \n" << std::flush;
    {
        PTR ptr_r = make_ptr_r();
        std::cout << "successful\nrvalue assign: \n" << std::flush;
    }
    ptr = make_ptr_r();
    std::cout << "successful" << std::endl;
    return 0;
}

With the above code, you can see the following output:

lvalue: 
    ctor  @0x7ffed71b7a00
    ctor&&@0x7ffed71b7a30   <@0x7ffed71b7a00
    dtor~~@0x7ffed71b7a00
successful
rvalue new: 
    ctor  @0x7ffed71b7a00
    dtor~~@0x7ffed71b7a00
    ctor&&@0x7ffed71b7a40   <@0x7ffed71b7a00
successful
rvalue assign: 
    dtor~~@0x7ffed71b7a40
*** Error in `./a.out': double free or corruption (fasttop): 0x0000000001d1bc40 ***
Aborted (core dumped)

As you can see after rvalue new, an object is constructed, then immediately destroyed, then the destroyed object is passed to a move constructor. Since the move constructor therefore accesses the destroyed variables, this is the source of segmentation faults.

I have tried this with g++ (Ubuntu 4.9.2-0ubuntu1~14.04) 4.9.2 as well as Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn).


Solution

  • You're returning a reference to a temporary. You can tell this from your output:

    rvalue new: 
        ctor  @0x7ffed71b7a00
        dtor~~@0x7ffed71b7a00
        ctor&&@0x7ffed71b7a40   <@0x7ffed71b7a00
    

    You construct and destroy 7a00 before you move construct 7a40 with it. The fact that you happen to return an rvalue reference rather than an lvalue reference doesn't matter. It's basically still something of the form:

    T& foo() {
        T object;
        return object;
    }
    

    That's why the make_ptr_l works - you're returning a value, not a reference. And the std::move() there isn't necessary.