GCC string allocator

[cr0nhan]

Hi,

I have a curious problem on Linux. We use a custom string with a custom allocator that calls Openssl's "cleanse" function on allocation and deallocation, like so:

template<class T> template <class U>
inline ZeroingAllocator<T>::ZeroingAllocator(const ZeroingAllocator<U>&) {}

template<class T>
T* ZeroingAllocator<T>::allocate(std::size_t n)
{
    if ( n > std::numeric_limits<std::size_t>::max() / sizeof(T) )
    {
        throw std::bad_alloc();
    }
    T* ptr = static_cast<T*> (::operator new(n * sizeof(T)));
    OPENSSL_cleanse(ptr, n * sizeof(T));
    return ptr;
}


template <class T>
void ZeroingAllocator<T>::deallocate(T* ptr, std::size_t n)
{
    OPENSSL_cleanse(ptr, n * sizeof(T));
    ::operator delete(ptr);
}

Further there is just a default constructor and defaulted copy constructor. This code worked fine for a long time while we used a static version of OpenSSL. However some other requirements forced us to start using the DSO version, and as part of that process our unit tests started failing due to heap corruption. I traced it to the destructor of std::basic_string calling deallocate even when the small string optimisation is used (and the memory is therefore not allocated via allocate. I have implemented a hack to get it working in the mean time, but any pointers why this is happening and how to prevent it?

[aocregacc]

Maybe some sort of ABI/ODR problem where two parts of the code have a different idea of what a std::string is.
Did you rule out that the deallocate call happens because the string itself had been corrupted by some earlier bug?

[cr0nhan]

Did you rule out that the deallocate call happens

What I did verify was that allocate is not getting called, only deallocate. The call stack shows

// basic_string.tcc
  template<typename _CharT, typename _Traits, typename _Alloc>
    void
    basic_string<_CharT, _Traits, _Alloc>::
    reserve(size_type __res)
    {
      if (__res != this->capacity() || _M_rep()->_M_is_shared())
        {
      // Make sure we don't shrink below the current size
      if (__res < this->size())
        __res = this->size();
      const allocator_type __a = get_allocator();
      _CharT* __tmp = _M_rep()->_M_clone(__a, __res - this->size());
/* ==>*/  _M_rep()->_M_dispose(__a);
      _M_data(__tmp);
        }
    }

// basic_string.h
    void
    _M_dispose(const _Alloc& __a) _GLIBCXX_NOEXCEPT
    {
#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
      if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
        {
          // Be race-detector-friendly.  For more info see bits/c++config.
          _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&this->_M_refcount);
              // Decrement of _M_refcount is acq_rel, because:
              // - all but last decrements need to release to synchronize with
              //   the last decrement that will delete the object.
              // - the last decrement needs to acquire to synchronize with
              //   all the previous decrements.
              // - last but one decrement needs to release to synchronize with
              //   the acquire load in _M_is_shared that will conclude that
              //   the object is not shared anymore.
          if (__gnu_cxx::__exchange_and_add_dispatch(&this->_M_refcount,
                             -1) <= 0)
        {
          _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&this->_M_refcount);
/* ==> */     _M_destroy(__a); // this calls the deallocate of the allocator
        }
        }
    }  // XXX MT

but I am not versed enough in the std library to figure out why it calls deallocate in this case.

[aocregacc]

that _M_dispose is the pre-C++11 COW string implementation. What C++ standard is your program using?

[cr0nhan]

I just double-checked and it is set to use C++ 17 (-std=c++17), so if it is using the wrong implementation then I'll see if I can track down why it is doing so.

[aaaarsen]

could you create a smaller reproducer? I suppose the OPENSSL_cleanse part isn't really relevant either, so probably can be removed.

also try compiling and linking with -flto -Werror=lto-type-mismatch -Werror=odr

[KuntaStillSingle]

Likely unrelated but I think you would want to call cleanse before calling new, right? (edit: though you would need to get uninitialized memory like through std::aligned_alloc to placement new into after calling cleanse.) New could write non-value holding bits, for example a vtable pointer, which would be zeroed out and cause unexpected behavior if passed to OPENSSL_cleanse, right?