Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simply creating a GeneralizedIterativeClosestPoint6D (gicp) object causes segmentation fault #2013

Closed
jasjuang opened this issue Sep 27, 2017 · 7 comments · Fixed by #2100
Closed
Labels
kind: bug Type of issue

Comments

@jasjuang
Copy link
Contributor

Your Environment

  • Operating System and version: Ubuntu 16.04
  • Compiler: GCC 5.4
  • PCL Version: Latest

Minimal code to Reproduce

CMakeLists

cmake_minimum_required(VERSION 3.7)
project(Example)

set(PROJECT_SRCS ${PROJECT_SOURCE_DIR}/main.cpp )

find_package( PCL REQUIRED )

add_executable(${PROJECT_NAME} ${PROJECT_SRCS})

target_include_directories(${PROJECT_NAME} SYSTEM PUBLIC                          
 ${PCL_INCLUDE_DIRS})

target_link_libraries(${PROJECT_NAME} ${PCL_LIBRARIES})

main.cpp

#include "pcl/registration/gicp6d.h"

int main(){
  pcl::GeneralizedIterativeClosestPoint6D gicp;

  return 0;
}

Current Behavior

ubuntu@XXX.XXX.XX.XX:~/test/build$ ./Example 
Segmentation fault (core dumped)

Expected Behavior

ubuntu@XXX.XXX.XX.XX:~/test/build$ ./Example 
ubuntu@XXX.XXX.XX.XX:~/test/build$

Context

This problem is preventing me from using the GICP function, which I would like to improve my result by replacing the traditional ICP with GICP.

@MichaelKorn Thank you for adding GICP into PCL, could you help me solve this problem?

@taketwo
Copy link
Member

taketwo commented Sep 28, 2017

FWIW, I get the following output (my environment is the same):

