Add Address Sanitizer options and fix reported issues (colmap#1390)

ahojnnes · lucasthahn · commit 589d3fe0aded · 2022-08-17T01:28:39.000-04:00
* Add Address Sanitizer options and fix reported issues

* Add CI build

* Fix

* Asan with clang
diff --git a/.azure-pipelines/build-ubuntu.yaml b/.azure-pipelines/build-ubuntu.yaml
@@ -2,9 +2,10 @@ parameters:
   displayName: 'Ubuntu 18.04'
   ubuntuVersion: '18.04'
   cudaEnabled: false
+  asanEnabled: false
 
 jobs:
-- job: ubuntu_build_${{ replace(parameters.ubuntuVersion, '.', '') }}_cuda_${{ parameters.cudaEnabled }}
+- job: ubuntu_build_${{ replace(parameters.ubuntuVersion, '.', '') }}_cuda_${{ parameters.cudaEnabled }}_asa_${{ parameters.asanEnabled }}
   displayName: '${{ parameters.displayName }}'
   pool:
     vmImage: 'ubuntu-${{ parameters.ubuntuVersion }}'
@@ -41,14 +42,22 @@ jobs:
         echo '##vso[task.setvariable variable=CXX]/usr/bin/cuda-g++'
       displayName: 'Install CUDA'
 
+  - ${{ if eq(parameters.asanEnabled, true) }}:
+    - script: |
+        sudo apt-get install -y clang-10
+        echo '##vso[task.setvariable variable=CC]/usr/bin/clang'
+        echo '##vso[task.setvariable variable=CXX]/usr/bin/clang++'
+      displayName: 'Install CUDA'
+
   - script: |
       cmake --version
       mkdir build
       cd build
       cmake .. \
         -GNinja \
         -DTESTS_ENABLED=ON \
-        -DCUDA_ARCHS=6.0
+        -DCUDA_ARCHS=6.0 \
+        -DASAN_ENABLED=${{ parameters.asanEnabled }}
       ninja
     displayName: 'Configure and build'
 
diff --git a/.azure-pipelines/build.yaml b/.azure-pipelines/build.yaml
@@ -16,6 +16,11 @@ jobs:
       displayName: 'Ubuntu 20.04 (CUDA)'
       ubuntuVersion: 20.04
       cudaEnabled: true
+  - template: build-ubuntu.yaml
+    parameters:
+      displayName: 'Ubuntu 20.04 (ASan)'
+      ubuntuVersion: 20.04
+      asanEnabled: true
   - template: build-mac.yaml
     parameters:
       displayName: 'Mac 10.15'
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -61,6 +61,7 @@ option(CUDA_ENABLED "Whether to enable CUDA, if available" ON)
 option(GUI_ENABLED "Whether to enable the graphical UI" ON)
 option(OPENGL_ENABLED "Whether to enable OpenGL, if available" ON)
 option(TESTS_ENABLED "Whether to build test binaries" OFF)
+option(ASAN_ENABLED "Whether to enable AddressSanitizer flags" OFF)
 option(PROFILING_ENABLED "Whether to enable google-perftools linker flags" OFF)
 option(CGAL_ENABLED "Whether to enable the CGAL library" ON)
 option(BOOST_STATIC "Whether to enable static boost library linker flags" ON)
@@ -198,6 +199,16 @@ else()
     message(STATUS "Disabling interprocedural optimization")
 endif()
 
+if(ASAN_ENABLED)
+    message(STATUS "Enabling ASan flags")
+    if(IS_CLANG OR IS_GNU)
+        add_compile_options(-fsanitize=address -fno-omit-frame-pointer -fsanitize-address-use-after-scope)
+        add_link_options(-fsanitize=address)
+    else()
+        message(FATAL_ERROR "Unsupported compiler for ASan mode")
+    endif()
+endif()
+
 if(CUDA_FOUND)
     if(CUDA_ENABLED)
         add_definitions("-DCUDA_ENABLED")
diff --git a/doc/install.rst b/doc/install.rst
@@ -316,6 +316,22 @@ with the source code ``hello_world.cc``::
     }
 
 
+----------------
+AddressSanitizer
+----------------
+
+If you want to build COLMAP with address sanitizer flags enabled, you need to
+use a recent compiler with ASan support. For example, you can manually install
+a recent clang version on your Ubuntu machine and invoke CMake as follows::
+
+    CC=/usr/bin/clang CXX=/usr/bin/clang++ cmake .. \
+        -DASAN_ENABLED=ON \
+        -DTESTS_ENABLED=ON \
+        -DCMAKE_BUILD_TYPE=RelWithDebInfo
+
+Note that it is generally useful to combine ASan with debug symbols to get
+meaningful traces for reported issues.
+
 -------------
 Documentation
 -------------
