From 71c824b91ab3024359ff324bd07a67645c9f08e8 Mon Sep 17 00:00:00 2001 From: gereon Date: Tue, 4 Dec 2012 21:20:05 +0100 Subject: [PATCH] hunting for memory erros adding make targets to call mrmc and mrmc-tests with valgrind fixing some memory errors in SSM, SSMTest, BitVector adding an additional check to readLabFile --- CMakeLists.txt | 3 +++ src/parser/readLabFile.cpp | 10 +++++++++- src/storage/BitVector.h | 20 ++++++++++---------- src/storage/SquareSparseMatrix.h | 12 ++++++------ test/storage/SquareSparseMatrixTest.cpp | 11 +++++++++++ 5 files changed, 39 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 52cf0f1a3..99386c34b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -171,3 +171,6 @@ configure_file ( "${PROJECT_SOURCE_DIR}/mrmc-config.h.in" "${PROJECT_BINARY_DIR}/mrmc-config.h" ) + +add_custom_target(memcheck valgrind --leak-check=full --show-reachable=yes ./mrmc examples/dtmc/crowds/crowds5_5.tra examples/dtmc/crowds/crowds5_5.lab) +add_custom_target(memcheck-tests valgrind --leak-check=full --show-reachable=yes ./mrmc-tests) diff --git a/src/parser/readLabFile.cpp b/src/parser/readLabFile.cpp index d53e1c424..87602652b 100644 --- a/src/parser/readLabFile.cpp +++ b/src/parser/readLabFile.cpp @@ -124,7 +124,15 @@ LabParser::LabParser(uint_fast64_t node_count, const char * filename) { buf += cnt; cnt = strcspn(buf, separator); // position of next separator - if (cnt > 0) + if (cnt >= sizeof(proposition)) + { + /* + * if token is longer than our buffer, the following strncpy code might get risky... + */ + LOG4CPLUS_ERROR(logger, "Wrong file format in (" << filename << "). Atomic proposition with length > " << (sizeof(proposition)-1) << " was found."); + throw mrmc::exceptions::wrong_file_format(); + } + else if (cnt > 0) { /* * next token is: #DECLARATION: just skip it diff --git a/src/storage/BitVector.h b/src/storage/BitVector.h index 87e2b8fac..3d0df0907 100644 --- a/src/storage/BitVector.h +++ b/src/storage/BitVector.h @@ -113,17 +113,17 @@ public: if (initTrue) { // Finally, create the full bucket array. - bucketArray = new uint64_t[bucketCount]; + this->bucketArray = new uint64_t[bucketCount]; // Now initialize the values. for (uint_fast64_t i = 0; i < bucketCount; ++i) { - bucketArray[i] = -1ll; + this->bucketArray[i] = -1ll; } truncateLastBucket(); } else { // Finally, create the full bucket array. - bucketArray = new uint64_t[bucketCount](); + this->bucketArray = new uint64_t[bucketCount](); } } @@ -135,7 +135,7 @@ public: BitVector(const BitVector &bv) : bucketCount(bv.bucketCount), bitCount(bv.bitCount), endIterator(*this, bitCount, bitCount, false) { LOG4CPLUS_WARN(logger, "Invoking copy constructor."); bucketArray = new uint64_t[bucketCount]; - std::copy(bv.bucketArray, bv.bucketArray + bucketCount, bucketArray); + std::copy(bv.bucketArray, bv.bucketArray + this->bucketCount, this->bucketArray); } //! Destructor @@ -143,8 +143,8 @@ public: * Destructor. Frees the underlying bucket array. */ ~BitVector() { - if (bucketArray != nullptr) { - delete[] bucketArray; + if (this->bucketArray != nullptr) { + delete[] this->bucketArray; } } @@ -179,7 +179,7 @@ public: } // Reserve a temporary array for copying. - uint_fast64_t* tempArray = new uint_fast64_t[newBucketCount]; + uint64_t* tempArray = new uint64_t[newBucketCount]; // Copy over the elements from the old bit vector. uint_fast64_t copySize = (newBucketCount <= bucketCount) ? newBucketCount : bucketCount; @@ -506,9 +506,9 @@ private: * from (bitCount + 1). */ void truncateLastBucket() { - if ((bitCount & mod64mask) != 0) { - uint64_t mask = ((1ll << (bitCount & mod64mask)) - 1ll); - bucketArray[bucketCount - 1] = bucketArray[bucketCount - 1] & mask; + if ((this->bitCount & mod64mask) != 0) { + uint64_t mask = ((1ll << (this->bitCount & mod64mask)) - 1ll); + this->bucketArray[this->bucketCount - 1] = this->bucketArray[this->bucketCount - 1] & mask; } } diff --git a/src/storage/SquareSparseMatrix.h b/src/storage/SquareSparseMatrix.h index ac5260599..fd9c0b904 100644 --- a/src/storage/SquareSparseMatrix.h +++ b/src/storage/SquareSparseMatrix.h @@ -112,19 +112,19 @@ public: */ ~SquareSparseMatrix() { setState(MatrixStatus::UnInitialized); - if (valueStorage != NULL) { + if (valueStorage != nullptr) { //free(value_storage); delete[] valueStorage; } - if (columnIndications != NULL) { + if (columnIndications != nullptr) { //free(column_indications); delete[] columnIndications; } - if (rowIndications != NULL) { + if (rowIndications != nullptr) { //free(row_indications); delete[] rowIndications; } - if (diagonalStorage != NULL) { + if (diagonalStorage != nullptr) { //free(diagonal_storage); delete[] diagonalStorage; } @@ -507,7 +507,7 @@ public: uint_fast64_t rowStart; uint_fast64_t rowEnd; uint_fast64_t zeroCount = 0; - for (uint_fast64_t row = 0; row <= rowCount; ++row) { + for (uint_fast64_t row = 0; row < rowCount; ++row) { rowStart = rowIndications[row]; rowEnd = rowIndications[row + 1]; while (rowStart < rowEnd) { @@ -518,7 +518,7 @@ public: } // Then add the elements on the diagonal. - for (uint_fast64_t i = 0; i <= rowCount; ++i) { + for (uint_fast64_t i = 0; i < rowCount; ++i) { if (diagonalStorage[i] == 0) zeroCount++; // tripletList.push_back(IntTriplet(i, i, diagonalStorage[i])); } diff --git a/test/storage/SquareSparseMatrixTest.cpp b/test/storage/SquareSparseMatrixTest.cpp index e10d6bf88..da42aacc4 100644 --- a/test/storage/SquareSparseMatrixTest.cpp +++ b/test/storage/SquareSparseMatrixTest.cpp @@ -157,6 +157,8 @@ TEST(SquareSparseMatrixTest, ConversionFromDenseEigen_ColMajor_SparseMatrixTest) ASSERT_EQ(target, row * 10 + col); } } + + delete ssm; } TEST(SquareSparseMatrixTest, ConversionFromDenseEigen_RowMajor_SparseMatrixTest) { @@ -187,6 +189,8 @@ TEST(SquareSparseMatrixTest, ConversionFromDenseEigen_RowMajor_SparseMatrixTest) ASSERT_EQ(target, row * 10 + col); } } + + delete ssm; } TEST(SquareSparseMatrixTest, ConversionFromSparseEigen_ColMajor_SparseMatrixTest) { @@ -234,6 +238,8 @@ TEST(SquareSparseMatrixTest, ConversionFromSparseEigen_ColMajor_SparseMatrixTest ASSERT_TRUE(ssm->getValue(coeff.row(), coeff.col(), &target)); ASSERT_EQ(target, coeff.value()); } + + delete ssm; } TEST(SquareSparseMatrixTest, ConversionFromSparseEigen_RowMajor_SparseMatrixTest) { @@ -281,6 +287,8 @@ TEST(SquareSparseMatrixTest, ConversionFromSparseEigen_RowMajor_SparseMatrixTest ASSERT_TRUE(ssm->getValue(coeff.row(), coeff.col(), &target)); ASSERT_EQ(target, coeff.value()); } + + delete ssm; } TEST(SquareSparseMatrixTest, ConversionToSparseEigen_RowMajor_SparseMatrixTest) { @@ -311,4 +319,7 @@ TEST(SquareSparseMatrixTest, ConversionToSparseEigen_RowMajor_SparseMatrixTest) ASSERT_EQ(values[row * 10 + col], esm->coeff(row, col)); } } + + delete esm; + delete ssm; }