GitHub-Proxy/eigen
1
0
Fork 0
mirror of https://gitlab.com/libeigen/eigen.git synced 2025-07-29 00:02:40 +08:00
Code Issues Packages Projects Releases Wiki Activity

Constructing a Matrix/Array with implicit transpose could lead to memory leaks.

Browse Source 
Also reduced code duplication for Matrix/Array constructors
This commit is contained in:
Christoph Hertzberg 2015-04-16 13:25:20 +02:00
parent e0cff9ae0d
commit 3be9f5c4d7
6 changed files with 76 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
Eigen/src/Core/Array.h
View File

@ -74,7 +74,7 @@ class Array
{
return Base::operator=(other);
}
/** Set all the entries to \a value.
* \sa DenseBase::setConstant(), DenseBase::fill()
*/
@ -101,7 +101,7 @@ class Array
*/
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Array& operator=(const ArrayBase<OtherDerived>& other)
EIGEN_STRONG_INLINE Array& operator=(const DenseBase<OtherDerived>& other)
{
return Base::_set(other);
}
@ -222,43 +222,18 @@ class Array
m_storage.data()[3] = val3;
}
/** Constructor copying the value of the expression \a other */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Array(const ArrayBase<OtherDerived>& other)
: Base(other.rows() * other.cols(), other.rows(), other.cols())
{
Base::_check_template_params();
Base::_set_noalias(other);
}
/** Copy constructor */
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Array(const Array& other)
: Base(other.rows() * other.cols(), other.rows(), other.cols())
{
Base::_check_template_params();
Base::_set_noalias(other);
}
/** Copy constructor with in-place evaluation */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Array(const ReturnByValue<OtherDerived>& other)
{
Base::_check_template_params();
Base::resize(other.rows(), other.cols());
other.evalTo(*this);
}
: Base(other)
{ }
/** \sa MatrixBase::operator=(const EigenBase<OtherDerived>&) */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Array(const EigenBase<OtherDerived> &other)
: Base(other.derived().rows() * other.derived().cols(), other.derived().rows(), other.derived().cols())
{
Base::_check_template_params();
Base::_resize_to_match(other);
*this = other;
}
: Base(other.derived())
{ }
EIGEN_DEVICE_FUNC inline Index innerStride() const { return 1; }
EIGEN_DEVICE_FUNC inline Index outerStride() const { return this->innerSize(); }

53
Eigen/src/Core/Matrix.h
View File

@ -25,7 +25,7 @@ namespace Eigen {
*
* The first three template parameters are required:
* \tparam _Scalar \anchor matrix_tparam_scalar Numeric type, e.g. float, double, int or std::complex<float>.
* User defined sclar types are supported as well (see \ref user_defined_scalars "here").
* User defined scalar types are supported as well (see \ref user_defined_scalars "here").
* \tparam _Rows Number of rows, or \b Dynamic
* \tparam _Cols Number of columns, or \b Dynamic
*
@ -170,7 +170,7 @@ class Matrix
*/
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Matrix& operator=(const MatrixBase<OtherDerived>& other)
EIGEN_STRONG_INLINE Matrix& operator=(const DenseBase<OtherDerived>& other)
{
return Base::_set(other);
}
@ -266,8 +266,8 @@ class Matrix
*
* \warning This constructor is disabled for fixed-size \c 1x1 matrices. For instance,
* calling Matrix<double,1,1>(1) will call the initialization constructor: Matrix(const Scalar&).
* For fixed-size \c 1x1 matrices it is thefore recommended to use the default
* constructor Matrix() instead, especilly when using one of the non standard
* For fixed-size \c 1x1 matrices it is therefore recommended to use the default
* constructor Matrix() instead, especially when using one of the non standard
* \c EIGEN_INITIALIZE_MATRICES_BY_{ZERO,\c NAN} macros (see \ref TopicPreprocessorDirectives).
*/
EIGEN_STRONG_INLINE explicit Matrix(Index dim);
@ -281,8 +281,8 @@ class Matrix
*
* \warning This constructor is disabled for fixed-size \c 1x2 and \c 2x1 vectors. For instance,
* calling Matrix2f(2,1) will call the initialization constructor: Matrix(const Scalar& x, const Scalar& y).
* For fixed-size \c 1x2 or \c 2x1 vectors it is thefore recommended to use the default
* constructor Matrix() instead, especilly when using one of the non standard
* For fixed-size \c 1x2 or \c 2x1 vectors it is therefore recommended to use the default
* constructor Matrix() instead, especially when using one of the non standard
* \c EIGEN_INITIALIZE_MATRICES_BY_{ZERO,\c NAN} macros (see \ref TopicPreprocessorDirectives).
*/
EIGEN_DEVICE_FUNC
@ -315,37 +315,10 @@ class Matrix
}
/** \brief Constructor copying the value of the expression \a other */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Matrix(const MatrixBase<OtherDerived>& other)
: Base(other.rows() * other.cols(), other.rows(), other.cols())
{
// This test resides here, to bring the error messages closer to the user. Normally, these checks
// are performed deeply within the library, thus causing long and scary error traces.
EIGEN_STATIC_ASSERT((internal::is_same<Scalar, typename OtherDerived::Scalar>::value),
YOU_MIXED_DIFFERENT_NUMERIC_TYPES__YOU_NEED_TO_USE_THE_CAST_METHOD_OF_MATRIXBASE_TO_CAST_NUMERIC_TYPES_EXPLICITLY)
Base::_check_template_params();
Base::_set_noalias(other);
}
/** \brief Copy constructor */
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Matrix(const Matrix& other)
: Base(other.rows() * other.cols(), other.rows(), other.cols())
{
Base::_check_template_params();
Base::_set_noalias(other);
}
/** \brief Copy constructor with in-place evaluation */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Matrix(const ReturnByValue<OtherDerived>& other)
{
Base::_check_template_params();
Base::resize(other.rows(), other.cols());
other.evalTo(*this);
}
EIGEN_STRONG_INLINE Matrix(const Matrix& other) : Base(other)
{ }
/** \brief Copy constructor for generic expressions.
* \sa MatrixBase::operator=(const EigenBase<OtherDerived>&)
@ -353,14 +326,8 @@ class Matrix
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE Matrix(const EigenBase<OtherDerived> &other)
: Base(other.derived().rows() * other.derived().cols(), other.derived().rows(), other.derived().cols())
{
Base::_check_template_params();
Base::_resize_to_match(other);
// FIXME/CHECK: isn't *this = other.derived() more efficient. it allows to
// go for pure _set() implementations, right?
*this = other;
}
: Base(other.derived())
{ }
EIGEN_DEVICE_FUNC inline Index innerStride() const { return 1; }
EIGEN_DEVICE_FUNC inline Index outerStride() const { return this->innerSize(); }

35
Eigen/src/Core/PlainObjectBase.h
View File

@ -69,7 +69,7 @@ template<typename MatrixTypeA, typename MatrixTypeB, bool SwapPointers> struct m
#ifdef EIGEN_PARSED_BY_DOXYGEN
namespace internal {
// this is a warkaround to doxygen not being able to understand the inheritence logic
// this is a workaround to doxygen not being able to understand the inheritance logic
// when it is hidden by the dense_xpr_base helper struct.
template<typename Derived> struct dense_xpr_base_dispatcher_for_doxygen;// : public MatrixBase<Derived> {};
/** This class is just a workaround for Doxygen and it does not not actually exist. */
@ -479,6 +479,10 @@ class PlainObjectBase : public internal::dense_xpr_base<Derived>::type
}
#endif
/** Copy constructor */
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE PlainObjectBase(const PlainObjectBase& other)
: Base(), m_storage(other.m_storage) { }
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE PlainObjectBase(Index a_size, Index nbRows, Index nbCols)
: m_storage(a_size, nbRows, nbCols)
@ -498,15 +502,36 @@ class PlainObjectBase : public internal::dense_xpr_base<Derived>::type
return this->derived();
}
/** \sa MatrixBase::operator=(const EigenBase<OtherDerived>&) */
/** \sa PlainObjectBase::operator=(const EigenBase<OtherDerived>&) */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE PlainObjectBase(const DenseBase<OtherDerived> &other)
: m_storage()
{
_check_template_params();
resizeLike(other);
_set_noalias(other);
}
/** \sa PlainObjectBase::operator=(const EigenBase<OtherDerived>&) */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE PlainObjectBase(const EigenBase<OtherDerived> &other)
: m_storage(other.derived().rows() * other.derived().cols(), other.derived().rows(), other.derived().cols())
: m_storage()
{
_check_template_params();
internal::check_rows_cols_for_overflow<MaxSizeAtCompileTime>::run(other.derived().rows(), other.derived().cols());
Base::operator=(other.derived());
resizeLike(other);
*this = other.derived();
}
/** \brief Copy constructor with in-place evaluation */
template<typename OtherDerived>
EIGEN_DEVICE_FUNC
EIGEN_STRONG_INLINE PlainObjectBase(const ReturnByValue<OtherDerived>& other)
{
_check_template_params();
// FIXME this does not automatically transpose vectors if necessary
resize(other.rows(), other.cols());
other.evalTo(this->derived());
}
/** \name Map

1
test/CMakeLists.txt
View File

@ -252,6 +252,7 @@ ei_add_test(vectorwiseop)
ei_add_test(special_numbers)
ei_add_test(rvalue_types)
ei_add_test(dense_storage)
ei_add_test(ctorleak)
# # ei_add_test(denseLM)

2
test/array.cpp
View File

@ -22,6 +22,8 @@ template<typename ArrayType> void array(const ArrayType& m)
ArrayType m1 = ArrayType::Random(rows, cols),
m2 = ArrayType::Random(rows, cols),
m3(rows, cols);
ArrayType m4 = m1; // copy constructor
VERIFY_IS_APPROX(m1, m4);
ColVectorType cv1 = ColVectorType::Random(rows);
RowVectorType rv1 = RowVectorType::Random(cols);

36
test/ctorleak.cpp
View File

@ -4,48 +4,66 @@
struct Foo
{
static unsigned object_count;
static unsigned object_limit;
static Index object_count;
static Index object_limit;
int dummy;
Foo()
{
#ifdef EIGEN_EXCEPTIONS
// TODO: Is this the correct way to handle this?
if (Foo::object_count > Foo::object_limit) { throw Foo::Fail(); }
if (Foo::object_count > Foo::object_limit) { std::cout << "\nThrow!\n"; throw Foo::Fail(); }
#endif
std::cout << '+';
++Foo::object_count;
}
~Foo()
{
std::cout << '-';
--Foo::object_count;
}
class Fail : public std::exception {};
};
unsigned Foo::object_count = 0;
unsigned Foo::object_limit = 0;
Index Foo::object_count = 0;
Index Foo::object_limit = 0;
#undef EIGEN_TEST_MAX_SIZE
#define EIGEN_TEST_MAX_SIZE 3
void test_ctorleak()
{
typedef DenseIndex Index;
typedef Matrix<Foo, Dynamic, Dynamic> MatrixX;
typedef Matrix<Foo, Dynamic, 1> VectorX;
Foo::object_count = 0;
for(int i = 0; i < g_repeat; i++) {
Index rows = internal::random<Index>(2,EIGEN_TEST_MAX_SIZE), cols = internal::random<Index>(2,EIGEN_TEST_MAX_SIZE);
Foo::object_limit = internal::random(0, rows*cols - 2);
Foo::object_limit = internal::random<Index>(0, rows*cols - 2);
std::cout << "object_limit =" << Foo::object_limit << std::endl;
#ifdef EIGEN_EXCEPTIONS
try
{
#endif
Matrix<Foo, Dynamic, Dynamic> m(rows, cols);
std::cout << "\nMatrixX m(" << rows << ", " << cols << ");\n";
MatrixX m(rows, cols);
#ifdef EIGEN_EXCEPTIONS
VERIFY(false); // not reached if exceptions are enabled
}
catch (const Foo::Fail&) { /* ignore */ }
#endif
VERIFY_IS_EQUAL(Index(0), Foo::object_count);
{
Foo::object_limit = (rows+1)*(cols+1);
MatrixX A(rows, cols);
VERIFY_IS_EQUAL(Foo::object_count, rows*cols);
VectorX v=A.row(0);
VERIFY_IS_EQUAL(Foo::object_count, (rows+1)*cols);
v = A.col(0);
VERIFY_IS_EQUAL(Foo::object_count, rows*(cols+1));
}
VERIFY_IS_EQUAL(Index(0), Foo::object_count);
}
VERIFY_IS_EQUAL(static_cast<unsigned>(0), Foo::object_count);
}