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

Workaround for misalignment of SubmapSlice and Eigen data structures in heterogeneous builds #1910

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

twdragon
Copy link

Due to different structure alignment rules in different compilers (or even the same compiler in different compiling contexts), the SubmapSlice structure defined in ./cartographer/io/submap_painter.h comes into cartographer_ros or another external project misaligned by 48 bytes because of different sizeof() return values (224 vs 256 bytes). This behaviour is proven when testing Cartographer compiled using Clang 15 and GCC 11.3 on Mint Linux 22 with a customized build of ROS. The reason is deeply sitting problem with implementation-defined STL shipped with different kernels and build pipelines. This pull request adds a semi-portable workaround for this issue forcing alignment of SubmapSlice structure to 32 bytes as is required by GCC.

  • Added a workaround for misaligned SubmapSlice structure between cartographer and cartographer_ros built using different compilers or compilation contexts
  • Rearranged SubmapSlice field order after static analysis performed with clangd
  • Added commented non-portable workaround with #pragma pack
  • Closes Crash due to unexpected free() on cairo_data #1909

	- Added a workaround for misaligned SubmapSlice
	  structure between cartographer and cartographer_ros
	  built using different compilers or compile contexts
	- Rearranged SubmapSlice field order after static
	  analysis performed with clangd
	- Added commented non-portable workaround with
	  \#pragma pack
	- Closes cartographer-project#1909

Signed-off-by: Andrey Vukolov <andrey.vukolov@elettra.eu>
@twdragon twdragon changed the title Workaround for misalignment SubmapSlice in heterogeneous builds Workaround for misalignment of SubmapSlice in heterogeneous builds Nov 11, 2022
@twdragon
Copy link
Author

This issue persists also in other parts of the project, so I will continue the investigation

	- Adopted C++17 standard, removed CXX standard requirement
	- Removed Abseil's absl::unique_ptr in favour of std::unique_ptr
	  producing errors when built on Ubuntu 22 with fresh Abseil built
	  from source
	- Added DISABLE_AVX_ALIGNMENT CMake option to prevent Eigen
	  (especially built from source) from misalignment of the data
	  structures passed outside the library namespace on the machines
	  with modern CPUs providing AVX vectorization out of the box
	- Tested on Intel Core-i7 6700, any performance decreaseing was
	  not discovered

Signed-off-by: Andrey Vukolov <andrey.vukolov@elettra.eu>
@twdragon twdragon changed the title Workaround for misalignment of SubmapSlice in heterogeneous builds Workaround for misalignment of SubmapSlice and Eigen data structures in heterogeneous builds Nov 14, 2022
@twdragon
Copy link
Author

Updated involving the DISABLE_AVX_ALIGNMENT CMake option to disable Eigen-side AVX alignment issues on recent CPUs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash due to unexpected free() on cairo_data
1 participant