[filters] Potentially undefined behavior in VoxelGridCovariance

[thallerod]

Undefined behavior if floating-point expression doesn't fit into an int. https://github.com/PointCloudLibrary/pcl/blob/0932486c52a2cf4f0821e25d5ea2d5767fff8381/filters/include/pcl/filters/impl/voxel_grid_covariance.hpp#L91

[mvieth]

Hi, how did you become aware that this could be a problem? Did you encounter this in practice, or was there a compiler warning, or something else? And why exactly would it not fit into an int? Because min_p is very large, or because inverse_leafsize is very large?

[thallerod]

Purely by inspection. But something like this would trigger it:

#include <iostream>
#include <pcl/filters/voxel_grid_covariance.h>

int dummy(double offset, double leafsize) {
  pcl::PointCloud<pcl::PointXYZ>::Ptr pc(new pcl::PointCloud<pcl::PointXYZ>);

  for(int i = 0; i < 10; ++i)
  {
    pc->emplace_back(offset + i, 0.0, 0.0);
  }

  pcl::VoxelGridCovariance<pcl::PointXYZ> vgc;
  vgc.setInputCloud(pc);
  vgc.setLeafSize(leafsize, 1.0, 1.0);
  vgc.filter();
  return vgc.getLeaves().size();
}

int main()
{
  std::cout << dummy(1e5, 1e-3) << "\n";
  std::cout << dummy(1e6, 1e-3) << "\n";
  std::cout << dummy(1e7, 1e-3) << "\n";  // (offset / leafsize) > std::numeric_limits<int>::max()
  std::cout << dummy(1e6, 1e-4) << "\n";  // (offset / leafsize) > std::numeric_limits<int>::max()
}

Output (expecting 10 leaves):

10
10
1
1

[mvieth]

Thanks for the additional information. So a combination of a point cloud that is far away from (0, 0, 0), and a small leaf size could potentially trigger this. I will look into it further when I have time. Maybe, instead of using min_b_, it might be an option to subtract min_p from every point, then multiply with inverse_leaf_size_. But we need to make sure that this does not change the behaviour of VoxelGridCovariance, and does not make it slower.