C++ Pointer being freed was not allocated (possibly, an issue with unique_ptr or boost::ublas)

486 Views Asked by At

This is a follow up on one of my previous questions. The issue that I am dealing with is explained in detail in the formulation of the aforementioned question. Unfortunately, I was not able to provide a minimal example that showcases the problem.

In this question, I am making an attempt to redefine the problem and provide a minimal example. The code presented in the example below executes and does what it is supposed to do. However, in a slightly more complex case presented in the previous question, sometimes, it results in the runtime error

dynamic_links(3941,0x7fff749a2310) malloc: *** error for object 0x61636f6c65720054: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

Unfortunately, I was only able to produce the error when the optimisation is set to -O3 (possibly -O2 as well). This creates a problem when using debugging tools. Unfortunately, I was not able to reproduce the problem with no/minimal code optimisation. For a reference, I am using gcc 4.9.1.

In the present question, I would like to understand if the structural design of the class inheritance mechanism that I am using could, potentially, be dangerous from the perspective of the dynamic memory allocation. Please find the code below:

namespace ublas = boost::numeric::ublas;

template<typename TScalarType = double>
using ublasRn = ublas::vector<TScalarType>;

class Base
{
public:
    virtual ~Base(void) = 0;
};
Base::~Base(void){}

template<typename T1, typename T2>
class Composite : public Base
{
protected:
    T1 T1Instance;
    std::unique_ptr<T2> u_T2Instance;
public:
    Composite(){}
    virtual ~Composite(void){}
    const std::type_info& returnT1TypeID(void) const
        {return typeid(T1);}
    const std::type_info& returnT2TypeID(void) const
        {return typeid(T2);}
};

template<typename T2>
class CompositeCT: virtual public Composite<double, T2>
{
public:
    using Composite<double, T2>::Composite;
    virtual ~CompositeCT(void)
        {}
};

template<typename T1>
class CompositeRn: virtual public Composite<T1, ublasRn<double>>
{
public:
    using Composite<T1, ublasRn<double>>::Composite;
    virtual ~CompositeRn(void){}
};

class CompositeCTRn :
    public CompositeCT<ublasRn<double>>,
    public CompositeRn<double>
{
public:
    CompositeCTRn(void):
        Composite<double, ublasRn<double>>(),
        CompositeCT<ublasRn<double>>(),
        CompositeRn<double>()
        {};
};

template<typename T1, typename T2, class TComposite>
class Trajectory: public Base
{
protected:
    std::vector<std::unique_ptr<TComposite>> m_Trajectory;
public:
    Trajectory(void)
        {checkType();}
    void checkType(void) const
        {
            TComposite CI;
            if (
                !(
                    CI.returnT1TypeID() == typeid(T1) &&
                    CI.returnT2TypeID() == typeid(T2)
                    )
                )
            throw std::runtime_error("Error");
        }
    virtual ~Trajectory(void){}
};

int main()
{

    Trajectory<
        double,
        ublasRn<>,
        CompositeCTRn
        > T;

    std::cout << 123 << std::endl;
}

Note. I am using the external library boost::ublas. I believe that it is not unlikely that the problem is related to the dynamic memory allocation mechanism for the ublas objects.

1

There are 1 best solutions below

4
sehe On BEST ANSWER

Well. I'm thinking you're really pushing the type system. I cannot begin to fathom why you would want this egregious type hierarchy:

enter image description here

That's a lot of damage done in ~60 lines of code. I cannot think of a reasonable situation where Liskov Substitution Principle holds for this hierarchy.

I also note that this kind of emphasis on runtime polymorphism does seem to run counter to the design goals of C++ and uBlas in particular.

Especially this line (declaring the center of the diagram, essentially):

