Segfault with std::vector =-operation to uninitialized space

Question

I get segmentation faults when I use the =-operator to copy a struct that contains a std::vector to uninitialized memory. The critical code looks like that:

template<typename T>
ComponentContainer
{
  T* buffer;
  size_t capacity;
  size_t m_size;

public:

  ComponentContainer();
  ~ComponentContainer();
  size_t size();
  void resize(size_t size);
  T & operator[](size_t index);
};

template<typename T>
void ComponentContainer<T>::resize(size_t newSize)
{
  if(this->m_size >= newSize)
  {
    this->m_size = newSize;
  }
  else
  {
    if(this->capacity < newSize)
    {
      const size_t newCapacity = capacity*2;
      T* newBuffer = (T*)malloc(newCapacity*sizeof(T));
      for(size_t i = 0; i<m_size; i++)
      {
        // checks if this->buffer[i] is valid intialized memory
        if(pseudo_checkIfElementIsInitialized(i))
        {
          // when this is uncommented no segfault happens
          //new (&newBuffer[i]) T(); 
          newBuffer[i] = this->buffer[i]; // <- segfault happens here 
        }
      }
      this->capacity = newCapacity;
      free(this->buffer);
      this->buffer = newBuffer;
    }
    this->m_size = newSize;
  }
}

The T-type is a struct with a std::vector of structs when I get the segfault. I suspect that the std::vector =-operator uses somehow the left side variable newBuffer[i] and the segmentation fault happens since newBuffer[i] is not initialized.

Objects will be created only with in-placement new with the function T & operator[](size_t index). The malloc should only allocate the memory without initializing anything.

I tried to write a simple example but that hasn't worked out so well:

#include <iostream>
#include <vector>


struct Hello
{
    Hello()
    {
        std::cout << "constructor" << std::endl;
    }
    ~Hello()
    {
        std::cout << "destructor" << std::endl;
    }
    std::vector<double> v = std::vector<double>(1);
};


int main()
{
    Hello* buffer = (Hello*)malloc(1*sizeof(Hello));
    char* noise = (char*)buffer;
    for(size_t i = 0; i<sizeof(Hello); i++)
    {
        noise[i] = 100;
    }
    auto tmp = Hello();
    tmp.v[0] = 6.6;
    //new (&buffer[0]) Hello();
    buffer[0] = tmp;
    std::cout << buffer[0].v[0] << std::endl;

    return 0;
}

It works fine without segfault. I assume that is because the uninitialized memory was just by chance ok for the std::vector =-operation.

So

a) is that theory correct

and if yes

b) how to solve this problem without using a default constructor (T()) for every class that i use as T for my ComponentContainer

Why are you using `malloc` like this in a C++ program? You are not creating any objects when you use `malloc` -- all you're doing is allocating bytes. No wonder it seg faults. — PaulMcKenzie, Sep 11 '18 at 01:23
`// when this is uncommented no segfault happens` -- `new (&newBuffer[i]) T();` -- Well, that's what [placement-new](https://stackoverflow.com/questions/222557/what-uses-are-there-for-placement-new) does -- creates objects. Why did you think faking what placement-new is doing would turn out well? — PaulMcKenzie, Sep 11 '18 at 01:28
@PaulMcKenzie because some structs I use don't have default constructors. — Darius Duesentrieb, Sep 11 '18 at 01:29
*because some structs I use don't have default constructors* -- That is more of a reason to *not* use `malloc`. Other than `placement-new`, where did you get the idea you can use `malloc` to create non-POD objects in C++? This is just a fundamental of C++, and that is you don't use `malloc`, `calloc`, etc. to create C++ objects, especially ones that are created from non-POD types. — PaulMcKenzie, Sep 11 '18 at 01:32
I don't want to create nonPOD objects with malloc, that malloc is only to reserve memory. Objects will be created by in-placement new with the `T & operator[](size_t index)` function. — Darius Duesentrieb, Sep 11 '18 at 01:38
But you _are not_ creating objects with the placement new commented out. — tkausl, Sep 11 '18 at 01:41
@DariusDuesentrieb Also, why not just use `std::vector` outright instead of your `ComponentContainer`? Almost every, if not every implementation of `std::vector` uses placement-new when reserving memory for the entries. Seems like you're trying to reinvent the wheel. — PaulMcKenzie, Sep 11 '18 at 01:43
To use std::vector I would need a default constructor for every possible `T`-type. I would like to avoid this. — Darius Duesentrieb, Sep 11 '18 at 01:46
[No you don't?](http://coliru.stacked-crooked.com/a/71d41dcd5a7ad4f5) — tkausl, Sep 11 '18 at 01:50
Yes, but your code doesn't resize either. Its more like an reserve, even though you're changing the size. You've got a bunch of unconstructed objects since you never default-construct them. And you are not allowed to use them before you construct them. So, just use vector, use reserve and push_back/emplace_back. — tkausl, Sep 11 '18 at 02:01
@DariusDuesentrieb -- That is what `vector::reserve()` is for. Then you can just emplace_back() in a loop. [Similar to this](http://coliru.stacked-crooked.com/a/806fbe011b233b4c). — PaulMcKenzie, Sep 11 '18 at 02:01
I work on an entity-component library, for every component type I have an array. An entity is essentially a int that is the index to the components that belong to that entity. So for example, An entity has index 4 and a float and a char component are attached to that entity. When i want to access the char component, i just look here: component[4]. That means there might be some points in a component-array that dont belong to an entity, because the entity that has the same index (4) does not have this type (short) attached to it. That means i need to resize component-arrays. — Darius Duesentrieb, Sep 11 '18 at 02:11
if you want it to work this way, you have to make sure `pseudo_checkIfElementIsInitialized` does the right thing. `=` on uninitialized memory just won't work. In your sample, `buffer[0] = tmp;` exhibits undefined behavior because `buffer[0]` hasn't been constructed. — kmdreko, Sep 11 '18 at 02:35
You need to post an [mcve] You are in no position to determine what code is "critical code" and what is not. — xaxxon, Sep 11 '18 at 02:47
@xaxxon updated minimal example. Also here: http://coliru.stacked-crooked.com/a/067d676267152698 — Darius Duesentrieb, Sep 11 '18 at 12:26
@kmdreko where is that specified? I assume that is not correct for POD types. If it is indeed undefined for non-POD types then you answered the first part of my question, thank you. newBuffer[i] should be not initialized, `pseudo_checkIfElementIsInitialized` just checks, if the right side (`this->buffer[i]`) has been constructed. — Darius Duesentrieb, Sep 11 '18 at 12:28
@tkausl std::vector::reserve also needs a default constructor — Darius Duesentrieb, Sep 11 '18 at 12:35
@DariusDuesentrieb That's not true. For `reserve`, `T` need only be _MoveInsertable_. — Lightness Races in Orbit, Sep 11 '18 at 12:35

Lightness Races in Orbit · Accepted Answer · 2018-09-11T16:53:40.450

Well, yeah. You can't assign to an object that doesn't exist.

Uncomment the line that fixes it!

If you can't default construct, then copy construct:

new (&newBuffer[i]) T(this->buffer[i]);

And if you can't do that, then, well, you know the rest.

The malloc should only allocate the memory without initializing anything.

Is it possible that you've underestimated the weight of this statement? You don't just get memory then decide whether or not to initialise it with some values. You have to actually create objects before using them; this is not optional. You're programming C++, not manipulating bits and bytes on a tape :)

Segfault with std::vector =-operation to uninitialized space

1 Answers1