thomasmoelhave / tpie

Templated Portable I/O Environment
Other
112 stars 24 forks source link

Fix delete[] of invalid pointer in tpie_new #248

Closed Mortal closed 3 years ago

Mortal commented 3 years ago

When invoking tpie_new<T>() with T being a class where the constructor throws an exception, TPIE would do the equivalent of

constexpr int n = std::is_polymorphic_v<T> ? 8 : 0;
uint8_t * p = new uint8_t[n + sizeof(T)];
uint8_t * pp = p + n;
// Try to construct T and catch the thrown exception
delete[] pp;

...which is incorrect when std::is_polymorphic_v is true, i.e. when T is a class with virtual methods, because delete[] must be applied to the original pointer returned from new[] (i.e. p instead of pp).

This is a simple programming error because an "is_polymorphic_v" check was done at allocation time, but not at deallocation time.

Reported by GCC 11.1.0 -Wfree-nonheap-object:

In destructor ‘tpie::allocation_scope_magic<T>::~allocation_scope_magic() [with T = ...(some class with virtual methods)]’,
    inlined from ‘T* tpie::tpie_new(Args&& ...) [with T = ...; Args = {}]’ at .../tpie/memory.h:264:1,
    inlined from ...(user code):
.../tpie/memory.h:229:17: warning: ‘void operator delete [](void*)’ called on pointer ‘<unknown>’ with nonzero offset 8 [-Wfree-nonheap-object]
  229 |                 delete[] reinterpret_cast<uint8_t*>(data);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.../tpie/memory.h: In member function ...(user code):
.../tpie/memory.h:161:23: note: returned from ‘void* operator new [](std::size_t)’
  161 |         uint8_t * x = new uint8_t[sizeof(T)+sizeof(size_t)];
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Mortal commented 3 years ago

The same compiler warning caught a bug in an earlier version of this PR (missing if (pp != nullptr) in __deallocate).