1

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).

5
  • 2
    make_ptr_r returns a reference to a local variable. The variable dies when the function exits, and the reference becomes dangling. Any attempt to actually use this reference exhibits undefined behavior. The fact that this is an rvalue reference doesn't change anything. Apr 15, 2015 at 20:54
  • 1
    To add to what @IgorTandetnik said: The return type of make_ptr_r should be PTR without the &&. Check the output of your code and see if it makes sense to you. Apr 15, 2015 at 20:56
  • @DanielFrey I did check the output and even mentioned that it doesn't make so much sense. What I misunderstood was the idea that std::move() with a return type of rvalue-reference should move the object out of local.
    – inetknght
    Apr 15, 2015 at 21:14
  • 1
    @inetknght Sorry, no offense intended! I meant the output after you change the return type as I suggested and compare it to you previous output. I hope the difference will help you understand that even without the && on the return type, your move-ctor and move-assignment will then be correct and help to reduce copy operations. Apr 15, 2015 at 21:16
  • 1
    std::move doesn't move anything. It's just a convenient way to cast an object to rvalue reference: std::move(x) is roughly equivalent to static_cast<decltype(x)&&>(x). Something else (e.g. a move constructor) can now take this reference as a parameter, and move data out of x. Apr 15, 2015 at 21:18

1 Answer 1

2

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.

2
  • So the rvalue reference in this case specifically does not move the object as I had understood it would?
    – inetknght
    Apr 15, 2015 at 21:12
  • 1
    @inetknght std::move() does not move. It is just a cast.
    – Barry
    Apr 15, 2015 at 21:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.