*** Error in `./Example': double free or corruption (out): 0x00007fffbd48d330 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f84e301e7e5]
/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7f84e302737a]
/lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f84e302b53c]
./Example(_ZN9__gnu_cxx13new_allocatorIfE10deallocateEPfm+0x20)[0x4e7310]
./Example(_ZNSt16allocator_traitsISaIfEE10deallocateERS0_Pfm+0x30)[0x4e72e0]
./Example(_ZNSt12_Vector_baseIfSaIfEE13_M_deallocateEPfm+0x3b)[0x4e727b]
./Example(_ZNSt12_Vector_baseIfSaIfEED2Ev+0x29)[0x4e71c9]
./Example(_ZNSt6vectorIfSaIfEED2Ev+0x48)[0x4e6f58]
./Example(_ZN3pcl19PointRepresentationINS_11PointXYZLABEED2Ev+0x27)[0x4e6fa7]
./Example(_ZN3pcl34GeneralizedIterativeClosestPoint6D21MyPointRepresentationD2Ev+0x15)[0x4e6e25]
./Example(_ZN3pcl34GeneralizedIterativeClosestPoint6DD2Ev+0x33)[0x4e6a03]
./Example(main+0x51)[0x4e6891]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f84e2fc7830]
./Example(_start+0x29)[0x4e6769]

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Sep 28, 2017

Valgrind is a big help for memory related problems. I used it sometimes in the past for mem leaks, but I assume its functionalities should also be prepared for double free scenarios.

@SergioRAgostinho SergioRAgostinho added the kind: bug Type of issue label Nov 23, 2017
@SergioRAgostinho SergioRAgostinho added this to the pcl-1.9.0 milestone Nov 23, 2017
@SergioRAgostinho
Copy link
Member

$ valgrind ./gicp_double_free 
==20154== Memcheck, a memory error detector
==20154== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==20154== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==20154== Command: ./gicp_double_free
==20154== 
==20154== Conditional jump or move depends on uninitialised value(s)
==20154==    at 0x528DAB: std::_Vector_base<float, std::allocator<float> >::_M_deallocate(float*, unsigned long) (stl_vector.h:177)
==20154==    by 0x525BA7: std::_Vector_base<float, std::allocator<float> >::~_Vector_base() (stl_vector.h:160)
==20154==    by 0x523A93: std::vector<float, std::allocator<float> >::~vector() (stl_vector.h:425)
==20154==    by 0x523D99: pcl::PointRepresentation<pcl::PointXYZLAB>::~PointRepresentation() (in /home/sergio/Development/pcl_test_cases/gicp_double_free/build/gicp_double_free)
==20154==    by 0x521D7B: pcl::GeneralizedIterativeClosestPoint6D::MyPointRepresentation::~MyPointRepresentation() (gicp6d.h:183)
==20154==    by 0x534846: pcl::GeneralizedIterativeClosestPoint6D::~GeneralizedIterativeClosestPoint6D() (in /home/sergio/Development/pcl_test_cases/gicp_double_free/build/gicp_double_free)
==20154==    by 0x51F50B: main (gicp_double_free.cpp:4)
==20154== 
==20154== 
==20154== Process terminating with default action of signal 11 (SIGSEGV)
==20154==  Bad permissions for mapped region at address 0x5B51F8
==20154==    at 0x51F9B2: boost::detail::atomic_exchange_and_add(int*, int) (sp_counted_base_gcc_x86.hpp:50)
==20154==    by 0x51FAC6: boost::detail::sp_counted_base::release() (sp_counted_base_gcc_x86.hpp:144)
==20154==    by 0x51FB7A: boost::detail::shared_count::~shared_count() (shared_count.hpp:443)
==20154==    by 0x5357C9: boost::shared_ptr<std::vector<int, std::allocator<int> > const>::~shared_ptr() (shared_ptr.hpp:323)
==20154==    by 0x535BA0: boost::shared_ptr<std::vector<int, std::allocator<int> > const>::reset() (shared_ptr.hpp:614)
==20154==    by 0x5358F4: pcl::KdTreeFLANN<pcl::PointXYZLAB, flann::L2_Simple<float> >::cleanup() (kdtree_flann.hpp:220)
==20154==    by 0x5352A6: pcl::KdTreeFLANN<pcl::PointXYZLAB, flann::L2_Simple<float> >::~KdTreeFLANN() (kdtree_flann.h:138)
==20154==    by 0x534858: pcl::GeneralizedIterativeClosestPoint6D::~GeneralizedIterativeClosestPoint6D() (in /home/sergio/Development/pcl_test_cases/gicp_double_free/build/gicp_double_free)
==20154==    by 0x51F50B: main (gicp_double_free.cpp:4)
==20154== 
==20154== HEAP SUMMARY:
==20154==     in use at exit: 325,934 bytes in 2,341 blocks
==20154==   total heap usage: 3,026 allocs, 685 frees, 494,484 bytes allocated
==20154== 
==20154== LEAK SUMMARY:
==20154==    definitely lost: 0 bytes in 0 blocks
==20154==    indirectly lost: 0 bytes in 0 blocks
==20154==      possibly lost: 0 bytes in 0 blocks
==20154==    still reachable: 325,934 bytes in 2,341 blocks
==20154==         suppressed: 0 bytes in 0 blocks
==20154== Rerun with --leak-check=full to see details of leaked memory
==20154== 
==20154== For counts of detected and suppressed errors, rerun with: -v
==20154== Use --track-origins=yes to see where uninitialised values come from
==20154== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Nov 23, 2017

The problem seems not to be in GeneralizedIterativeClosestPoint6D but in GeneralizedIterativeClosestPoint.

@SergioRAgostinho
Copy link
Member

I just made a stripped the GICP6D class to the bare minimum so I can compile the snippet.

  • If I define the constructor inline no segfault and no double free occurs;
  • If not, then the segfault occurs.

I'm starting to think this is an issue with compilation flags.

#define PCL_NO_PRECOMPILE
#include <pcl/registration/gicp.h>

namespace pcl
{
  struct EIGEN_ALIGN16 _PointXYZLAB
  {
    PCL_ADD_POINT4D; // this adds the members x,y,z
    union
    {
      struct
      {
        float L;
        float a;
        float b;
      };
      float data_lab[4];
    };
    EIGEN_MAKE_ALIGNED_OPERATOR_NEW
  };

  /** \brief A custom point type for position and CIELAB color value */
  struct PointXYZLAB : public _PointXYZLAB
  {
    inline PointXYZLAB ()
    {
      x = y = z = 0.0f; data[3]     = 1.0f;  // important for homogeneous coordinates
      L = a = b = 0.0f; data_lab[3] = 0.0f;
    }
  };
}

// register the custom point type in PCL
POINT_CLOUD_REGISTER_POINT_STRUCT(pcl::_PointXYZLAB,
    (float, x, x)
    (float, y, y)
    (float, z, z)
    (float, L, L)
    (float, a, a)
    (float, b, b)
)
POINT_CLOUD_REGISTER_POINT_WRAPPER(pcl::PointXYZLAB, pcl::_PointXYZLAB)

namespace pcl
{
  
  class PCL_EXPORTS GeneralizedIterativeClosestPoint6D : public GeneralizedIterativeClosestPoint<PointXYZRGBA, PointXYZRGBA>
  {
    typedef PointXYZRGBA PointSource;
    typedef PointXYZRGBA PointTarget;

      /** \brief Holds the converted (LAB) data cloud. */
      pcl::PointCloud<PointXYZLAB>::Ptr cloud_lab_;

      /** \brief Holds the converted (LAB) model cloud. */
      pcl::PointCloud<PointXYZLAB>::Ptr target_lab_;

      /** \brief 6d-tree to search in model cloud. */
      KdTreeFLANN<PointXYZLAB> target_tree_lab_;

      /** \brief The color weight. */
      float lab_weight_;

    public:

      /** \brief constructor.
       *
       * \param[in] lab_weight the color weight
       */
      GeneralizedIterativeClosestPoint6D (float lab_weight = 0.032f)
        : cloud_lab_ (new pcl::PointCloud<PointXYZLAB>)
        , target_lab_ (new pcl::PointCloud<PointXYZLAB>)
        , lab_weight_ (lab_weight)
      {
        // set rescale mask (leave x,y,z unchanged, scale L,a,b by lab_weight)
        float alpha[6] = { 1.0, 1.0, 1.0, lab_weight_, lab_weight_, lab_weight_ };
        point_rep_.setRescaleValues (alpha);
      }


    protected:
      /**  \brief Custom point representation to perform kdtree searches in more than 3 (i.e. in all 6) dimensions. */
      class MyPointRepresentation : public PointRepresentation<PointXYZLAB>
      {
          using PointRepresentation<PointXYZLAB>::nr_dimensions_;
          using PointRepresentation<PointXYZLAB>::trivial_;

        public:
          typedef boost::shared_ptr<MyPointRepresentation> Ptr;
          typedef boost::shared_ptr<const MyPointRepresentation> ConstPtr;

          MyPointRepresentation ()
          {
            nr_dimensions_ = 6;
            trivial_ = false;
          }

          virtual
          ~MyPointRepresentation ()
          {
          }

          inline Ptr
          makeShared () const
          {
            return Ptr (new MyPointRepresentation (*this));
          }

          virtual void
          copyToFloatArray (const PointXYZLAB &p, float * out) const
          {
            // copy all of the six values
            out[0] = p.x;
            out[1] = p.y;
            out[2] = p.z;
            out[3] = p.L;
            out[4] = p.a;
            out[5] = p.b;
          }
      };

      /** \brief Enables 6d searches with kd-tree class using the color weight. */
      MyPointRepresentation point_rep_;
  };
}

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Nov 23, 2017

tldr: SSE definitions are not being properly exported anymore.

cd /home/sergio/Development/3rdparty/pcl/build/registration && /usr/bin/c++   -DPCLAPI_EXPORTS -DQT_CORE_LIB -DQT_GUI_LIB -Dqh_QHpointer -isystem /usr/include/eigen3 -isystem /usr/include/ni -isystem /usr/include/openni2 -I/home/sergio/Development/3rdparty/pcl/recognition/include/pcl/recognition/3rdparty -isystem /usr/include/qt4 -isystem /usr/include/qt4/QtGui -isystem /usr/include/qt4/QtCore -I/home/sergio/Development/3rdparty/pcl/build/include -I/home/sergio/Development/3rdparty/pcl/common/include -I/home/sergio/Development/3rdparty/pcl/octree/include -I/home/sergio/Development/3rdparty/pcl/kdtree/include -I/home/sergio/Development/3rdparty/pcl/search/include -I/home/sergio/Development/3rdparty/pcl/sample_consensus/include -I/home/sergio/Development/3rdparty/pcl/features/include -I/home/sergio/Development/3rdparty/pcl/filters/include -I/home/sergio/Development/3rdparty/pcl/registration/include  -Wall -Wextra -Wno-unknown-pragmas -fno-strict-aliasing -Wno-format-extra-args -Wno-sign-compare -Wno-invalid-offsetof -Wno-conversion  -march=native -msse4.2 -mfpmath=sse -Wabi -pthread -fopenmp -g -fPIC   -o CMakeFiles/pcl_registration.dir/src/gicp6d.cpp.o -c /home/sergio/Development/3rdparty/pcl/registration/src/gicp6d.cpp

if I add target_compile_definitions(${PROJECT_NAME} PUBLIC ${PCL_DEFINITIONS}) this is the output

/usr/bin/c++   -D -march="native -msse4.2 -mfpmath=sse " -DDISABLE_DAVIDSDK -DDISABLE_DSSDK -DDISABLE_ENSENSO -DDISABLE_LIBUSB_1_0 -DDISABLE_PCAP -DDISABLE_PNG -DDISABLE_RSSDK -Dqh_QHpointer -I/usr/include/vtk-5.10 -I/home/sergio/Development/3rdparty/pcl/install/include/pcl-1.8 -I/usr/include/eigen3 -I/usr/include/ni -I/usr/include/openni2  -Wno-deprecated -g   -o CMakeFiles/gicp_double_free.dir/gicp_double_free.cpp.o -c /home/sergio/Development/pcl_test_cases/gicp_double_free/gicp_double_free.cpp
<command-line>:0:1: error: macro names must be identifiers
CMakeFiles/gicp_double_free.dir/build.make:62: recipe for target 'CMakeFiles/gicp_double_free.dir/gicp_double_free.cpp.o' failed

If I then compile with the same SSE definitions no more compiler error and not more segfaults

cmake .. -DPCL_DIR=~/Development/3rdparty/pcl/install/share/pcl-1.8/ -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="-march=native -msse4.2 -mfpmath=sse"

@taketwo
Copy link
Member

taketwo commented Sep 8, 2018

In preparation of #2100 merge, I decided to test this again. I can still reproduce this with current master and the code provided by @jasjuang, which is no surprise because it fails to call:

add_definitions(${PCL_DEFINITIONS})

Adding this line to CMakeLists solves the problem for me. So I don't think this can be counted as a bug. That said, of course it's a bit of a friction, and we will remove it with #2100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Type of issue
Projects
None yet
3 participants