Search code examples
c++memorygoogletestraii

RAII memory corruption in Google Test


I've implemented an automatical deleter for C-pointers. The code works in a test program, but when I use the code in Google Test, the strange things happen. I can't understand why. Have I written with undefined behaviour? Or does Google Test interfere somehow?

The code below, if the macro ASSERT_THAT is commented-out, prints:

i1 = 0x8050cf0
i2 = 0x8050d00
got: 0x8050cf0
got: 0x8050d00
go delete: 0x8050cf0
go delete: 0x8050d00

Two pointers are created, the guard gets these pointers and later deletes them. So far exactly as desired.

If the macro is active, the result is:

i1 = 0x8054cf0
i2 = 0x8054d00
got: 0x8054cf0
got: 0x8054d00
go delete: 0x8054c01

For some reason, the code deletes another pointer then one that got. I'm totally confused. Can you help spot the problem?

#include <iostream>
#include <gmock/gmock.h>

using namespace testing;

class Scope_Guard {
public:
  Scope_Guard(std::initializer_list<int*> vals)
    : vals_(vals)
    {
    for (auto ptr: vals_) {
      std::cerr << "got: " << ptr << std::endl;
    }
  }
  ~Scope_Guard() {
    for (auto ptr: vals_) {
      std::cerr << "go delete: " << ptr << std::endl;
      delete ptr;
    }
  }
  Scope_Guard(Scope_Guard const& rhs) = delete;
  Scope_Guard& operator=(Scope_Guard rhs) = delete;
private:
  std::initializer_list<int*> vals_;
};

TEST(Memory, GuardWorksInt) {
  int* i1 = new int(1);
  int* i2 = new int(2);
  std::cerr << "i1 = " << i1 << std::endl;
  std::cerr << "i2 = " << i2 << std::endl;
  Scope_Guard g{i1, i2};

  ASSERT_THAT(1, Eq(1)); // (*)
}

int main(int argc, char** argv) {
  InitGoogleTest(&argc, argv);
  return RUN_ALL_TESTS();
}

Solution

  • It is undefined behavior:

    You are copying a std::initializer_list from the constructor argument into a class member.

    Copying a std::initializer_list does not copy its underlying elements. Therefore after leaving the constructor, there is no guarantee that vals_ contains anything valid anymore.

    Use a std::vector for the member instead and construct it from the initializer list.

    I am not sure about your intentions with this guard, but it would probably be easier to just use std::unique_ptr.