class CompositeCTRn : public CompositeCT<ublasRn<double> >, public CompositeRn<double> {

This effectively declares a class with a single virtual base, which is "aliased" via three bases:

using VBase  = Composite<double, ublasRn<double> >;
using CTBase = CompositeCT<ublasRn<double> >;
using RnBase = CompositeRn<double>;

This means there should be only one _i1 and one _i2 member in the type. That means that on all conforming compilers, the following should compile and run without triggering any asserts or causing memory leaks etc.:

class CompositeCTRn : public CompositeCT<ublasRn<double> >, public CompositeRn<double> {
    using VBase  = Composite<double, ublasRn<double> >;
    using CTBase = CompositeCT<ublasRn<double> >;
    using RnBase = CompositeRn<double>;
public:
    CompositeCTRn() : VBase(), CTBase(), RnBase() {
        VBase::_i1 = 1;
        VBase::_i2.reset(new ublasRn<double>(3));

        CTBase::_i1 = 2;
        CTBase::_i2.reset(new ublasRn<double>(3));

        RnBase::_i1 = 3;
        RnBase::_i2.reset(new ublasRn<double>(3));

        assert(3 == VBase::_i1);
        assert(3 == CTBase::_i1);
        assert(3 == RnBase::_i1);

        assert(VBase::_i2.get() == CTBase::_i2.get());
        assert(VBase::_i2.get() == RnBase::_i2.get());

        RnBase::_i2.reset();
        assert(!(VBase::_i2 || CTBase::_i2 || RnBase::_i2));
    };
};

In fact, that's exactly what happens on gcc 4.8, 4.9, 5.0 and clang++ 3.5.

Live On Coliru

#include <boost/numeric/ublas/vector.hpp>

namespace ublas = boost::numeric::ublas;

template <typename T = double> using ublasRn = ublas::vector<T>;

struct Base {
    virtual ~Base() {}
};

template <typename T1, typename T2> class Composite : public Base {
  protected:
      template <typename, typename, typename> friend class Trajectory; // or make this public
      using Type1 = T1;
      using Type2 = T2;

      Type1 _i1;
      std::unique_ptr<Type2> _i2;

  public:
      Composite() = default;
};

template <typename T2> class CompositeCT : virtual public Composite<double, T2> {
  public:
    using Composite<double, T2>::Composite;
};

template <typename T1> class CompositeRn : virtual public Composite<T1, ublasRn<double> > {
  public:
    using Composite<T1, ublasRn<double> >::Composite;
};

class CompositeCTRn : public CompositeCT<ublasRn<double> >, public CompositeRn<double> {
    using VBase  = Composite<double, ublasRn<double> >;
    using CTBase = CompositeCT<ublasRn<double> >;
    using RnBase = CompositeRn<double>;
  public:
    CompositeCTRn() : VBase(), CTBase(), RnBase() {
        VBase::_i1 = 1;
        VBase::_i2.reset(new ublasRn<double>(3));

        CTBase::_i1 = 2;
        CTBase::_i2.reset(new ublasRn<double>(3));

        RnBase::_i1 = 3;
        RnBase::_i2.reset(new ublasRn<double>(3));

        assert(3 == VBase::_i1);
        assert(3 == CTBase::_i1);
        assert(3 == RnBase::_i1);

        assert(VBase::_i2.get() == CTBase::_i2.get());
        assert(VBase::_i2.get() == RnBase::_i2.get());

        RnBase::_i2.reset();
        assert(!(VBase::_i2 || CTBase::_i2 || RnBase::_i2));
    };
};

template <typename T1, typename T2, class TComposite> class Trajectory : public Base {
    static_assert(std::is_same<T1, typename TComposite::Type1>::value, "mismatched composite");
    static_assert(std::is_same<T2, typename TComposite::Type2>::value, "mismatched composite");
  protected:
    std::vector<std::unique_ptr<TComposite> > m_Trajectory;

  public:
    Trajectory() { checkType(); }
    void checkType() const {
        std::vector<TComposite> CI(1000);
    }
    virtual ~Trajectory() {}
};

#include <iostream>

int main() {
    Trajectory<double, ublasRn<>, CompositeCTRn> T;
    std::cout << 123 << "\n";
}

Under valgrind:

==15664== Memcheck, a memory error detector
==15664== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==15664== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==15664== Command: ./test
==15664== 
123
==15664== 
==15664== HEAP SUMMARY:
==15664==     in use at exit: 0 bytes in 0 blocks
==15664==   total heap usage: 6,001 allocs, 6,001 frees, 184,000 bytes allocated
==15664== 
==15664== All heap blocks were freed -- no leaks are possible
==15664== 
==15664== For counts of detected and suppressed errors, rerun with: -v
==15664== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

No problem at all.

Notes

  • I replaced the type check with a static assert (because, why not?!)

    template <typename T1, typename T2, class TComposite> class Trajectory : public Base {
        static_assert(std::is_same<T1, typename TComposite::Type1>::value, "mismatched composite");
        static_assert(std::is_same<T2, typename TComposite::Type2>::value, "mismatched composite");
    

Summary / TL;DR

I've looked at your code. Other than a nice bouquet of design smells I don't see anything wrong with it, really. If you need more help, you'll have to start being specific about versions, flags, compilers, architecture...