Undefined Behaviour - Null pointer passed as argument to memset.

[RLThomaz]

Environment

• Operating System and version: Ubuntu 18.04 running on a Docker container
• Compiler: g++-9 with flags -O3 -fsanitize=undefined
• PCL Version: PCL 1.8 (the one available by default on Ubuntu 18.04)

Context

I'm cleaning up any possible undefined behaviour (UB) in my project thus I activated the g++ undefined behaviour sanitiser (UBSAN). My project uses PCL's Poisson (pcl/surface/poisson) for surface reconstruction, which is returning a runtime error due to UB detected by UBSAN. The UB is related to the resize of a vector type defined by the Poisson module itself. The line memset(m_pV, 0, N*sizeof(T) ); is the offending line here since the pointer m_pV can be a nullptr in this call.

template<class T>
void Vector<T>::Resize( size_t N )
{
  if(m_N!=N){
    if(m_N){delete[] m_pV;}
    m_pV=NULL;
    m_N = N;
    if(N){m_pV = new T[N];}
  }
  memset( m_pV, 0, N*sizeof(T) );
}

Expected Behavior

The code still works as expected. However, since it is a UB, we cannot assume that this will hold true for all systems.

• No runtime error.

Current Behavior

/usr/include/pcl-1.8/pcl/surface/3rdparty/poisson4/vector.hpp:71:13: runtime error: null pointer passed as argument 1, which is declared to never be null

Code to Reproduce

Compile any code using Poisson with -O3 and -fsanitze=undefined flags.

Possible Solution

Simply check if m_pV is not a nullptr before calling memset.

template<class T>
void Vector<T>::Resize( size_t N )
{
  if(m_N!=N){
    if(m_N){delete[] m_pV;}
    m_pV=NULL;
    m_N = N;
    if(N){m_pV = new T[N];}
  }
 if (m_pV == nullptr)
 {
     // throw error here
 }
  memset( m_pV, 0, N*sizeof(T) );
}

[kunaltyagi]

Thanks. In future, just create a PR with the proposed change. Easier for everyone

[RLThomaz]

Sorry about that, I'm not familiar enough with PCL's standard for throwing errors, or even if an error should be thrown there. Next time I'll try making a PR.

[kunaltyagi]

No harm done. Do check the PR if it suffices.

I'm not familiar enough with PCL's standard for throwing errors

Someone will correct you if you're doing it wrong. Nobody is out for blood here.