Possible eigen unaligned memory access

[mitjap]

I might have found a potential issue with unaligned Eigen objects and std::vector.

I think the issue is with Half_plane and Half_planes definitions https://github.com/openMVG/openMVG/blob/5e98d504bb76ba2d1d07ae80ac2acb10b3d6f97d/src/openMVG/geometry/half_space_intersection.hpp#L27 There are some std::vector specializations https://github.com/openMVG/openMVG/blob/74deb33d12bf275a3b3a9afc833f4760be90f031/src/openMVG/numeric/eigen_alias_definition.hpp#L142 that solve this kind of issues but do not include Eigen::Hyperplane type. The solution would be to add specialization or use using Half_plane = Eigen::Hyperplane<double, 3, DontAlign> as type definition.

[pmoulon]

We are calling this just before https://github.com/openMVG/openMVG/blob/5e98d504bb76ba2d1d07ae80ac2acb10b3d6f97d/src/openMVG/geometry/half_space_intersection.hpp#L14 so I think we should be safe.

Did you encounter any compilation or running time issue?

[mitjap]

You are right. I completely missed this call. I did not encounter any issues, just got suspicious when I was looking at the code.

[pmoulon]

No worries. It is better to double-check!

[mitjap]

I'm reopening this as I think there is another possibility of unaligned memory access. In both cases there are fixed-size Eigen matrices as members and object is dynamically created using new operator. I've not encountered any runtime issues becouse of this.

http://eigen.tuxfamily.org/dox/group__TopicStructHavingEigenMembers.html

What kind of code needs to be changed? you have a class that has as a member a fixed-size vectorizable Eigen object, and then you dynamically create an object of that class.

Eigen might use SIMD instructions when assigning members in constructor (e. g. relative_rotation_(relative_rotation)) which would cause a crash if memory is not aligned.

https://github.com/openMVG/openMVG/blob/5e98d504bb76ba2d1d07ae80ac2acb10b3d6f97d/src/openMVG/sfm/sfm_data_BA_ceres.cpp#L41 https://github.com/openMVG/openMVG/blob/5e98d504bb76ba2d1d07ae80ac2acb10b3d6f97d/src/openMVG/sfm/sfm_data_BA_ceres.cpp#L444

---

https://github.com/openMVG/openMVG/blob/5e98d504bb76ba2d1d07ae80ac2acb10b3d6f97d/src/openMVG/multiview/rotation_averaging_l2.cpp#L192 https://github.com/openMVG/openMVG/blob/5e98d504bb76ba2d1d07ae80ac2acb10b3d6f97d/src/openMVG/multiview/rotation_averaging_l2.cpp#L268

---

https://github.com/openMVG/openMVG/blob/5e98d504bb76ba2d1d07ae80ac2acb10b3d6f97d/src/openMVG/multiview/rotation_averaging_common.hpp#L38 I think this last one should be

using RelativeRotations = std::vector<RelativeRotation, Eigen::aligned_allocator<RelativeRotation>>;
using RelativeRotations_map = std::map<Pair, RelativeRotation, std::less<Pair>, Eigen::aligned_allocator<std::pair<const Pair, RelativeRotation>>>;