-1

I've browsed topics on this error message: "free(): double free detected in tcache 2" but none covers my case (at least not in the direct way).

The program executes and returns correct values. However, I receive free(): double free detected in tcache 2 after its run. If I remove lines corresponding to char array, marked as //ERROR there's no such issue. Could someone help me to understand what's going on there?

The code:

#include <cstring>
#include <iostream>
#include <iomanip>
#include <fstream>
#include <string>
#include <sstream>
#include <vector>

#pragma pack(push, r1, 1)
struct Header2 {
    char id[15]; // ERROR
    std::vector<int> length = std::vector<int>(2);
    int count;
};
#pragma pack(pop, r1)

int main() {
  Header2 h;
  strcpy(h.id, "xdddd");  // ERROR
  h.length =  {3, 5};
  h.count = 42;

  std::fstream fh2;
  fh2.open("test.txt", std::fstream::out | std::fstream::binary);
  fh2.write((char*)&h, sizeof(Header2));
  fh2.close();

Header2 h3;

  fh2.open("test.txt", std::fstream::in | std::fstream::binary);
  fh2.read((char *) &h3, sizeof(Header2));
  fh2.close();

   if(!fh2.good()) {
      std::cout << "Error occurred at reading time!" << std::endl;
      return 1;
   }

  std::vector<int> out = h3.length;
  std::cout << h3.id << " "  << std::endl; // ERROR
  std::cout << out[0] << "," <<out[1]  << " " << h3.count << std::endl;
  std::cout << "End of program" << std::endl;
  return 0;
}

In the code, it is important that the sizes of struct variables are fixed, because different programs would read output files.

The problem investigated with TIO Run gives additional message: /srv/wrappers/cpp-gcc: line 5: 15547 Aborted (core dumped) ./.bin.tio "$@" < .input.tio)

  • 2
    That double free might be the least of your worries: https://godbolt.org/z/ofqzdKo8e – user4581301 Sep 16 '22 at 22:27
  • 1
    The question asked about stems from the fact that `std::vector` is far too complex a structure to be written to a file with `write`. `vector` is basically a bunch of pointers, and one of them owns a dynamic allocation. When you write the `vector` to the file, the data you've added to the `vector` is not written, the pointers are. When the `vector` is read back out of the file, the "new" `vector` gets the **exact same pointers**. Now you have two `vector`s both owning the same dynamic allocation. Sooner or later both go out of scope and Boom! Double free. Assuming the program holds up that long – user4581301 Sep 16 '22 at 22:31
  • If you exited the program and loaded the `vector` from the file in a future run of the program the pointers written refer to the previous run of the program and almost certainly are no longer valid, so you're doomed either way. You need to [serialize](https://en.wikipedia.org/wiki/Serialization) the `vector`, basically write the stuff in the `vector` to the file in a documented and controlled manner (a protocol) and then when reading back, make a new `vector` that you can read the contents in the file into. – user4581301 Sep 16 '22 at 22:35
  • Since you state *In the code, it is important that the sizes of struct variables are fixed*, could I talk you into using a `std::array` in place of the `vector`? `vector`' nature is dynamic. `array` is not. It'll always be 2 `int`s long and is simple enough to dump to a binary file. – user4581301 Sep 16 '22 at 22:42
  • Side note: `char id[15]; // ERROR` and `strcpy(h.id, "xdddd"); // ERROR` are a red herring. They have nothing to do with the problem other than somehow allow you to not see it. Maybe the program silently crashes without them. – user4581301 Sep 16 '22 at 22:45
  • 1
    Struct packing is a recipe for problems if you don’t know what you’re doing. I would advise just writing a serialize and deserialize function. The compiler will most likely optimize it into a memcpy if it’s safe to do so. – Taekahn Sep 16 '22 at 23:33
  • 1
    And even if you do know what you're doing, you're one change of compiler or version from not knowing what you are doing. Or not doing anything at all. – user4581301 Sep 16 '22 at 23:45

1 Answers1

2

TL;DR version: Use std::array instead of std::vector.

#include <cstring>
#include <iostream>
#include <iomanip>
#include <fstream>
#include <string>
#include <sstream>
#include <array> // change here

#pragma pack(push, r1, 1)
struct Header2 {
    char id[15];
    std::array<int, 2> length = std::array<int, 2>(); // change here
    int count;
};
#pragma pack(pop, r1)

int main() {
  Header2 h;
  strcpy(h.id, "xdddd");  
  h.length =  {3, 5};
  h.count = 42;

  std::fstream fh2;
  fh2.open("test.txt", std::fstream::out | std::fstream::binary);
  fh2.write((char*)&h, sizeof(Header2));
  fh2.close();

Header2 h3;

  fh2.open("test.txt", std::fstream::in | std::fstream::binary);
  fh2.read((char *) &h3, sizeof(Header2));
  fh2.close();

   if(!fh2.good()) {
      std::cout << "Error occurred at reading time!" << std::endl;
      return 1;
   }

  std::array<int, 2> out = h3.length; // change here
  std::cout << h3.id << " "  << std::endl; // ERROR
  std::cout << out[0] << "," <<out[1]  << " " << h3.count << std::endl;
  std::cout << "End of program" << std::endl;
  return 0;
}

Since the question states In the code, it is important that the sizes of struct variables are fixed, std::vector is not the right tool for the job. Vector is dynamic. std::array is not. It'll always be 2 ints long and is simple enough to dump to a binary file.

std::vector, on the other hand, is far too complex a structure to be written to a file with write. vector is basically a pointer to a data buffer owned (What is ownership of resources or pointers?) by the vector and some book-keeping variables tracking the capacity of the buffer and amount of the buffer currently in use (and are usually also pointers). When you write the vector to the file, the data "in" the vector is not written because it's not in the vector. A pointer to it is. The pointer to the data buffer is written to the file. When the vector is read back out of the file, the "new" vector gets the exact same pointer. Now you have two vectors both owning the same dynamic allocation. Sooner or later both go out of scope and Boom! Double free. Assuming the program holds up that long. Usually when the first vector goes out of scope the second vector is left pointing to invalid memory and becomes a bomb that goes off when the vector's deceased data buffer is accessed.

If you wrote the vector and exited the program, you're still doomed. The data's not in the file. When a future run of the program loads the vector, the pointer refers to the memory allocated in the previous run of the program and almost certainly is no longer valid (and, trust me on this, it's actually worse if the memory IS valid). You need to serialize the vector, basically write the stuff in the vector to the file in a documented and controlled manner (a protocol) and then when reading back, make a new vector that you can read the contents in the file into.

But in this case it appears that vector is overkill and std::array is a better fit.

Also watch out when writing files: Not every system (and sometimes not even the same system if different compilers are used for the programs) agree on the size and byte order of an int. Prefer to use the fixed width integers defined in cstddef and write the values in an agreed-upon endian.

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • If it’s fixed to 2 ints, why even use an array? It’s called length, I’m sure there is some sort of context to the two different lengths but without that context I’ll just call them `int length_1; int length_2;` – Taekahn Sep 16 '22 at 23:40
  • 1
    @Taekahn Or use `std::pair`. I claim a combination of laziness and wanting to change as little as possible out from under the asker as my reasoning behind pitching `std::array`. Because I like clean naming, I'd prefer a custom struct over `std::pair`. – user4581301 Sep 16 '22 at 23:43