ODR violation in GCC 6.3.0 with types defined in two separate translation units

Question

We are seeing some strange behavior in GCC with the following code example. The strange behavior is ODR violation in GCC 6.3.0 with types defined in two separate translation units. It is possibly related to recursive type definitions or incomplete types.

We are unsure whether our code is valid, or if we are depending on undefined behavior in the way our types are recursively defined. Please see how the variant-like Dynamic class template is defined and instantiated in two separate cpp files.

dynamic_test.h:

#pragma once

#include <algorithm>
#include <type_traits>

namespace dynamic
{
    template <class T>
    void erasure_destroy( const void *p )
    {
        reinterpret_cast<const T*>( p )->~T();
    }

    template <class T>
    void erasure_copy( void *pDest, const void *pSrc )
    {
        ::new( pDest ) T( *reinterpret_cast<const T*>( pSrc ) );
    }

    template <class T>
    struct TypeArg {};

    struct ErasureFuncs
    {
        template <class T = ErasureFuncs>
        ErasureFuncs( TypeArg<T> t = TypeArg<T>() ) :
            pDestroy( &erasure_destroy<T> ),
            pCopy( &erasure_copy<T> )
        {
            (void)t;
        }

        std::add_pointer_t<void( const void* )> pDestroy;
        std::add_pointer_t<void( void*, const void* )> pCopy;
    };

    enum class TypeValue
    {
        Null,
        Number,
        Vector
    };

    template <typename T>
    using unqual = std::remove_cv_t<std::remove_reference_t<T>>;

    template <class Base, class Derived>
    using disable_if_same_or_derived = std::enable_if_t<!std::is_base_of<Base, unqual<Derived>>::value>;

    template <template <class> class TypesT>
    struct Dynamic
    {
        using Types = TypesT<Dynamic>;

        using Null = typename Types::Null;
        using Number = typename Types::Number;
        using Vector = typename Types::Vector;

        Dynamic()
        {
            construct<Null>( nullptr );
        }

        ~Dynamic()
        {
            m_erasureFuncs.pDestroy( &m_data );
        }

        Dynamic( const Dynamic &d ) :
            m_typeValue( d.m_typeValue ),
            m_erasureFuncs( d.m_erasureFuncs )
        {
            m_erasureFuncs.pCopy( &m_data, &d.m_data );
        }

        Dynamic( Dynamic &&d ) = delete;

        template <class T, class = disable_if_same_or_derived<Dynamic, T>>
        Dynamic( T &&value )
        {
            construct<unqual<T>>( std::forward<T>( value ) );
        }

        Dynamic &operator=( const Dynamic &d ) = delete;
        Dynamic &operator=( Dynamic &&d ) = delete;

    private:
        static TypeValue to_type_value( TypeArg<Null> )
        {
            return TypeValue::Null;
        }

        static TypeValue to_type_value( TypeArg<Number> )
        {
            return TypeValue::Number;
        }

        static TypeValue to_type_value( TypeArg<Vector> )
        {
            return TypeValue::Vector;
        }

        template <class T, class...Args>
        void construct( Args&&...args )
        {
            m_typeValue = to_type_value( TypeArg<T>() );
            m_erasureFuncs = TypeArg<T>();
            new ( &m_data ) T( std::forward<Args>( args )... );
        }

    private:
        TypeValue m_typeValue;
        ErasureFuncs m_erasureFuncs;
        std::aligned_union_t<0, Null, Number, Vector> m_data;
    };
}

void test1();
void test2();

dynamic_test_1.cpp:

#include "dynamic_test.h"

#include <vector>

namespace
{
    template <class DynamicType>
    struct Types
    {
        using Null = std::nullptr_t;
        using Number = long double;
        using Vector = std::vector<DynamicType>;
    };

    using D = dynamic::Dynamic<Types>;
}

void test1()
{
    D::Vector v1;
    v1.emplace_back( D::Number( 0 ) );
}

dynamic_test_2.cpp:

#include "dynamic_test.h"

#include <vector>

namespace
{
    template <class DynamicType>
    struct Types
    {
        using Null = std::nullptr_t;
        using Number = double;
        using Vector = std::vector<DynamicType>;
    };

    using D = dynamic::Dynamic<Types>;
}

void test2()
{
    D::Vector v1;
    v1.emplace_back( D::Number( 0 ) );
}

main.cpp:

#include "dynamic_test.h"

int main( int, char* const [] )
{
    test1();
    test2();
    return 0;
}

Running this code causes a SIGSEGV with the following stack trace:

