1

As follow up to: Double free of child object after using the copy constructor

I followed the rule of 5 as suggested. But now it seems like the move assignment is happening on an uninitialized object (to object id 0)? I expected it to move from object 3 to object 2.

I have created the following (minimum?) example which seems to trigger my issue:

#include <stdio.h>
#include <stdint.h>

class A
{
public:
    A()
    {
        myCtr = ++ctr;
        printf("class A default Constructor - object id: %u\n", myCtr);
    }

    A(const A &a2) {
        myCtr = ++ctr;
        printf("class A copy constructor - object id: %u\n", myCtr);

    }

    A(A &&a2) {
        myCtr = a2.myCtr;
        a2.myCtr = 0;

        printf("class A move constructor - object id: %u\n", myCtr);

    }

    A & operator=(const A &a2) {
        myCtr = ++ctr;

        printf("class A copy assignment - from object id: %u - to object id: %u\n", a2.myCtr, myCtr);

        return *this;
    }

    A & operator=(A &&a2) {
        printf("class A move assignment - from object id: %u - to object id: %u\n", a2.myCtr, myCtr);

        if (this != &a2) {
            //myCtr = a2.myCtr;
            //a2.myCtr = 0;
        }

        return *this;
    }

    ~A()
    {
        printf("class A destructor - object id: %u\n", myCtr);
    }

private:
    uint64_t myCtr;
    static uint64_t ctr;
};

class B
{
public:
    B() {

    }

    B(char * input, uint32_t len) {
        for (uint32_t i = 0; i < len; i++)
        {
            /* do something */
        }
    }

    B(const B &b2) {
        characters = A(b2.characters);
    }

    B(B &&b2) {
        characters = A(b2.characters);
    }

    B & operator=(const B &b2) {
        characters = A(b2.characters);
    }

    B & operator=(B &&b2) {
        characters = A(b2.characters);
    }

    ~B() {

    }


private:
    A characters;
};

uint64_t A::ctr = 0;

int main(int argc, char *argv[]) {
    B b1 = B((char *)"b1", 2);
    B b2 = b1;

    return 0;
}

This produces the following output:

class A default Constructor - object id: 1
class A default Constructor - object id: 2
class A copy constructor - object id: 3
class A move assignment - from object id: 3 - to object id: 0
class A destructor - object id: 3
class A destructor - object id: 2
class A destructor - object id: 1

This is the line that i did not expect:

class A move assignment - from object id: 3 - to object id: 0

Expectation:

class A move assignment - from object id: 3 - to object id: 2

I am using the following compiler: Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26429.4

In case you down vote. Please specify why. I will gladly try to improve my question.

Edit:

It seems like only Visual C++ compiler targeting the x86 platform makes this issue occur. It works fine with g++ (x86 & x64), clang (x86 & x64) and msvc (x64). This makes it harder to figure out the origin of this issue for me.

eKKiM
  • 421
  • 5
  • 18
  • Tip: don't say `A` inside your class definition, just say `A`. – Kerrek SB Jun 10 '18 at 11:12
  • 1
    I don't understand what the problem is. What does your code do? What did you expect to happen instead? Is all of that code really necessary to demonstrate the issue? See [mcve]. – melpomene Jun 10 '18 at 11:24
  • 1
    Templating is removed because it had no usage in the example. I added the output and clarified where the output does not what i expected it to do. – eKKiM Jun 10 '18 at 11:28
  • I get `class A move assignment - from object id: 3 - to object id: 2`. – melpomene Jun 10 '18 at 11:31
  • @melpomene thats what i was expecting but does not happen in my environment. What compiler are you using? – eKKiM Jun 10 '18 at 11:40
  • Whatever https://ideone.com/fah3Gy uses (probably g++). – melpomene Jun 10 '18 at 11:41
  • Use [`"%" PRIu64` instead of `"%u"`](https://stackoverflow.com/a/9225648/5376789), otherwise the behavior is undefined. Then see if it works as expected after this correction. – xskxzr Jun 10 '18 at 14:16

1 Answers1

1

In constructor

B(const B &b2) {
    characters = A(b2.characters);
}

You firstly create temporary object of type A and then move it to the characters variable. This is why move assignment happens.

Try to replace this constructor by:

B(const B &b2) : characters(b2.characters)
{   
}

and you will get the result which you predict (https://ideone.com/yqvINu, VS example: http://rextester.com/PTPZKR93068).

  • It seems like the issue only occurs when using MSVC++ compiler targetting the x86 platform. I am not sure which compiler is used in your provided link. – eKKiM Jun 10 '18 at 12:15
  • The rextester.com is targeting x64: http://rextester.com/TWXH56217 I can only reproduce the issue using MSVC++ targeting x86! – eKKiM Jun 10 '18 at 12:38