diff --git a/lib/Graclus/multilevelLib/mlkkm.c b/lib/Graclus/multilevelLib/mlkkm.c
@@ -182,12 +182,12 @@ int MLKKMPartitioning(CtrlType *ctrl, GraphType *graph, int nparts, int chain_le
   GraphType *cgraph;
   int wgtflag=3, numflag=0, options[10], edgecut;
   float ncut;
-  idxtype *cptr, *cind;
+  // idxtype *cptr, *cind;
   int numcomponents;
   char *mlwkkm_fname = "coarse.graph";
 
-  cptr = idxmalloc(graph->nvtxs, "MLKKMPartitioning: cptr");
-  cind = idxmalloc(graph->nvtxs, "MLKKMPartitioning: cind");
+  // cptr = idxmalloc(graph->nvtxs, "MLKKMPartitioning: cptr");
+  // cind = idxmalloc(graph->nvtxs, "MLKKMPartitioning: cind");
   //printf("Computing the number of connected components.\n");
   /*numcomponents = FindComponents(ctrl, graph, cptr, cind);
 
diff --git a/lib/PBA/pba.cpp b/lib/PBA/pba.cpp
@@ -116,14 +116,14 @@ void ParallelBA::SetFocalMask(const int* fmask, float weight) {
   if (_optimizer && weight > 0) _optimizer->SetFocalMask(fmask, weight);
 }
 
-void* ParallelBA::operator new(size_t size) {
-  void* p = malloc(size);
-  if (p == 0) {
-    const std::bad_alloc ba;
-    throw ba;
-  }
-  return p;
-}
+// void* ParallelBA::operator new(size_t size) {
+//   void* p = malloc(size);
+//   if (p == 0) {
+//     const std::bad_alloc ba;
+//     throw ba;
+//   }
+//   return p;
+// }
 
 ParallelBA* NewParallelBA(ParallelBA::DeviceT device) {
   return new ParallelBA(device);
diff --git a/lib/PBA/pba.h b/lib/PBA/pba.h
@@ -132,7 +132,7 @@ class ParallelBA {
  public:
   PBA_EXPORT ParallelBA(DeviceT device = PBA_CUDA_DEVICE_DEFAULT,
                         const int num_threads = -1);
-  PBA_EXPORT void* operator new(size_t size);
+  // PBA_EXPORT void* operator new(size_t size);
   PBA_EXPORT virtual ~ParallelBA();
 
  public:
diff --git a/src/base/cost_functions_test.cc b/src/base/cost_functions_test.cc
@@ -39,9 +39,9 @@
 using namespace colmap;
 
 BOOST_AUTO_TEST_CASE(TestBundleAdjustmentCostFunction) {
-  ceres::CostFunction* cost_function =
+  std::unique_ptr<ceres::CostFunction> cost_function(
       BundleAdjustmentCostFunction<SimplePinholeCameraModel>::Create(
-          Eigen::Vector2d::Zero());
+          Eigen::Vector2d::Zero()));
   double qvec[4] = {1, 0, 0, 0};
   double tvec[3] = {0, 0, 0};
   double point3D[3] = {0, 0, 1};
@@ -69,10 +69,11 @@ BOOST_AUTO_TEST_CASE(TestBundleAdjustmentCostFunction) {
 }
 
 BOOST_AUTO_TEST_CASE(TestBundleAdjustmentConstantPoseCostFunction) {
-  ceres::CostFunction* cost_function = BundleAdjustmentConstantPoseCostFunction<
-      SimplePinholeCameraModel>::Create(ComposeIdentityQuaternion(),
-                                        Eigen::Vector3d::Zero(),
-                                        Eigen::Vector2d::Zero());
+  std::unique_ptr<ceres::CostFunction> cost_function(
+      BundleAdjustmentConstantPoseCostFunction<
+          SimplePinholeCameraModel>::Create(ComposeIdentityQuaternion(),
+                                            Eigen::Vector3d::Zero(),
+                                            Eigen::Vector2d::Zero()));
   double point3D[3] = {0, 0, 1};
   double camera_params[3] = {1, 0, 0};
   double residuals[2];
@@ -134,9 +135,9 @@ BOOST_AUTO_TEST_CASE(TestBundleAdjustmentConstantPoint3DCostFunction) {
 }
 
 BOOST_AUTO_TEST_CASE(TestRigBundleAdjustmentCostFunction) {
-  ceres::CostFunction* cost_function =
+  std::unique_ptr<ceres::CostFunction> cost_function(
       RigBundleAdjustmentCostFunction<SimplePinholeCameraModel>::Create(
-          Eigen::Vector2d::Zero());
+          Eigen::Vector2d::Zero()));
   double rig_qvec[4] = {1, 0, 0, 0};
   double rig_tvec[3] = {0, 0, -1};
   double rel_qvec[4] = {1, 0, 0, 0};
@@ -167,22 +168,23 @@ BOOST_AUTO_TEST_CASE(TestRigBundleAdjustmentCostFunction) {
 }
 
 BOOST_AUTO_TEST_CASE(TestRelativePoseCostFunction) {
-  ceres::CostFunction* cost_function = RelativePoseCostFunction::Create(
-      Eigen::Vector2d(0, 0), Eigen::Vector2d(0, 0));
+  std::unique_ptr<ceres::CostFunction> cost_function(
+      RelativePoseCostFunction::Create(Eigen::Vector2d(0, 0),
+                                       Eigen::Vector2d(0, 0)));
   double qvec[4] = {1, 0, 0, 0};
   double tvec[3] = {0, 1, 0};
   double residuals[1];
   const double* parameters[2] = {qvec, tvec};
   BOOST_CHECK(cost_function->Evaluate(parameters, residuals, nullptr));
   BOOST_CHECK_EQUAL(residuals[0], 0);
 
-  cost_function = RelativePoseCostFunction::Create(Eigen::Vector2d(0, 0),
-                                                   Eigen::Vector2d(1, 0));
+  cost_function.reset(RelativePoseCostFunction::Create(Eigen::Vector2d(0, 0),
+                                                       Eigen::Vector2d(1, 0)));
   BOOST_CHECK(cost_function->Evaluate(parameters, residuals, nullptr));
   BOOST_CHECK_EQUAL(residuals[0], 0.5);
 
-  cost_function = RelativePoseCostFunction::Create(Eigen::Vector2d(0, 0),
-                                                   Eigen::Vector2d(1, 1));
+  cost_function.reset(RelativePoseCostFunction::Create(Eigen::Vector2d(0, 0),
+                                                       Eigen::Vector2d(1, 1)));
   BOOST_CHECK(cost_function->Evaluate(parameters, residuals, nullptr));
   BOOST_CHECK_EQUAL(residuals[0], 0.5);
 }
diff --git a/src/base/line.cc b/src/base/line.cc
@@ -38,6 +38,13 @@ extern "C" {
 }
 
 namespace colmap {
+namespace {
+
+struct RawDeleter {
+  void operator()(double* p) { free(p); }
+};
+
+}  // namespace
 
 std::vector<LineSegment> DetectLineSegments(const Bitmap& bitmap,
                                             const double min_length) {
@@ -55,9 +62,9 @@ std::vector<LineSegment> DetectLineSegments(const Bitmap& bitmap,
                                          bitmap_data.end());
 
   int num_segments;
-  std::unique_ptr<double> segments_data(lsd(&num_segments,
-                                            bitmap_data_double.data(),
-                                            bitmap.Width(), bitmap.Height()));
+  std::unique_ptr<double, RawDeleter> segments_data(
+      lsd(&num_segments, bitmap_data_double.data(), bitmap.Width(),
+          bitmap.Height()));
 
   std::vector<LineSegment> segments;
   segments.reserve(num_segments);
diff --git a/src/base/reconstruction.cc b/src/base/reconstruction.cc
@@ -1487,8 +1487,8 @@ size_t Reconstruction::FilterPoints3DWithLargeReprojectionError(
     class Point3D& point3D = Point3D(point3D_id);
 
     if (point3D.Track().Length() < 2) {
-      DeletePoint3D(point3D_id);
       num_filtered += point3D.Track().Length();
+      DeletePoint3D(point3D_id);
       continue;
     }
 
diff --git a/src/estimators/similarity_transform.h b/src/estimators/similarity_transform.h
@@ -109,8 +109,8 @@ SimilarityTransformEstimator<kDim, kEstimateScale>::Estimate(
     dst_mat.col(i) = dst[i];
   }
 
-  const auto model = Eigen::umeyama(src_mat, dst_mat, kEstimateScale)
-                         .topLeftCorner(kDim, kDim + 1);
+  const M_t model = Eigen::umeyama(src_mat, dst_mat, kEstimateScale)
+                        .topLeftCorner(kDim, kDim + 1);
 
   if (model.array().isNaN().any()) {
     return std::vector<M_t>{};
diff --git a/src/ui/colormaps.cc b/src/ui/colormaps.cc
@@ -150,7 +150,7 @@ void PointColormapGroundResolution::Prepare(
 
     const Eigen::Vector3f xyz = point3D.second.XYZ().cast<float>();
 
-    for (const auto track_el : point3D.second.Track().Elements()) {
+    for (const auto& track_el : point3D.second.Track().Elements()) {
       const auto& image = images[track_el.image_id];
       const float focal_length = focal_lengths[image.CameraId()];
       const float focal_length2 = focal_length * focal_length;

Original file line number	Diff line number	Diff line change
`@@ -1487,8 +1487,8 @@ size_t Reconstruction::FilterPoints3DWithLargeReprojectionError(`
`1487`	`1487`	`class Point3D& point3D = Point3D(point3D_id);`
`1488`	`1488`
`1489`	`1489`	`if (point3D.Track().Length() < 2) {`
`1490`		`- DeletePoint3D(point3D_id);`
`1491`	`1490`	`num_filtered += point3D.Track().Length();`
	`1491`	`+ DeletePoint3D(point3D_id);`
`1492`	`1492`	`continue;`
`1493`	`1493`	`}`
`1494`	`1494`