Why does gcc 8 cause double free in this std::string example

1 day ago 2
ARTICLE AD BOX

As indicated by the comments, this is likely a compiler bug. I ran the source code from https://godbolt.org/z/sGWWbc7WG compiled with gcc 8.5 locally on my Linux installation in valgrind, and I got this output:

==48== Invalid free() / delete / delete[] / realloc() ==48== at 0x484C3B4: operator delete(void*) (vg_replace_malloc.c:1131) ==48== by 0x401711: deallocate (new_allocator.h:125) ==48== by 0x401711: deallocate (alloc_traits.h:462) ==48== by 0x401711: _M_destroy (basic_string.h:230) ==48== by 0x401711: _M_dispose (basic_string.h:225) ==48== by 0x401711: ~basic_string (basic_string.h:661) ==48== by 0x401711: MyClass::run() (example.cpp:16) ==48== by 0x4011AD: main (example.cpp:27) ==48== Address 0x4e54080 is 0 bytes inside a block of size 41 free'd ==48== at 0x484C3B4: operator delete(void*) (vg_replace_malloc.c:1131) ==48== by 0x4014D1: deallocate (new_allocator.h:125) ==48== by 0x4014D1: deallocate (alloc_traits.h:462) ==48== by 0x4014D1: _M_destroy (basic_string.h:230) ==48== by 0x4014D1: _M_dispose (basic_string.h:225) ==48== by 0x4014D1: ~basic_string (basic_string.h:661) ==48== by 0x4014D1: MyClass::shrinkAndCopy(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (example.cpp:7) ==48== by 0x4015F3: MyClass::modify(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (example.cpp:11) ==48== by 0x4016CC: MyClass::run() (example.cpp:20) ==48== by 0x4011AD: main (example.cpp:27) ==48== Block was alloc'd at ==48== at 0x4848F93: operator new(unsigned long) (vg_replace_malloc.c:487) ==48== by 0x40137E: void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>(char const*, char const*, std::forward_iterator_tag) (basic_string.tcc:219) ==48== by 0x401673: _M_construct_aux<char const*> (basic_string.h:240) ==48== by 0x401673: _M_construct<char const*> (basic_string.h:259) ==48== by 0x401673: basic_string<> (basic_string.h:520) ==48== by 0x401673: MyClass::run() (example.cpp:16) ==48== by 0x4011AD: main (example.cpp:27)

This backtrace clearly shows that the original buffer of the string created in run was freed twice. The first free happened inside shrinkAndCopy, while the second free happens in run during exception unwinding. The debug info ties the call of the string destructor that frees the buffer a second time to the declaration of the local variable stream (line 16).

It makes sense that the original 41-character buffer is freed from inside shrinkAndCopy, because substr created a new string which will then get move-assigned to stream, so stream is supposed to manage the new buffer now, and the old buffer is obsolete. During unwinding, the new buffer should have been freed, not the old one again. It seems that the exception unwinding code in run does not operate on the state of stream it is supposed to have.

If you look at the compiled and linked code in compiler explorer (configured at https://godbolt.org/z/8Mvr3TbYY), observe how [rsp] (which contains the pointer to the string buffer) is loaded into the register rbx in line 401 (gcc calls that value _10). The unwind code starts at line 422, and uses rbx, again naming it _10, so this is supposed to be the value loaded into rbx before calling run. This ends up as the parameter to delete[]. This is obviously bad, because stream._M_dataplus has been modified by run.

Read Entire Article