Merge pull request #1432 from turol/asan

Asan

.travis.sh

@@ -1,6 +1,6 @@
 function generate()
 {
-    cmake -G "Unix Makefiles" -DASSIMP_NO_EXPORT=$TRAVIS_NO_EXPORT -DBUILD_SHARED_LIBS=$SHARED_BUILD -DASSIMP_COVERALLS=$ENABLE_COVERALLS -DASSIMP_ERROR=ON
+    cmake -G "Unix Makefiles" -DASSIMP_NO_EXPORT=$TRAVIS_NO_EXPORT -DBUILD_SHARED_LIBS=$SHARED_BUILD -DASSIMP_COVERALLS=$ENABLE_COVERALLS -DASSIMP_ERROR=ON -DASSIMP_ASAN=$ASAN
 }
 
 if [ $ANDROID ]; then

.travis.yml

@@ -39,16 +39,16 @@ matrix:
   include:
     - os: linux
       compiler: gcc
-      env: LINUX=1 TRAVIS_NO_EXPORT=YES ENABLE_COVERALLS=ON
+      env: LINUX=1 TRAVIS_NO_EXPORT=YES ENABLE_COVERALLS=ON  ASAN=OFF
     - os: linux
       compiler: gcc
-      env: LINUX=1 TRAVIS_NO_EXPORT=NO  ENABLE_COVERALLS=OFF
+      env: LINUX=1 TRAVIS_NO_EXPORT=NO  ENABLE_COVERALLS=OFF ASAN=ON
     - os: linux
       compiler: gcc
-      env: LINUX=1 SHARED_BUILD=ON TRAVIS_NO_EXPORT=NO  ENABLE_COVERALLS=OFF
+      env: LINUX=1 SHARED_BUILD=ON TRAVIS_NO_EXPORT=NO  ENABLE_COVERALLS=OFF  ASAN=OFF
     - os: linux
       compiler: gcc
-      env: LINUX=1 SHARED_BUILD=ON TRAVIS_NO_EXPORT=NO  ENABLE_COVERALLS=OFF
+      env: LINUX=1 SHARED_BUILD=ON TRAVIS_NO_EXPORT=NO  ENABLE_COVERALLS=OFF  ASAN=OFF
 
 install:
   - if [ $ANDROID ]; then wget -c http://dl.google.com/android/ndk/android-ndk-${PV}-${PLATF}.tar.bz2 && tar xf android-ndk-${PV}-${PLATF}.tar.bz2 ; fi

CMakeLists.txt

@@ -82,6 +82,10 @@ OPTION ( ASSIMP_WERROR
   "Treat warnings as errors."
   OFF
 )
