Skip to content

Conversation

MaximSmolskiy
Copy link

-- The C compiler identification is GNU 11.4.0
-- The CXX compiler identification is GNU 11.4.0
In file included from /home/mithridat/Downloads/pcl/recognition/src/implicit_shape_model.cpp:43:
/home/mithridat/Downloads/pcl/recognition/include/pcl/recognition/impl/implicit_shape_model.hpp: In member function ‘void pcl::features::ISMVoteList<PointT>::findStrongestPeaks(std::vector<pcl::ISMPeak, Eigen::aligned_allocator<pcl::ISMPeak> >&, int, double, double) [with PointT = pcl::PointXYZ]’:
/home/mithridat/Downloads/pcl/recognition/include/pcl/recognition/impl/implicit_shape_model.hpp:174:21: warning: ‘*(float*)((char*)&strongest_peak + offsetof(Eigen::Vector3f, Eigen::Matrix<float, 3, 1, 0, 3, 1>::<unnamed>.Eigen::PlainObjectBase<Eigen::Matrix<float, 3, 1, 0, 3, 1> >::<unnamed>.Eigen::MatrixBase<Eigen::Matrix<float, 3, 1, 0, 3, 1> >::<unnamed>.Eigen::DenseBase<Eigen::Matrix<float, 3, 1, 0, 3, 1> >::<unnamed>.Eigen::DenseCoeffsBase<Eigen::Matrix<float, 3, 1, 0, 3, 1>, 3>::<unnamed>.Eigen::DenseCoeffsBase<Eigen::Matrix<float, 3, 1, 0, 3, 1>, 1>::<unnamed>.Eigen::DenseCoeffsBase<Eigen::Matrix<float, 3, 1, 0, 3, 1>, 0>::<unnamed>))’ may be used uninitialized [-Wmaybe-uninitialized]
  174 |     Eigen::Vector3f strongest_peak;
      |                     ^~~~~~~~~~~~~~
/home/mithridat/Downloads/pcl/recognition/include/pcl/recognition/impl/implicit_shape_model.hpp:174:21: warning: ‘strongest_peak.Eigen::Matrix<float, 3, 1, 0, 3, 1>::<unnamed>.Eigen::PlainObjectBase<Eigen::Matrix<float, 3, 1, 0, 3, 1> >::m_storage.Eigen::DenseStorage<float, 3, 3, 1, 0>::m_data.Eigen::internal::plain_array<float, 3, 0, 0>::array[1]’ may be used uninitialized [-Wmaybe-uninitialized]

Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your pull request. I am pretty sure that the warning is a false positive: strongest_peak is only read from if peak_counter is not zero (line 191), which can only happen if strongest_peak was set to something valid (lines 185 and 187). I am assuming you are testing with PCL 1.14.0 or newer, see 053995b
I was also not able to reproduce the warning with GCC 11.5.0, so perhaps the maybe-uninitialized detection logic has been improved between the two compiler versions?
If you would still like to initialize strongest_peak with -1, please add an additional check in line 191, testing that strongest_peak is now different from -1 (to have a consistent logic)

@MaximSmolskiy
Copy link
Author

Thank you for your pull request. I am pretty sure that the warning is a false positive: strongest_peak is only read from if peak_counter is not zero (line 191), which can only happen if strongest_peak was set to something valid (lines 185 and 187).

Yes, I also think that warning is false positive. And I think it's better to get rid of such warnings if it's not too difficult

I am assuming you are testing with PCL 1.14.0 or newer, see 053995b

Yes, I tested master branch

If you would still like to initialize strongest_peak with -1, please add an additional check in line 191, testing that strongest_peak is now different from -1 (to have a consistent logic)

There are four related variables - best_density, strongest_peak, best_peak_ind and peak_counter.
So, it's better to add additional checks for all of them in line 191 or leave as is - only for peak_counter?

@MaximSmolskiy MaximSmolskiy requested a review from mvieth August 23, 2025 18:29
@mvieth
Copy link
Member

mvieth commented Aug 24, 2025

There are four related variables - best_density, strongest_peak, best_peak_ind and peak_counter. So, it's better to add additional checks for all of them in line 191 or leave as is - only for peak_counter?

I would at least like a check for strongest_peak, e.g. something like if( peak_counter == 0 || (strongest_peak(0) == -1.0f && strongest_peak(1) == -1.0f && strongest_peak(2) == -1.0f)) break;
You are right, a similar argument could be made for best_density and best_peak_ind, so feel free to add similar checks for them too, if you like.

Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, looks good now.

@mvieth mvieth added this to the pcl-1.16.0 milestone Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants