Why do Clang and VS2013 accept moving brace-initialized default arguments, but not GCC 4.8 or 4.9?

Question

Like the title suggests, I have a short demo program that compiles on with all of those compilers, but core dumps when ran after compiling with gcc 4.8 and gcc 4.9:

Any ideas as to why?

#include <unordered_map>

struct Foo : std::unordered_map<int,int> {
    using std::unordered_map<int, int>::unordered_map;
    // ~Foo() = default; // adding this allows it to work
};

struct Bar {
    Bar(Foo f = {}) : _f(std::move(f)) {}
    // using any of the following constructors fixes the problem:
    // Bar(Foo f = Foo()) : _f(std::move(f)) {}
    // Bar(Foo f = {}) : _f(f) {}

    Foo _f;
};

int main() {
    Bar b;

    // the following code works as expected
    // Foo f1 = {};
    // Foo f2 = std::move(f1);
}

My compilation settings:

g++ --std=c++11 main.cpp

Here is a backtrace from GDB:

#0  0x00007fff95d50866 in __pthread_kill ()
#1  0x00007fff90ba435c in pthread_kill ()
#2  0x00007fff8e7d1bba in abort ()
#3  0x00007fff9682e093 in free ()
#4  0x0000000100002108 in __gnu_cxx::new_allocator<std::__detail::_Hash_node_base*>::deallocate ()
#5  0x0000000100001e7d in std::allocator_traits<std::allocator<std::__detail::_Hash_node_base*> >::deallocate ()
#6  0x0000000100001adc in std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<int const, int>, false> > >::_M_deallocate_buckets ()
#7  0x000000010000182e in std::_Hashtable<int, std::pair<int const, int>, std::allocator<std::pair<int const, int> >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_deallocate_buckets ()
#8  0x000000010000155a in std::_Hashtable<int, std::pair<int const, int>, std::allocator<std::pair<int const, int> >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::~_Hashtable ()
#9  0x000000010000135c in std::unordered_map<int, int, std::hash<int>, std::equal_to<int>, std::allocator<std::pair<int const, int> > >::~unordered_map ()
#10 0x00000001000013de in Foo::~Foo ()
#11 0x0000000100001482 in Bar::~Bar ()
#12 0x0000000100001294 in main ()

*** error for object 0x1003038a0: pointer being freed was not allocated ***

Amaleta

what flags are you using on gcc? – Frum

Why happens when you use this constructor: Bar(Foo f = {Foo()}) : _f(std::move(f)) {} – Frum

It core dumps, same reason. – Amaleta

Can you run a gdb backtrace? – Frum

gcc 4.9 exhibits the same behavior. And sure, I'll post a bt in a sec. – Amaleta

A bit simplified version: coliru.stacked-crooked.com/a/7be23631a99ee9f0 (reduced to default argument {} and std::unordered_map constructors/destructors) – Scorpion

That actually doesn't compile in gcc 4.9 error: converting to 'Foo {aka std::unordered_map<int, int>}' from initializer list would use explicit constructor – Amaleta

the destructor of Foo is being called twice – Zarf

I think I would expect it to be called twice, right? One for the object created in Bar::Bar and once for Bar::~Bar – Amaleta

I'm guessing it's a bug in GCC. If you compare this example to @zch's example, the only difference is the call to bar explicitly passes in a parameter of {} instead of using the default argument, and those two should be the same; yet this one runs fine but the other one crashes. – Langdon

One can't call destructor twice on same object. That's a reportable error. – Frum

This is clearly a bug in gcc (probably in the hashtable move constructor in libstdc++), please report to gcc bugzilla. – Reticulation

@Frum it also depends on what is the effect of that std::move – Zarf

There are two Foo destrutors being called and that's expected: Once for the temporary created in Bar::Bar (which is then moved to Bar::_f) and then once for ~Bar::Bar. – Amaleta

I'll report it to them, kinda interesting regardless. It looks like adding a destructor to Foo, even ~Foo() = default; allows it to work. – Amaleta

@Amaleta try to fix it with Bar(Foo f = {{1,2}}), works for me, apparently when the initializer_list is empty you got problem with the allocation and deallocation. I don't know if it's a real bug or not, I would like to know what the libstdc++ team has to say about this. – Zarf

@user2485710, I understand. But strictly speaking f and _f bar are not the same object. Once the std::move completes, there should be no guts in f to destruct. Or am I mistaken? – Frum

@Frum In a nutshell it depends on how the implementation of unordered_map is designed to work; std::move is a cast, that cast triggers the T&& signature for the constructor, it's all about what happens inside the constructor. My guess is that the case with an empty it's not being handled that well because if you provide even just 1 pair it works. In general you can't say anything for sure when you just have a cast like std::move, the constructor is the big ? here . – Zarf

@DanielFrey but I don't think that this is the main problem here – Zarf

@DanielFrey, reference? I don't think inheriting standard containers is illegal. – Scorpion

@Scorpion Am I mixing something up here? Maybe, can't find a reference for this right now, so I deleted the comment. – Newsreel

The problem is also related to std::move as removing that fixes the problem. For what it's worth, Two objects destructing is expected though the content of the one that was moved is in an undetermined state. Accessing it after a move leads to undefined behavior. – Amaleta

