From 58687aa5e638d365d5e41c1e6c66cbfc44fce85f Mon Sep 17 00:00:00 2001 From: Moritz Klammler Date: Sun, 6 Jul 2014 06:58:13 +0200 Subject: Avoid memory leak when constructor of user-defined type throws exception. The added check `ctorleak.cpp` demonstrates how the leak can be reproduced. The test appears to pass but it is leaking the storage of the (not created) matrix. I don't know how to make this test fail in the existing test suite but you can run it through Valgrind (or another debugger) to verify the leak. $ ./check.sh ctorleak && valgrind --leak-check=full ./test/ctorleak This patch fixes this leak by adding some try-catch-delete-rethrow blocks to `Eigen/src/Core/util/Memory.h`. --- Eigen/src/Core/util/Memory.h | 83 ++++++++++++++++++++++++++++++++++++-------- test/CMakeLists.txt | 2 ++ test/ctorleak.cpp | 43 +++++++++++++++++++++++ 3 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 test/ctorleak.cpp diff --git a/Eigen/src/Core/util/Memory.h b/Eigen/src/Core/util/Memory.h index dd9285714..4b3dcde3a 100644 --- a/Eigen/src/Core/util/Memory.h +++ b/Eigen/src/Core/util/Memory.h @@ -338,15 +338,6 @@ template<> inline void* conditional_aligned_realloc(void* ptr, size_t new *** Construction/destruction of array elements *** *****************************************************************************/ -/** \internal Constructs the elements of an array. - * The \a size parameter tells on how many objects to call the constructor of T. - */ -template inline T* construct_elements_of_array(T *ptr, size_t size) -{ - for (size_t i=0; i < size; ++i) ::new (ptr + i) T; - return ptr; -} - /** \internal Destructs the elements of an array. * The \a size parameters tells on how many objects to call the destructor of T. */ @@ -357,6 +348,24 @@ template inline void destruct_elements_of_array(T *ptr, size_t size) while(size) ptr[--size].~T(); } +/** \internal Constructs the elements of an array. + * The \a size parameter tells on how many objects to call the constructor of T. + */ +template inline T* construct_elements_of_array(T *ptr, size_t size) +{ + size_t i; + try + { + for (i = 0; i < size; ++i) ::new (ptr + i) T; + return ptr; + } + catch (...) + { + destruct_elements_of_array(ptr, i); + throw; + } +} + /***************************************************************************** *** Implementation of aligned new/delete-like functions *** *****************************************************************************/ @@ -376,14 +385,30 @@ template inline T* aligned_new(size_t size) { check_size_for_overflow(size); T *result = reinterpret_cast(aligned_malloc(sizeof(T)*size)); - return construct_elements_of_array(result, size); + try + { + return construct_elements_of_array(result, size); + } + catch (...) + { + aligned_free(result); + throw; + } } template inline T* conditional_aligned_new(size_t size) { check_size_for_overflow(size); T *result = reinterpret_cast(conditional_aligned_malloc(sizeof(T)*size)); - return construct_elements_of_array(result, size); + try + { + return construct_elements_of_array(result, size); + } + catch (...) + { + conditional_aligned_free(result); + throw; + } } /** \internal Deletes objects constructed with aligned_new @@ -412,7 +437,17 @@ template inline T* conditional_aligned_realloc_new(T* pt destruct_elements_of_array(pts+new_size, old_size-new_size); T *result = reinterpret_cast(conditional_aligned_realloc(reinterpret_cast(pts), sizeof(T)*new_size, sizeof(T)*old_size)); if(new_size > old_size) - construct_elements_of_array(result+old_size, new_size-old_size); + { + try + { + construct_elements_of_array(result+old_size, new_size-old_size); + } + catch (...) + { + conditional_aligned_free(result); + throw; + } + } return result; } @@ -422,7 +457,17 @@ template inline T* conditional_aligned_new_auto(size_t s check_size_for_overflow(size); T *result = reinterpret_cast(conditional_aligned_malloc(sizeof(T)*size)); if(NumTraits::RequireInitialization) - construct_elements_of_array(result, size); + { + try + { + construct_elements_of_array(result, size); + } + catch (...) + { + conditional_aligned_free(result); + throw; + } + } return result; } @@ -434,7 +479,17 @@ template inline T* conditional_aligned_realloc_new_auto( destruct_elements_of_array(pts+new_size, old_size-new_size); T *result = reinterpret_cast(conditional_aligned_realloc(reinterpret_cast(pts), sizeof(T)*new_size, sizeof(T)*old_size)); if(NumTraits::RequireInitialization && (new_size > old_size)) - construct_elements_of_array(result+old_size, new_size-old_size); + { + try + { + construct_elements_of_array(result+old_size, new_size-old_size); + } + catch (...) + { + conditional_aligned_free(result); + throw; + } + } return result; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4521d07e4..fed5c0e06 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -251,6 +251,8 @@ ei_add_test(bicgstab) ei_add_test(sparselu) ei_add_test(sparseqr) +ei_add_test(ctorleak) + # ei_add_test(denseLM) if(QT4_FOUND) diff --git a/test/ctorleak.cpp b/test/ctorleak.cpp new file mode 100644 index 000000000..72ab94b66 --- /dev/null +++ b/test/ctorleak.cpp @@ -0,0 +1,43 @@ +#include "main.h" + +#include // std::exception + +struct Foo +{ + int dummy; + Foo() { if (!internal::random(0, 10)) throw Foo::Fail(); } + class Fail : public std::exception {}; +}; + +namespace Eigen +{ + template<> + struct NumTraits + { + typedef double Real; + typedef double NonInteger; + typedef double Nested; + enum + { + IsComplex = 0, + IsInteger = 1, + ReadCost = -1, + AddCost = -1, + MulCost = -1, + IsSigned = 1, + RequireInitialization = 1 + }; + static inline Real epsilon() { return 1.0; } + static inline Real dummy_epsilon() { return 0.0; } + }; +} + +void test_ctorleak() +{ + try + { + Matrix m(14, 92); + eigen_assert(false); // not reached + } + catch (const Foo::Fail&) { /* ignore */ } +} -- cgit v1.2.3