From d1af9449e1aea5240b3073e5adfd9dcaf549ad58 Mon Sep 17 00:00:00 2001 From: Markus Vieth Date: Tue, 26 Nov 2024 20:11:59 +0100 Subject: [PATCH] Print warning if incompatible alignment option chosen Users repeatedly report segmentation faults coming from Memory.h in the Eigen code (aligned_free/handmade_aligned_free). This is because PCL uses Eigen's handmade_aligned_malloc but user code does not use the corresponding handmade_aligned_free (or the other way around), due to chosen SSE/AVX flags and the resulting alignment requirements. PCL's CMake config handles this by automatically setting SSE/AVX options in the user project as they are used in PCL, but not everyone uses CMake. This commit adds a check within PCL's header files for this kind of incompatibility. At first I considered solving theses incompatibilities automatically by setting EIGEN_MALLOC_ALREADY_ALIGNED and EIGEN_MAX_ALIGN_BYTES, but that would have been more complex and error-prone, so I decided only for checking and warning. I would have preferred `#warning` which however is not available on MSVC. `#pragma message` is an option on MSVC, but this is too easily overlooked IMO. Users have the option to silence this by defining the `PCL_SILENCE_MALLOC_WARNING` macro (in case of false positive). --- CMakeLists.txt | 15 +++++++++++++++ common/include/pcl/memory.h | 15 +++++++++++++++ pcl_config.h.in | 2 ++ 3 files changed, 32 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4799eee0938..a2b10fc2d77 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -440,6 +440,21 @@ if(WITH_SYSTEM_CJSON) endif() endif() +set(CMAKE_REQUIRED_LIBRARIES Eigen3::Eigen) # so that Eigen/Core is found below +CHECK_CXX_SOURCE_COMPILES(" +#include +#if (EIGEN_DEFAULT_ALIGN_BYTES==0) || EIGEN_MALLOC_ALREADY_ALIGNED +#error Eigen will not use handmade_aligned_malloc (which is fine, we just throw an error here to make this compilation fail) +#endif +int main() { return 0; }" +PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC) +if (PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC) # CHECK_CXX_SOURCE_COMPILES does not necessarily set this to 0 or 1 + set(PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC 1) +else() + set(PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC 0) +endif() +unset(CMAKE_REQUIRED_LIBRARIES) + ### ---[ Create the config.h file set(pcl_config_h_in "${CMAKE_CURRENT_SOURCE_DIR}/pcl_config.h.in") set(pcl_config_h "${CMAKE_CURRENT_BINARY_DIR}/include/pcl/pcl_config.h") diff --git a/common/include/pcl/memory.h b/common/include/pcl/memory.h index d42e47aa42e..08686547d24 100644 --- a/common/include/pcl/memory.h +++ b/common/include/pcl/memory.h @@ -44,6 +44,7 @@ */ #include // for has_custom_allocator +#include // for PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC #include // for EIGEN_MAKE_ALIGNED_OPERATOR_NEW @@ -51,6 +52,20 @@ #include // for std::enable_if_t, std::false_type, std::true_type #include // for std::forward +#if !defined(PCL_SILENCE_MALLOC_WARNING) +#if PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC +// EIGEN_DEFAULT_ALIGN_BYTES and EIGEN_MALLOC_ALREADY_ALIGNED will be set after including Eigen/Core +// this condition is the same as in the function aligned_malloc in Memory.h in the Eigen code +#if (defined(EIGEN_DEFAULT_ALIGN_BYTES) && EIGEN_DEFAULT_ALIGN_BYTES==0) || (defined(EIGEN_MALLOC_ALREADY_ALIGNED) && EIGEN_MALLOC_ALREADY_ALIGNED) +#error "Potential runtime error due to aligned malloc mismatch! You likely have to compile your code with AVX enabled or define EIGEN_MAX_ALIGN_BYTES=32" +#endif +#else // PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC +#if (defined(EIGEN_DEFAULT_ALIGN_BYTES) && EIGEN_DEFAULT_ALIGN_BYTES!=0) && (defined(EIGEN_MALLOC_ALREADY_ALIGNED) && !EIGEN_MALLOC_ALREADY_ALIGNED) +#error "Potential runtime error due to aligned malloc mismatch! PCL was likely compiled without AVX support but you enabled AVX for your code" +#endif +#endif // PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC +#endif // !defined(PCL_SILENCE_MALLOC_WARNING) + /** * \brief Macro to signal a class requires a custom allocator * diff --git a/pcl_config.h.in b/pcl_config.h.in index abed8381b3b..2be34b19b9f 100644 --- a/pcl_config.h.in +++ b/pcl_config.h.in @@ -35,6 +35,8 @@ #endif //PCL_MINOR_VERSION #endif +#define PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC ${PCL_USES_EIGEN_HANDMADE_ALIGNED_MALLOC} + #cmakedefine HAVE_OPENNI 1 #cmakedefine HAVE_OPENNI2 1