+OPTION ( ASSIMP_ASAN
+  "Enable AddressSanitizer."
+  OFF
+)
 OPTION ( SYSTEM_IRRXML
   "Use system installed Irrlicht/IrrXML library."
   OFF
@@ -232,6 +236,11 @@ if (ASSIMP_WERROR)
   ENDIF()
 endif()
 
+if (ASSIMP_ASAN)
+    SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
+    SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address")
+endif()
+
 INCLUDE (FindPkgMacros)
 INCLUDE (PrecompiledHeader)
 

code/B3DImporter.cpp

@@ -82,6 +82,20 @@ static const aiImporterDesc desc = {
 
 //#define DEBUG_B3D
 
+template<typename T>
+void DeleteAllBarePointers(std::vector<T>& x)
+{
+    for(auto p : x)
+    {
+        delete p;
+    }
+}
+
+B3DImporter::~B3DImporter()
+{
+    DeleteAllBarePointers(_animations);
+}
+
 // ------------------------------------------------------------------------------------------------
 bool B3DImporter::CanRead( const std::string& pFile, IOSystem* /*pIOHandler*/, bool /*checkSig*/) const{
 
@@ -558,13 +572,19 @@ aiNode *B3DImporter::ReadNODE( aiNode *parent ){
 void B3DImporter::ReadBB3D( aiScene *scene ){
 
     _textures.clear();
+
     _materials.clear();
 
     _vertices.clear();
+
     _meshes.clear();
 
+    DeleteAllBarePointers(_nodes);
     _nodes.clear();
+
     _nodeAnims.clear();
+
+    DeleteAllBarePointers(_animations);
     _animations.clear();
 
     string t=ReadChunk();

code/B3DImporter.h

@@ -59,6 +59,8 @@ namespace Assimp{
 
 class B3DImporter : public BaseImporter{
 public:
+    B3DImporter() = default;
+    virtual ~B3DImporter();
 
     virtual bool CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const;
 

code/MakeVerboseFormat.cpp

@@ -193,14 +193,14 @@ bool MakeVerboseFormatProcess::MakeVerboseFormat(aiMesh* pcMesh)
     p = 0;
     while (pcMesh->HasTextureCoords(p))
     {
-        delete pcMesh->mTextureCoords[p];
+        delete[] pcMesh->mTextureCoords[p];
         pcMesh->mTextureCoords[p] = apvTextureCoords[p];
         ++p;
     }
     p = 0;
     while (pcMesh->HasVertexColors(p))
     {
-        delete pcMesh->mColors[p];
+        delete[] pcMesh->mColors[p];
         pcMesh->mColors[p] = apvColorSets[p];
         ++p;
     }

code/PretransformVertices.cpp

@@ -634,7 +634,7 @@ void PretransformVertices::Execute( aiScene* pScene)
         aiNode* newRoot = new aiNode();
         newRoot->mName = pScene->mRootNode->mName;
         delete pScene->mRootNode;
-        pScene->mRootNode = new aiNode();
+        pScene->mRootNode = newRoot;
 
         if (1 == pScene->mNumMeshes && !pScene->mNumLights && !pScene->mNumCameras)
         {

code/glTF2Asset.h

@@ -511,7 +511,7 @@ namespace glTF2
 
 			/// \fn ~SEncodedRegion()
 			/// Destructor.
-			~SEncodedRegion() { delete [] DecodedData; }
+			~SEncodedRegion() { delete[] DecodedData; }
 		};
 
 		/******************* Variables *******************/

code/glTF2Asset.inl

@@ -313,7 +313,7 @@ inline void Buffer::Read(Value& obj, Asset& r)
         if (dataURI.base64) {
             uint8_t* data = 0;
             this->byteLength = Util::DecodeBase64(dataURI.data, dataURI.dataLength, data);
-            this->mData.reset(data);
+            this->mData.reset(data, std::default_delete<uint8_t[]>());
 
             if (statedLength > 0 && this->byteLength != statedLength) {
                 throw DeadlyImportError("GLTF: buffer \"" + id + "\", expected " + to_string(statedLength) +
@@ -326,7 +326,7 @@ inline void Buffer::Read(Value& obj, Asset& r)
                                         " bytes, but found " + to_string(dataURI.dataLength));
             }
 
-            this->mData.reset(new uint8_t[dataURI.dataLength]);
+            this->mData.reset(new uint8_t[dataURI.dataLength], std::default_delete<uint8_t[]>());
             memcpy( this->mData.get(), dataURI.data, dataURI.dataLength );
         }
     }
@@ -357,7 +357,7 @@ inline bool Buffer::LoadFromStream(IOStream& stream, size_t length, size_t baseO
         stream.Seek(baseOffset, aiOrigin_SET);
     }
 
-    mData.reset(new uint8_t[byteLength]);
+    mData.reset(new uint8_t[byteLength], std::default_delete<uint8_t[]>());
 
     if (stream.Read(mData.get(), byteLength, 1) != 1) {
         return false;
@@ -432,7 +432,7 @@ uint8_t* new_data;
 	// Copy data which place after replacing part.
 	memcpy(&new_data[pBufferData_Offset + pReplace_Count], &mData.get()[pBufferData_Offset + pBufferData_Count], pBufferData_Offset);
 	// Apply new data
-	mData.reset(new_data);
+	mData.reset(new_data, std::default_delete<uint8_t[]>());
 	byteLength = new_data_size;
 
 	return true;
@@ -451,7 +451,7 @@ inline void Buffer::Grow(size_t amount)
     if (amount <= 0) return;
     uint8_t* b = new uint8_t[byteLength + amount];
     if (mData) memcpy(b, mData.get(), byteLength);
-    mData.reset(b);
+    mData.reset(b, std::default_delete<uint8_t[]>());
     byteLength += amount;
 }
 

code/glTFAsset.inl

@@ -341,7 +341,7 @@ inline bool Buffer::LoadFromStream(IOStream& stream, size_t length, size_t baseO
         stream.Seek(baseOffset, aiOrigin_SET);
     }
 
-    mData.reset(new uint8_t[byteLength]);
+    mData.reset(new uint8_t[byteLength], std::default_delete<uint8_t[]>());
 
     if (stream.Read(mData.get(), byteLength, 1) != 1) {
         return false;

test/unit/TestModelFactory.h

@@ -60,7 +60,7 @@ public:
     static aiScene *createDefaultTestModel( float &opacity  ) {
         aiScene *scene( new aiScene );
         scene->mNumMaterials = 1;
-        scene->mMaterials = new aiMaterial*;
+        scene->mMaterials = new aiMaterial*[scene->mNumMaterials];
         scene->mMaterials[ 0 ] = new aiMaterial;
         aiColor3D color( 1, 0, 0 );
         EXPECT_EQ( AI_SUCCESS, scene->mMaterials[ 0 ]->AddProperty( &color, 1, AI_MATKEY_COLOR_DIFFUSE ) );
@@ -70,7 +70,7 @@ public:
         EXPECT_EQ( AI_SUCCESS, scene->mMaterials[ 0 ]->AddProperty( &opacity, 1, AI_MATKEY_OPACITY ) );
 
         scene->mNumMeshes = 1;
-        scene->mMeshes = new aiMesh*;
+        scene->mMeshes = new aiMesh*[scene->mNumMeshes];
         scene->mMeshes[ 0 ] = new aiMesh;
         scene->mMeshes[ 0 ]->mMaterialIndex = 0;
         scene->mMeshes[ 0 ]->mPrimitiveTypes = aiPrimitiveType_TRIANGLE;
@@ -80,7 +80,7 @@ public:
         scene->mMeshes[ 0 ]->mVertices[ 1 ] = aiVector3D( 0, 1, 0 );
         scene->mMeshes[ 0 ]->mVertices[ 2 ] = aiVector3D( 0, 0, 1 );
         scene->mMeshes[ 0 ]->mNumFaces = 1;
-        scene->mMeshes[ 0 ]->mFaces = new aiFace;
+        scene->mMeshes[ 0 ]->mFaces = new aiFace[scene->mMeshes[ 0 ]->mNumFaces];
         scene->mMeshes[ 0 ]->mFaces[ 0 ].mNumIndices = 3;
         scene->mMeshes[ 0 ]->mFaces[ 0 ].mIndices = new unsigned int[ 3 ];
         scene->mMeshes[ 0 ]->mFaces[ 0 ].mIndices[ 0 ] = 0;
@@ -89,7 +89,7 @@ public:
 
         scene->mRootNode = new aiNode;
         scene->mRootNode->mNumMeshes = 1;
-        scene->mRootNode->mMeshes = new unsigned int( 0 );
+        scene->mRootNode->mMeshes = new unsigned int[scene->mRootNode->mNumMeshes]{ 0 };
 
         return scene;
     }

test/unit/utIssues.cpp

@@ -74,6 +74,7 @@ TEST_F( utIssues, OpacityBugWhenExporting_727 ) {
         EXPECT_EQ( AI_SUCCESS, newScene->mMaterials[ 0 ]->Get( AI_MATKEY_OPACITY, newOpacity ) );
         EXPECT_EQ( opacity, newOpacity );
     }
+    delete scene;
 }
 
 #endif // ASSIMP_BUILD_NO_EXPORT

test/unit/utMaterialSystem.cpp

@@ -73,7 +73,7 @@ TEST_F(MaterialSystemTest, testFloatArrayProperty)
 {
     float pf[] = {0.0f,1.0f,2.0f,3.0f};
     unsigned int pMax = sizeof(pf) / sizeof(float);
-    this->pcMat->AddProperty(&pf,pMax,"testKey2");
+    this->pcMat->AddProperty(pf,pMax,"testKey2");
     pf[0] = pf[1] = pf[2] = pf[3] = 12.0f;
 
     EXPECT_EQ(AI_SUCCESS, pcMat->Get("testKey2",0,0,pf,&pMax));
@@ -97,7 +97,7 @@ TEST_F(MaterialSystemTest, testIntArrayProperty)
 {
     int pf[] = {0,1,2,3};
     unsigned int pMax = sizeof(pf) / sizeof(int);
-    this->pcMat->AddProperty(&pf,pMax,"testKey4");
+    this->pcMat->AddProperty(pf,pMax,"testKey4");
     pf[0] = pf[1] = pf[2] = pf[3] = 12;
 
     EXPECT_EQ(AI_SUCCESS, pcMat->Get("testKey4",0,0,pf,&pMax));

test/unit/utObjImportExport.cpp

@@ -237,6 +237,15 @@ TEST_F( utObjImportExport, obj_import_test ) {
     differ.showReport();
 
     m_im->FreeScene();
+    for(unsigned int i = 0; i < expected->mNumMeshes; ++i)
+    {
+        delete expected->mMeshes[i];
+    }
+    delete[] expected->mMeshes;
+    expected->mMeshes = nullptr;
+    delete[] expected->mMaterials;
+    expected->mMaterials = nullptr;
+    delete expected;
 }
 
 TEST_F( utObjImportExport, issue1111_no_mat_name_Test ) {

test/unit/utObjTools.cpp

@@ -106,8 +106,9 @@ TEST_F( utObjTools, countComponents_TwoLines_Success ) {
     TestObjFileParser test_parser;
     std::string data( "-2.061493116917992e-15 -0.9009688496589661 \\\n-0.4338837265968323" );
     std::vector<char> buffer;
-    buffer.resize( data.size() );
+    buffer.resize( data.size() + 1 );
     ::memcpy( &buffer[ 0 ], &data[ 0 ], data.size() );
+    buffer[ buffer.size() - 1 ] = '\0';
     test_parser.setBuffer( buffer );
 
     size_t numComps = test_parser.testGetNumComponentsInDataDefinition();

test/unit/utRemoveVCProcess.cpp

@@ -72,4 +72,5 @@ TEST_F( utRevmoveVCProcess, issue1266_ProcessMeshTest_NoCrash ) {
     scene->mMeshes[ 0 ] = mesh;
     RemoveVCProcess *process = new RemoveVCProcess;
     process->Execute( scene );
-}
+    delete scene;
+}

test/unit/utSceneCombiner.cpp

@@ -42,6 +42,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "UnitTestPCH.h"
 #include <assimp/SceneCombiner.h>
 #include <assimp/mesh.h>
+#include <memory>
 
 using namespace ::Assimp;
 
@@ -63,8 +64,10 @@ TEST_F( utSceneCombiner, MergeMeshes_ValidNames_Test ) {
     mesh3->mName.Set( "mesh_3" );
     merge_list.push_back( mesh3 );
 
-    aiMesh *out( nullptr );
-    SceneCombiner::MergeMeshes( &out, 0, merge_list.begin(), merge_list.end() );
+    std::unique_ptr<aiMesh> out;
+    aiMesh* ptr = nullptr;
+    SceneCombiner::MergeMeshes( &ptr, 0, merge_list.begin(), merge_list.end() );
+    out.reset(ptr);
     std::string outName = out->mName.C_Str();
     EXPECT_EQ( "mesh_1.mesh_2.mesh_3", outName );
 }

test/unit/utSharedPPData.cpp

@@ -92,7 +92,7 @@ TEST_F(SharedPPDataTest, testPODProperty)
 // ------------------------------------------------------------------------------------------------
 TEST_F(SharedPPDataTest, testPropertyPointer)
 {
-    int *i = new int[35];
+    int *i = new int;
     shared->AddProperty("test16",i);
     int* o;
     EXPECT_TRUE(shared->GetProperty("test16",o));