std::vector::emplace_back bug when returning references (C++17)

user_4656131 0
Viewed 969

I've been trying to trace a bug for 10+ hours now, and by now I'm starting to think that the bug can't be on my side. However, I have a feeling it could be me who's just forgetting or misunderstanding something.

I have a class member of type std::vector called temp_materials, and inside the constructor (when temp_materials is still empty), this code runs:

    Material &stonewallbug = temp_materials.emplace_back(resource_lib.get_shader("DeferredGeometryShader"));
    stonewallbug.set_texture("texture_diffuse1", resource_lib.get_texture("StonewallDiffuse"));
    stonewallbug.set_texture("texture_specular1", resource_lib.get_texture("StonewallSpecular"));

    Material &containerbug = temp_materials.emplace_back(resource_lib.get_shader("DeferredGeometryShader"));
    containerbug.set_texture("texture_diffuse1", resource_lib.get_texture("ContainerDiffuse"));
    containerbug.set_texture("texture_specular1", resource_lib.get_texture("ContainerSpecular"));

    Material stonewall1 = temp_materials[0];
    Material container1 = temp_materials[1];

    Material stonewall2 = stonewallbug;
    Material container2 = containerbug;

If nothing goes wrong during the copying, stonewall1's contents should be equal to stonewall2, as should container1 to container2, correct?

However, I then insert all of them into a simple vector which's contents will be rendered later on:

    // Using temp_materials[0]
    auto stonewall1node = SceneNode(stonewall1, resource_lib.get_mesh("Cube"));
    stonewall1node.set_transform(glm::translate(glm::mat4(1.0f), glm::vec3(0.0f, 0.0f, 0.0f)));
    m_scene_list.push_back(stonewall1node);

    // Using temp_materials[1]
    auto container1node = SceneNode(container1, resource_lib.get_mesh("Cube"));
    container1node.set_transform(glm::translate(glm::mat4(1.0f), glm::vec3(0.0f, 0.0f, 1.0f)));
    m_scene_list.push_back(container1node);

    // Using stonewallbug
    auto stonewall2node = SceneNode(stonewall2, resource_lib.get_mesh("Cube"));
    stonewall2node.set_transform(glm::translate(glm::mat4(1.0f), glm::vec3(0.0f, 1.0f, 0.0f)));
    m_scene_list.push_back(stonewall2node);

    // Using containerbug
    auto container2node = SceneNode(container2, resource_lib.get_mesh("Cube"));
    container2node.set_transform(glm::translate(glm::mat4(1.0f), glm::vec3(0.0f, 1.0f, 1.0f)));
    m_scene_list.push_back(container2node);

Now, I'd expect there to be 2 "container" cubes stacked on each other, and 2 "stonewall" cubes stacked on each other, but this is the result I'm getting:

Result1

If I however move the line Material stonewall2 = stonewallbug; to between the creation of stonewallbug and containerbug, I get the result I expect.

Noticing this, I made a very, very simple C++ program:

#include <iostream>
#include <vector>
#include <string>


int main() {
    std::vector<std::string> strings;

    std::string& ref1 = strings.emplace_back("1");
    std::string& ref2 = strings.emplace_back("2");

    std::cout << "ref1: " << ref1 << std::endl;
    std::cout << "ref2: " << ref2 << std::endl;

    std::cout << "strings[0]: " << strings[0] << std::endl;
    std::cout << "strings[1]: " << strings[1] << std::endl;


    return 0;
}

And the result when running this is a lot of nonsense characters. However, if I output ref1 before ref2 is emplaced, it outputs the expected results. On here it says that vector's emplace_back is supposed to return a reference to the inserted element, but to me it doesn't seem like it's working like it should.

Am I misunderstanding something, or is this just a very obvious bug in g++ 7.1?

Edit: I can't believe I spent so many hours on such an obvious thing... :)

c++17 g++ c++ debugging
1 Answers

You have to be careful with references and iterators to container elements. Various mutating container operations invalidate references and iterators. The details vary by container and operation, but for std::vector, all inserting operations (such as push_back and emplace_back), as well as erasure from anything other than the end, invalidate both references and iterators in general (but see below for ways to avoid this).

Therefore, in the following code,

std::vector<T> v;

T & a = v.emplace_back(a, b, c);
T & b = v.emplace_back(x, y, z);

the second call of emplace_back invalidates the reference a. (And accessing an object through an invalid reference has undefined behaviour.)

To avoid invalidation in a std::vector, use the reserve and capacity facilities:

std::vector<T> v;
v.reserve(2);
T & a = v.emplace_back(a, b, c);
T & b = v.emplace_back(x, y, z);

An insertion at the end does not invalidate if the new size is less than or equal to the current capacity.

user_5967810
Related