@Amaleta I recall seeing a bug report, and I think Foo f = {} is the culprit. Here it is. It's probably unrelated, though. If so, ignore my comment. – Ehrsam

@user2485710, so, regardless of how the move constructor is implemented, this is necessarily a bug in std::unordered_map. – Frum

@Frum I'm not sure that's necessarily true... If just adding a ~Foo() = default; fixes the problem, It very well could be the compiler driver. – Amaleta

For what it's worth, I've placed a bug report and it's currently under investigation: gcc.gnu.org/bugzilla/show_bug.cgi?id=59713 – Amaleta

I'm not sure why I first thought it was with libstdc++, but after realizing that just adding that explicitly defined default destructor fixed it, I couldn't imagine it being the library. I have to agree, I don't think it has to do with unordered_map. See this and this. I may be wrong though. – Ehrsam

Answer 1

Update

It appears a fix for the problem has been checked in.

---

Interesting question. It definitely seems to be a bug with how GCC handles = {} initialized default arguments, which was a late addition to the standard. The problem can be reproduced with a pretty simple class in place of std::unordered_map<int,int>:

#include <utility>

struct PtrClass
{
    int *p = nullptr;
 
    PtrClass()
    {
        p = new int;
    }

    PtrClass(PtrClass&& rhs) : p(rhs.p)
    {
        rhs.p = nullptr;
    }

    ~PtrClass()
    {
        delete p;
    }
};

void DefArgFunc(PtrClass x = {})
{
    PtrClass x2{std::move(x)};
}

int main()
{
    DefArgFunc();
    return 0;
}

Compiled with g++ (Ubuntu 4.8.1-2ubuntu1~12.04) 4.8.1, it displays the same problem:

*** glibc detected *** ./a.out: double free or corruption (fasttop): 0x0000000001aa9010 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7fc2cd196b96]
./a.out[0x400721]
./a.out[0x4006ac]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7fc2cd13976d]
./a.out[0x400559]
======= Memory map: ========
bash: line 7:  2916 Aborted                 (core dumped) ./a.out

Digging a little deeper, GCC seems to create an extra object (though it only calls the constructor and destructor once each) when you use this syntax:

#include <utility>
#include <iostream>

struct SimpleClass
{    
    SimpleClass()
    {
        std::cout << "In constructor: " << this << std::endl;
    }

    ~SimpleClass()
    {
        std::cout  << "In destructor: " << this << std::endl;
    }
};

void DefArgFunc(SimpleClass x = {})
{
        std::cout << "In DefArgFunc: " << &x << std::endl;
}

int main()
{
    DefArgFunc();
    return 0;
}

Output:

In constructor: 0x7fffbf873ebf
In DefArgFunc: 0x7fffbf873ea0
In destructor: 0x7fffbf873ebf

Changing the default argument from SimpleClass x = {} to SimpleClass x = SimpleClass{} produces

In constructor: 0x7fffdde483bf
In DefArgFunc: 0x7fffdde483bf
In destructor: 0x7fffdde483bf

as expected.

What seems to be happening is that an object is created, the default constructor is called, and then something similar to a memcpy is performed. This "ghost object" is what is passed to the move constructor and modified. However, the destructor is called on the original, unmodified, object, which now shares some pointer with the move-constructed object. Both eventually try to free it, causing the issue.

The four changes that you noticed fixed the problem make sense given the above explanation:

// 1
// adding the destructor inhibits the compiler generated move constructor for Foo,
// so the copy constructor is called instead and the moved-to object gets a new
// pointer that it doesn't share with the "ghost object", hence no double-free
~Foo() = default;

// 2
// No  `= {}` default argument, GCC bug isn't triggered, no "ghost object"
Bar(Foo f = Foo()) : _f(std::move(f)) {}

// 3
// The copy constructor is called instead of the move constructor
Bar(Foo f = {}) : _f(f) {}

// 4
// No  `= {}` default argument, GCC bug isn't triggered, no "ghost object"
Foo f1 = {};
Foo f2 = std::move(f1);

Passing an argument to the constructor (Bar b(Foo{});) rather than using the default argument also solves the problem.

Margertmargery

Yep, it's definitely a bug that persists in gcc 4.9 (or at least the snapshot I have.) clang doesn't have the same bug. There already is a bug report and a couple of others that seem similar. But +1 for clarifying the issue with the test case. The problem goes away if you do SimpleClass x = SimpleClass{}. – Ehrsam

Yeah, sorry, I see the full comment chain on the question now. I hadn't read through all the hidden comments before. Oh well, I found it an interesting exercise anyway :) – Margertmargery

@remyabel I tried several different default arguments, only = {...} triggered the bug. I have to guess it's related to the late addition, but it's still strange because it should act exactly the same as SimpleClass x = SimpleClass{}, it's only a grammar change. The output when &x == 0x7fffdde483bf was from code that used that syntax. – Margertmargery

@Margertmargery I think the semantics of = SimpleClass{} and = {} are different. The first is default initialization in the case of non-POD types and just about the same as SimpleClass a; memset(a,0,sizeof(a)); for POD types whereas Simpleclass a = {} is construction via std::initializer_list for non-POD types and aggregate initialization for POD types. I don't really ever use the = SimpleClass{} form though, so I might be wrong. – Amaleta