0

The following code produces a segmentation fault upon assignment to a union member. This must be a result of misunderstanding unions, but I cannot pinpoint my error.

Q1: Is it necessary to include some initialization for the appropriate union member variables in T1's constructor?

I have tried something similar to the suggested solution (found in code block below) to initialize the appropriate member, but no success.

Q2: If T1 initialization is OK, why does assigning to this location produce a segmentation fault?

Relevant Code:

struct T1
{
    T1(int type)
    {
        this->type = type;
        //Q1 : need to init appropriate union struct here also? Something like:
        /*
        *if (type == 1)  
        *{
        *    union_member1.str = "";
        *}
        *else
        *{
        *    union_member2.value = 0;
        *}
        */
        //If so, better alternative to this?
    }

    int type;           //tells which union struct is used
    
    union               
    {
        struct              //type 1
        {
            std::string str;
        } union_member1;

        struct              //type 2
        {
            int value;
        } union_member2;
    };
};

struct T2
{
    bool used;
    /*
    * other variables here
    */
}; 

std::map<std::string, T2> mymap;
T1* global_ptr;

int main()
{
    for (auto& map_entry : mymap)
    {
        if (!(map_entry.second.used))
        {
            global_ptr = new T1(1);                                 
            global_ptr->union_member1.str = map_entry.first;        //Q2: SEG fault occurs with this assignment
            std::string s = map_entry.first;                        //    but not with this assignment
        }
    }
}

GDB Output

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b70b03 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_assign(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6

Obviously, this code is shaved down for readability reasons. Map entries have been thoroughly validated, as well.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
jallen
  • 43
  • 4
  • `std::string`s are too complicated to store in a `union` unless you do a lot of extra work. `string` lives and dies by its constructor, destructor, and special member functions and by default a `union` respects none of these. Do you have access to C++17 and [`std::variant`](https://en.cppreference.com/w/cpp/utility/variant)? – user4581301 Dec 07 '20 at 23:12
  • [This answer is a bit old](https://stackoverflow.com/a/3521998/4581301), but it explains the problem well and offers several potential solutions. – user4581301 Dec 07 '20 at 23:15
  • 1
    If anyone has a better/more up-to-date duplicate, please propose it. I'm not comfortable closing this question with a pre-C++11 duplicate that says it can't be done at all. – user4581301 Dec 07 '20 at 23:21
  • first question ive ever asked on here; you pointed to the problem in like 3 minutes. thanks a ton for the help! I am locked in c++11, however. The use of a union was to be fancy, so I was able to move things around just a bit and no more SEG fault. I would also be interested to see a more up to date solution to this. – jallen Dec 07 '20 at 23:25
  • With only C++11 at your disposal you can use [`boost::variant`](https://www.boost.org/doc/libs/1_74_0/doc/html/variant.html), or more likely this is some sort of assignment where you need to develop the solution yourself. In this case the keyword to search for is "tagged union". See the `struct TU` in [the answer I linked above](https://stackoverflow.com/a/3521998/4581301). It shows how to set up everything except safe copying of the union. – user4581301 Dec 08 '20 at 01:12
  • Why does this segfault? No constructor is run for `union_member1`. Which means no constructor is run for `str`. `str` is just a block of uninitialized memory. It's lifetime as an object hasn't started yet. `str`'s internal buffer pointer points Zeus knows where and it's length is an uninitialized book-keeping variable containing Thor knows what value. `global_ptr->union_member1.str = map_entry.first` tries to use this horribly damaged not-yet-a-string and promptly dies for any number of perfectly valid reasons, such as trying to write into invalid memory the bad buffer pointer points at. – user4581301 Dec 08 '20 at 01:20

1 Answers1

9

https://eel.is/c++draft/class.union#general-6

says that you can change the active member of a union via u.x = ... only if that assignment is a trivial assignment operator. std::string assignment operator is not trivial, and therefore the note applies

https://eel.is/c++draft/class.union#general-7

In cases where the above rule does not apply, the active member of a union can only be changed by the use of a placement new-expression.

If the string were directly a union member, you would do

    if (type == 1)  
    {
        new (&union_member_str) std::string();
    }

but it's a subobject of a union member which is an unnamed struct type, so you instead use placement new to construct the whole struct and that takes care of all its subobjects:

    if (type == 1)  
    {
        using UMT = decltype(union_member1);
        // or
        // typedef decltype(union_member1) UMT;
        new (&union_member1) UMT();
    }

Of course you will also need proper handling in copy-constructor, move-constructor, copy-assignment, move-assignment, and destructor (Rule of 5). So much better to use an existing class such as std::variant.

The corresponding destructor fragment for a direct string member is

    if (type == 1)  
    {
        union_member_str.string::~string();
    }

and for your unnamed struct type

    if (type == 1)  
    {
        using UMT = decltype(union_member1);
        union_member1.UMT::~UMT();
    }

Don't even try to write these without a typedef / type alias.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720