double free or corruption error when copying object with memcpy

Question

I have the following code:

#include <iostream>
#include <string>
#include <cstring>

struct test {
    std::string name;
    size_t id;
};


int main() {
    test t;
    t.name = "147.8.179.239";
    t.id = 10;

    char a[sizeof(t)] = "";
    std::memcpy(a, &t, sizeof(t));

    test b;
    std::memcpy(&b, a, sizeof(t)); 

    std::cout << b.name << " " << b.id << std::endl;
}

when I compile it and run it, it gives me the following error:

147.8.179.239 10
*** Error in `./test': double free or corruption (fasttop): 0x0000000000bf9c20 ***
Aborted (core dumped)

It turns out the code can print out the result. But how can I fix this error?

Variable length array. It's size is unknown at compile time, and is illegal in C++. — lost_in_the_source, Aug 18 '16 at 13:13
@Johnnylin Variable length array. It is forbidden in standard C++ (although it is allowed by some compiler extensions). — Algirdas Preidžius, Aug 18 '16 at 13:13
@stackptr how is that a VLA? `sizeof(t)` is known at compile time. If OP was doing `char a[t.name.length()]` it would be a VLA but as this is I don't see how this is a VLA. — Borgleader, Aug 18 '16 at 13:18
It's sad that compilers don't warn about using `memcpy` on non-`std::is_trivial::value` types — KABoissonneault, Aug 18 '16 at 14:10

dbush · Accepted Answer · 2016-08-18T13:32:09.720

17

By using memcpy the way you are, you have two std::string objects which are exactly identical. This includes any pointers they may use internally. So when the destructor for each object runs, they are both attempting to free the same pointer.

This is why you need to use either the copy constructor or assign one to the other (i.e. use the overridden operator=). It knows about those implementation differences and handles them correctly, i.e. it allocates a separate memory buffer for the destination object.

If you want to extract the string contained in a std::string, you need to serialize the object to a known representation. Then you can deserialize it to convert it back.

std::string s1 = "hello";
printf("len=%zu, str=%s\n",s1.size(),s1.c_str());

// serialize
char *c = new char[s1.size()+1];
strcpy(c, s1.c_str());
printf("c=%s\n",c);

// deserialize
std::string s2 = c;
printf("len=%zu, str=%s\n",s2.size(),s2.c_str());

You would perform similar steps for other class objects.

edited Aug 18 '16 at 13:32

answered Aug 18 '16 at 13:18

dbush

205,898
23
218
273

what i want to achieve is to convert a struct that contains `std::string` and other POD type to a char array. Then I can use socket to transmit it. Thanks for your explanation. – Johnnylin Aug 18 '16 at 13:24
1

@Johnnylin some_string.c_str() will give you the string's underlying char array, you can then send that, no need for copies. (also .size() will give you how many chars are in the array) – Borgleader Aug 18 '16 at 13:25
4

@Johnnylin Then you need to decide on a format for that data at the byte level because sockets provide byte-level pipes. You then need to write code to convert the data you want to send into the format you chose so that you can send it. – David Schwartz Aug 18 '16 at 20:25

Jesper Juhl · Answer 2 · 2016-08-18T13:21:12.777

12

You cannot memcpy() a non-POD struct like test. You are completely wrecking the std::string member.

You must use the copy constructor to copy C++ objects.

edited Aug 18 '16 at 13:21

answered Aug 18 '16 at 13:14

Jesper Juhl

30,449
3
47
70

1

So, there are two errors ? one is variable length array and the other is memcpy non-POD struct ? – Johnnylin Aug 18 '16 at 13:15
4

*VLAs are illegal* more precisely they are a non-standard extension of some compilers (gcc and possibly clang), that being said I don't see any VLAs. – Borgleader Aug 18 '16 at 13:16
If you just want to make `b` equal to `t`, then just do `b = t;` - or am I misunderstanding what you are trying to do? – Jesper Juhl Aug 18 '16 at 13:23
what I want to achieve is to convert a struct that contains std::string, together with other POD types, to a char array. Then I can use socket to transmit it. Thanks for your explanation. – Johnnylin Aug 18 '16 at 13:25

Ishay Peled · Answer 3 · 2016-08-18T13:24:00.193

6

The actual reason you're getting a double free error is pinned to the fact that instead of creating a new string object for your variables a and b, you just copy the reference (a string object is implemented using a variable length char *).

Since the string destructor frees this memory address when your program ends, and as explained above you have two string objects pointing to the same address, you get a double free error

This will work, like @JesperJuhl said, you must use a copy constructor

#include <iostream>
#include <string>
#include <cstring>

struct test
{
    std::string name;
    size_t id;
};


int main()
{
    test t;
    test a;
    test b;

    t.name = "147.8.179.239";
    t.id = 10;

    a=t;
    b=t;

    std::cout << b.name << " " << b.id << std::endl;
}

edited Aug 18 '16 at 13:24

answered Aug 18 '16 at 13:18

Ishay Peled

2,783
1
23
37

So basically, I cannot memcpy a struct that contains non-POD type (`string`), unless I change `std::string` to `char` array? – Johnnylin Aug 18 '16 at 13:22
1

Correct, it is generally good practice to use copy constructors for objects. Actually, there should be no valid reason not to if you're using C++ - the copy constructor abstracts all additional memory allocations you may not even be aware of that happen under the hood – Ishay Peled Aug 18 '16 at 13:25

double free or corruption error when copying object with memcpy

3 Answers3