Unsafe memcpy in RTCpp Copy Constructor and Assignment Operator

[GoogleCodeExporter]

I don't know how to put my own main function in the Umple file, so I modified 
the generated file that contained an empty main function to run some test code.

I used the following Umple file, main function, and added an include to the 
generated class cpp file to make it compile.

//test.ump
generate RTCpp;

class MyClass {
    String stringA;
}

//test_Main.cpp
#include <iostream>

#include <test/MyClass.h>
int main(int argc, char *argv[]){
    MyClass insA("abc");
    {
        MyClass insB(insA);
    }

    std::cout << insA.getStringA() << std::endl;

    return 0;
}

//test/MyClass.cpp, @line 20
#include <cstring> //for memcpy, used in internalCopy

All the generated files with modifications are attached.

I compile and run this example:

g++ -I. -Itest test/MyClass.cpp test_Main.cpp -o testProgram -fpermissive 
-std=gnu++11
./testProgram

This should output "abc", but it outputs a blank line instead. Note that if the 
line "MyClass insB(insA);" (constructing a copy object using the copy 
constructor) is commented out and the example recompiled and rerun, it does 
output "abc" as expected.

I believe this is because of the way internalCopy is implemented, a function 
used to perform the data transfer used both in the copy constructor and 
assignment operator. internalCopy (defined on line 109 in test/MyClass.cpp) 
uses memcpy to transfer the data. This will do a raw memory copy of the other 
class, rather than going through the other class's accepted copying mechanism. 
This is unsafe for classes with nontrivial copy constructors and assignment 
operators, as they may have to copy dynamic memory to a new copy, or produce 
copies of other resources. Bypassing the standard mechanism will mostly likely 
result in undefined behavior.

For example, in this case the actual string data is (most likely) stored as a 
char* pointer held inside the string class which points to some dynamically 
allocated memory owned by that instance of the string class. Doing a memory 
copy produces a new instance with the exact same pointer, that also thinks it 
owns the data. insB holds a "copy" string that thinks it owns the same memory 
as the string in insA. insB goes out of scope, and the string destructor is 
implicitly invoked, which frees the string memory. Since the memory is shared 
by insA, when the string is accessed from insA it is now invalid. 

I suggest changing internalCopy to use the assignment operator for each of its 
member variables to transfer data in a safe way. If the assignment operator 
does not exist for that member (causing the compiler to produce an error on 
compiling the C++ code), then that seems an issue currently out of scope for 
Umple to deal with, and something the programmer should implement manually. 

Original issue reported on code.google.com by Astronomix@gmail.com on 17 Jan 2014 at 10:48

Attachments:

• cppInternalCopyIssue.tar.gz

[GoogleCodeExporter]

Directing the comment at ahmed, I will email the originator directly about how 
to put a main directly in the Umple file.

Original comment by TimothyCLethbridge on 18 Jan 2014 at 1:32

• Added labels: cpp, Component-SemanticsAndGen, Diffic-Med, Security

[GoogleCodeExporter]

We delivered the fix. The cause was a  typo, the destination and target were 
switched accidentally in the method call.

Original comment by ahmedvc@gmail.com on 18 Jan 2014 at 11:39

• Changed state: Fixed