1 ??                                                                                                                                     0x1fa51  
2 dynamic::Dynamic<(anonymous namespace)::Types>::~Dynamic                                                        dynamic_test.h     66  0x40152b 
3 std::_Destroy<dynamic::Dynamic<(anonymous namespace)::Types>>                                                   stl_construct.h    93  0x4013c1 
4 std::_Destroy_aux<false>::__destroy<dynamic::Dynamic<(anonymous namespace)::Types> *>                           stl_construct.h    103 0x40126b 
5 std::_Destroy<dynamic::Dynamic<(anonymous namespace)::Types> *>                                                 stl_construct.h    126 0x400fa8 
6 std::_Destroy<dynamic::Dynamic<(anonymous namespace)::Types> *, dynamic::Dynamic<(anonymous namespace)::Types>> stl_construct.h    151 0x400cd1 
7 std::vector<dynamic::Dynamic<(anonymous namespace)::Types>>::~vector                                            stl_vector.h       426 0x400b75 
8 test2                                                                                                           dynamic_test_2.cpp 20  0x401796 
9 main                                                                                                            main.cpp           6   0x400a9f 

It's odd that constructing a Vector takes us directly to a destructor.

What is very strange, is that these errors go away when we do the following:

1. Rename "Types" in one of the cpp files, so that they do not use the same name for the class template.
2. Make the implementation of "Types" the same in each cpp file (change Number to double in each file).
3. Do not push a Number to a Vector.
4. Change the implementation of Dynamic to not use this recursive type definition style.

Here is a trimmed down example of an implementation which does work:

template <class Types>
struct Dynamic
{
    using Null = typename Types::Null;
    using Number = typename Types::Number;
    using Vector = typename Types::template Vector<Dynamic>;

...

    struct Types
{
    using Null = std::nullptr_t;
    using Number = long double;

    template <class DynamicType>
    using Vector = std::vector<DynamicType>;
};

We also see some warnings related to ODR violation when we compile with link time optimization (LTO):

dynamic_test.h:51: warning: type ‘struct Dynamic’ violates the C++ One Definition Rule [-Wodr]
struct Dynamic
         ^

Does anyone have some insight into what may be causing this issue?

Answer 1

Okay, it took me a while of playing with this on and off, but I finally got a very simple duplicate that gets at the heart of the matter. First, consider test1.cpp:

#include "header.h"

#include <iostream>

namespace {

template <class T>
struct Foo {
   static int foo() { return 1; };
};

using D = Bar<Foo>;

}

void test1() {
    std::cerr << "Test1: " << D::foo() << "\n";
}

Now, test2.cpp is exactly identical to this, except that Foo::foo returns 2, and the function declared at the bottom is called test2 and prints Test2: and so on. Next, header.h:

template <template <class> class TT>
struct Bar {
    using type = TT<Bar>;

    static int foo() { return type::foo(); }
};


void test1();
void test2();

Finally, main.x.cpp:

#include "header.h"

int main() {
    test1();
    test2();
    return 0;
}

You may be surprised to learn that this program prints:

Test1: 1
Test2: 1

Of course, that's only because I compile with:

g++ -std=c++14 main.x.cpp test1.cpp test2.cpp

If I reverse the order of the last two files, they both print 2.

What's happening is that the linker ends up using the first definition it encounters of Foo everywhere it's needed. Hmm, but we defined Foo in an anonymous namespace, which should give it internal linkage, avoiding this issue. So we compile just one TU and then use nm on it:

g++ -std=c++14 -c test1.cpp
nm -C test1.o

This yields the following:

                 U __cxa_atexit
                 U __dso_handle
0000000000000087 t _GLOBAL__sub_I__Z5test1v
0000000000000049 t __static_initialization_and_destruction_0(int, int)
0000000000000000 T test1()
000000000000003e t (anonymous namespace)::Foo<Bar<(anonymous namespace)::Foo> >::foo()
0000000000000000 W Bar<(anonymous namespace)::Foo>::foo()
                 U std::ostream::operator<<(int)
                 U std::ios_base::Init::Init()
                 U std::ios_base::Init::~Init()
                 U std::cerr
0000000000000000 r std::piecewise_construct
0000000000000000 b std::__ioinit
                 U std::basic_ostream<char, std::char_traits<char> >& std::operator<< <std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*)

Don't worry about the letters for now, other than upper case versus lower case. Lower case symbols are private, the way we expect internal linkage symbols to be. Upper case symbols are public, and have external linkage and are exposed to the linker.

What's interesting, and not really that surprising, is that while Foo may have internal linkage, Bar does not! The first translation unit already defines the symbol Bar<Foo> with external linkage. The second translation unit does the same. So when the linker links them, it sees two translation units trying to define the same symbol with external linkage. Note that it's an inline defined class member, so it's implicitly inline. So the linker handles this as it always does: it just silently drops all the definitions it runs into after the first (because the symbol is already defined; that's how the linker works, left to right).So Foo is correctly defined in each TU, but Bar<Foo> isn't.

Bottom line is that this is an ODR violation. You'll want to rethink some stuff.

Edit: it seems in fact like this is a bug in gcc. The standard's wording implies that the Foos should be treated uniquely in this situation, and thus the Bar templated on each Foo should be separate. Link to bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70413.