-3

I have written the following code which seems to be working fine with old std::string implentation. But with gcc 5.1, it crashes.

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

struct abc
{
   public:
   abc() {}
   abc(const std::string& x)
      : gullu(x)
   {
   }
   std::string gullu;
};

int main()
{
   abc *a = new abc("dhfghdf");
   abc *b = new abc();
   memcpy((void *)&b, (void *)&a, sizeof(abc));
   std::cout << a->gullu.data() << std::endl;
   std::cout << b->gullu.data();
   return 0;
}

Debugging it, I found that after doing memcpy the content of object 'a' becomes garbage.

After a's construction,

(gdb) p *a $1 = {gullu = {static npos = 4294967295, _M_dataplus = {> = {<__gnu_cxx::new_allocator> = {}, }, _M_p = 0x804ea18 "dhfghdf"}, _M_string_length = 7, {_M_local_buf = "dhfghdf\000\000\000\000\000\000\000\000", _M_allocated_capacity = 1734764644}}}

After b's construction

(gdb) p *b $2 = {gullu = {static npos = 4294967295, _M_dataplus = {> = {<__gnu_cxx::new_allocator> = {}, }, _M_p = 0x804ea38 ""}, _M_string_length = 0, {_M_local_buf = '\000' , _M_allocated_capacity = 0}}}

After memcpy

(gdb) p *a $4 = {gullu = {static npos = 4294967295, _M_dataplus = {> = {<__gnu_cxx::new_allocator> = {}, }, _M_p = 0x666468 }, _M_string_length = 2572404, {_M_local_buf = "t@'\000\030Ùÿ¿I\215\004\b\001\000\000", _M_allocated_capacity = 2572404}}}

(gdb) p *b $5 = {gullu = {static npos = 4294967295, _M_dataplus = {> = {<__gnu_cxx::new_allocator> = {}, }, _M_p = 0x804ea18 "dhfghdf"}, _M_string_length = 7, {_M_local_buf = "dhfghdf\000\000\000\000\000\000\000\000", _M_allocated_capacity = 1734764644}}}

I am using a thirdparty library which seems to be doing a memcpy, which was working with previous compiler and not working with gcc 5.1 because of this issue

Can someone help me with this?

Rahul Agrawal
  • 132
  • 1
  • 10
  • 4
    Why are you using `memcpy()` instead of a copy constructor at all? What you have always called undefined behavior, no matter which compiler version. – πάντα ῥεῖ Jul 28 '15 at 06:54
  • 1
    I am using a thirdparty library which seems to be doing a memcpy, which was working with previous compiler and not working with gcc 5.1 because of this issue – Rahul Agrawal Jul 28 '15 at 06:56
  • Don't memcpy std::string. You need to read up on the consequences of shallow copies. In any case, even if it were okay, you should only be copying the size of the pointer since that's what you get from `&a`. – paxdiablo Jul 28 '15 at 06:56
  • Its not crashing for me in `GCC 5.1(C++11/C++14/C++17)`. Gives output `dhfghdf dhfghdf` – Praveen Jul 28 '15 at 06:59
  • @Praveen: Undefined Behavior is often tricky to reproduce. – MSalters Jul 28 '15 at 08:19

3 Answers3

3

Well, you are not doing what you want. While you are clearly trying to copy the content of a string to another, what you are doing is just copy the value of a pointer (a) to another (b), but copying a wrong size!

By the way, what you intended is memcpy(b, a, sizeof(abc));, but this is not supposed to work too (it will work on several cases though)! As I've learned on my skin (and thanks to this answer), you can't memcpy an object that has non-trivial initialization. By reusing the storage of such an object you end its lifetime, but just memcpying to it you do not resurrect it, so the object pointed by b would be not alive.

Quotes from the C++11 standard (§3.8 Object lifetime [basic.life]):

The lifetime of an object is a runtime property of the object. An object is said to have non-trivial initialization if it is of a class or aggregate type and it or one of its members is initialized by a constructor other than a trivial default constructor. [ Note: initialization by a trivial copy/move constructor is non-trivial initialization. — end note ] The lifetime of an object of type T begins when: — storage with the proper alignment and size for type T is obtained, and — if the object has non-trivial initialization, its initialization is complete. The lifetime of an object of type T ends when: — if T is a class type with a non-trivial destructor (12.4), the destructor call starts, or — the storage which the object occupies is reused or released. §

Community
  • 1
  • 1
Paolo M
  • 12,403
  • 6
  • 52
  • 73
  • 3
    This is the core point- you simply cannot ever `memcpy` a `std::string`, no matter if you get the parameters right. – Puppy Jul 28 '15 at 07:16
  • @Puppy Yeah, it's pretty obvious for some complex objects, but on some legacy code I had a hard time figuring out what the problem was. It turned out (eventually) that the problem was an innocent looking qsort on an innocent looking class... But the copy ctor was not called and so some very obscure crash came out. – Paolo M Jul 28 '15 at 07:22
2

You're copying sizeof(abc) bytes of data but the pointers you give memcpy point to a and b's pointers, not the objects - the address-of in this (void*)&a is definitely wrong.

However you really should not be doing this because copying objects like that instead of using they copy ctor or other proper method will lead to crashes and undefined behaviour. In this specific case what might happen if the string is a bit longer is you end up with two string objects pointing to the same heap block therefore you will get double delete exiting the scope which is a non-recoverable crash.

Alexander Balabin
  • 2,055
  • 11
  • 13
2

Your code triggers undefined behavior because you are copying towards the address of pointer b more bytes than the size of the pointer: you are copying sizeof(abc) instead of sizeof(a).

Serge Rogatch
  • 13,865
  • 7
  • 86
  • 158