Fix ODR issue in type_caster which could result in UB when -flto is…

… enabled (#1028)

Closes #976 

One could expose the issue (before the fix) on macOS after installing
gcc-14 with homebrew with the command
`CXX=gcc-14 python -m build -w`

.gitignore

@@ -20,6 +20,9 @@ __pycache__
 *.o
 *.pkl
 
+CMakeCache.txt
+CMakeFiles/
+
 pip-wheel-metadata
 
 bench/*.svg

CMakeLists.txt

@@ -9,12 +9,14 @@ if(NOT DEFINED SKBUILD)
   )
 endif()
 
+include(CheckIPOSupported)
+check_ipo_supported(RESULT ipo_supported OUTPUT error)
+
 set(CMAKE_CXX_STANDARD
     14
     CACHE STRING "C++ version selection") # or 11, 14, 17, 20
 set(CMAKE_CXX_STANDARD_REQUIRED ON) # optional, ensure standard is supported
 set(CMAKE_CXX_EXTENSIONS OFF) # optional, keep compiler extensionsn off
-
 set(PYBIND11_FINDPYTHON ON)
 find_package(pybind11 CONFIG REQUIRED)
 
@@ -84,6 +86,15 @@ set(SOURCES_B
     extern/root/math/minuit2/src/mnxerbla.cxx)
 
 pybind11_add_module(_core MODULE ${SOURCES_A} ${SOURCES_B})
+
+target_include_directories(_core PRIVATE extern/root/math/minuit2/inc)
+target_compile_definitions(_core PUBLIC PYBIND11_DETAILED_ERROR_MESSAGES=1)
+set_target_properties(_core PROPERTIES VISIBILITY_INLINES_HIDDEN ON)
+if(ipo_supported)
+  set_target_properties(_core PROPERTIES INTERPROCEDURAL_OPTIMIZATION TRUE)
+endif()
+
+# Add compiler-specific extra warnings
 if(MSVC)
   target_compile_options(_core PRIVATE /std:c++14 /Y-)
 elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
@@ -92,19 +103,18 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
     _core
     PRIVATE -Wall
             -Wextra
-            -pedantic
             -Werror
-            -fstrict-aliasing
             -Werror=odr
             -Werror=lto-type-mismatch
-            -Werror=strict-aliasing)
+            -Werror=strict-aliasing
+            -pedantic
+            -fstrict-aliasing)
+  target_link_options(_core PRIVATE -Werror -Wodr -Wlto-type-mismatch)
 else()
   # lots of warnings from clang
   target_compile_options(_core PRIVATE -Wall -Wextra -pedantic -Werror
-                                       -fstrict-aliasing)
+                                       -Werror=odr -fstrict-aliasing)
+  target_link_options(_core PRIVATE -Werror -Wodr)
 endif()
-target_include_directories(_core PRIVATE extern/root/math/minuit2/inc)
-set_target_properties(_core PROPERTIES VISIBILITY_INLINES_HIDDEN ON)
-target_compile_definitions(_core PUBLIC PYBIND11_DETAILED_ERROR_MESSAGES=1)
 
 install(TARGETS _core DESTINATION iminuit)

src/application.cpp

@@ -1,6 +1,6 @@
+#include "pybind11.hpp"
 #include <Minuit2/FunctionMinimum.h>
 #include <Minuit2/MnApplication.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/contours.cpp

@@ -1,9 +1,8 @@
+#include "pybind11.hpp"
 #include <Minuit2/ContoursError.h>
 #include <Minuit2/FCNBase.h>
 #include <Minuit2/FunctionMinimum.h>
 #include <Minuit2/MnContours.h>
-#include <pybind11/pybind11.h>
-#include <pybind11/stl.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/fcn.cpp

@@ -1,12 +1,10 @@
 #include "fcn.hpp"
-#include "type_caster.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/FCNGradientBase.h>
 #include <Minuit2/MnPrint.h>
 #include <cmath>
 #include <cstdint>
 #include <limits>
-#include <pybind11/numpy.h>
-#include <pybind11/pybind11.h>
 #include <sstream>
 #include <vector>
 

src/fcn.hpp

@@ -1,6 +1,6 @@
+#include "pybind11.hpp"
 #include <Minuit2/FCNGradientBase.h>
 #include <cstdint>
-#include <pybind11/pytypes.h>
 #include <vector>
 
 namespace py = pybind11;

src/functionminimum.cpp

@@ -1,16 +1,13 @@
 #include "equal.hpp"
 #include "fcn.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/FunctionMinimum.h>
 #include <Minuit2/MinimumSeed.h>
 #include <Minuit2/MinimumState.h>
 #include <Minuit2/MnStrategy.h>
 #include <Minuit2/MnUserFcn.h>
 #include <Minuit2/MnUserParameterState.h>
-#include <pybind11/operators.h>
-#include <pybind11/pybind11.h>
-#include <pybind11/stl.h>
 #include <type_traits>
-#include <vector>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/functionminimum_extra.cpp

@@ -1,5 +1,6 @@
 #include "equal.hpp"
 #include "fcn.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/AnalyticalGradientCalculator.h>
 #include <Minuit2/FunctionMinimum.h>
 #include <Minuit2/MinimumState.h>
@@ -10,7 +11,6 @@
 #include <Minuit2/MnUserParameterState.h>
 #include <Minuit2/Numerical2PGradientCalculator.h>
 #include <Minuit2/VariableMetricEDMEstimator.h>
-#include <pybind11/pybind11.h>
 #include <type_traits>
 #include <vector>
 

src/functionminimum_pickle.cpp

@@ -1,9 +1,8 @@
+#include "pybind11.hpp"
 #include <Minuit2/FunctionMinimum.h>
 #include <Minuit2/MinimumSeed.h>
 #include <Minuit2/MinimumState.h>
 #include <Minuit2/MnUserParameterState.h>
-#include <pybind11/pybind11.h>
-#include <pybind11/stl.h>
 #include <type_traits>
 #include <vector>
 

src/hesse.cpp

@@ -1,9 +1,9 @@
+#include "pybind11.hpp"
 #include <Minuit2/FCNBase.h>
 #include <Minuit2/FunctionMinimum.h>
 #include <Minuit2/MnHesse.h>
 #include <Minuit2/MnUserFcn.h>
 #include <Minuit2/MnUserParameterState.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/lasymmatrix.hpp

@@ -1,8 +1,8 @@
 #ifndef IMINUIT_LASYMMATRIX
 #define IMINUIT_LASYMMATRIX
 
+#include "pybind11.hpp"
 #include <Minuit2/LASymMatrix.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/lavector.hpp

@@ -1,8 +1,8 @@
 #ifndef IMINUIT_LAVECTOR
 #define IMINUIT_LAVECTOR
 
+#include "pybind11.hpp"
 #include <Minuit2/LAVector.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/machineprecision.cpp

@@ -1,6 +1,5 @@
+#include "pybind11.hpp"
 #include <Minuit2/MnMachinePrecision.h>
-#include <pybind11/operators.h>
-#include <pybind11/pybind11.h>
 
 namespace ROOT {
 namespace Minuit2 {

src/main.cpp

@@ -1,4 +1,4 @@
-#include <pybind11/pybind11.h>
+#include "pybind11.hpp"
 
 namespace py = pybind11;
 

src/migrad.cpp

@@ -1,7 +1,7 @@
 #include "fcn.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/FCNBase.h>
 #include <Minuit2/MnMigrad.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/minimumstate.cpp

@@ -2,8 +2,6 @@
 #include "lavector.hpp"
 #include <Minuit2/MinimumState.h>
 #include <cmath>
-#include <pybind11/pybind11.h>
-#include <pybind11/stl.h>
 #include <type_traits>
 
 namespace py = pybind11;

src/minos.cpp

@@ -1,9 +1,9 @@
+#include "pybind11.hpp"
 #include <Minuit2/FCNBase.h>
 #include <Minuit2/FunctionMinimum.h>
 #include <Minuit2/MinosError.h>
 #include <Minuit2/MnMinos.h>
 #include <Minuit2/MnStrategy.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/minuitparameter.cpp

@@ -1,7 +1,6 @@
 #include "equal.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/MinuitParameter.h>
-#include <pybind11/operators.h>
-#include <pybind11/pybind11.h>
 
 namespace ROOT {
 namespace Minuit2 {

src/print.cpp

@@ -1,5 +1,5 @@
+#include "pybind11.hpp"
 #include <Minuit2/MnPrint.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/printimpl.cpp

@@ -1,5 +1,5 @@
 #include "Minuit2/MnPrint.h"
-#include <pybind11/pybind11.h>
+#include "pybind11.hpp"
 
 using ROOT::Minuit2::MnPrint;
 

src/type_caster.hpp → src/pybind11.hpp

@@ -1,10 +1,22 @@
+#ifndef PYBIND11_HPP
+#define PYBIND11_HPP
+
+// We must load pybind11 consistently through this header to ensure that our type_caster
+// specialization is used in every translation unit. Otherwise we get ODR violations.
+
 #include <pybind11/numpy.h>
+#include <pybind11/operators.h>
 #include <pybind11/pybind11.h>
+#include <pybind11/pytypes.h>
+#include <pybind11/stl.h>
 #include <vector>
 
 namespace pybind11 {
 namespace detail {
 
+// This specialization for vector<double> acts as an override from the more generic
+// template in pybind11/stl.h. We use it to cast the C++ vector into a numpy array
+// instead of a Python list.
 template <>
 struct type_caster<std::vector<double>> {
   using vec_t = std::vector<double>;
@@ -27,7 +39,6 @@ struct type_caster<std::vector<double>> {
     return false;
   }
 
-public:
   template <typename T>
   static handle cast(T&& src, return_value_policy, handle) {
     array_t<double> arr(static_cast<ssize_t>(src.size()));
@@ -40,3 +51,5 @@ struct type_caster<std::vector<double>> {
 
 } // namespace detail
 } // namespace pybind11
+
+#endif // PYBIND11_HPP

src/scan.cpp

@@ -1,7 +1,7 @@
+#include "pybind11.hpp"
 #include <Minuit2/FCNBase.h>
 #include <Minuit2/MnScan.h>
 #include <Minuit2/MnUserParameterState.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/simplex.cpp

@@ -1,7 +1,7 @@
 #include "fcn.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/FCNBase.h>
 #include <Minuit2/MnSimplex.h>
-#include <pybind11/pybind11.h>
 
 namespace py = pybind11;
 using namespace ROOT::Minuit2;

src/strategy.cpp

@@ -1,7 +1,6 @@
 #include "equal.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/MnStrategy.h>
-#include <pybind11/operators.h>
-#include <pybind11/pybind11.h>
 
 namespace ROOT {
 namespace Minuit2 {

src/usercovariance.cpp

@@ -1,8 +1,6 @@
 #include "equal.hpp"
-#include "type_caster.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/MnUserCovariance.h>
-#include <pybind11/operators.h>
-#include <pybind11/pybind11.h>
 #include <vector>
 
 namespace ROOT {

src/userparameterstate.cpp

@@ -1,8 +1,6 @@
 #include "equal.hpp"
-#include "type_caster.hpp"
+#include "pybind11.hpp"
 #include <Minuit2/MnUserParameterState.h>
-#include <pybind11/operators.h>
-#include <pybind11/pybind11.h>
 #include <type_traits>
 
 namespace ROOT {

src/usertransformation.cpp

@@ -1,6 +1,5 @@
+#include "pybind11.hpp"
 #include <Minuit2/MnUserTransformation.h>
-#include <pybind11/pybind11.h>
-#include <pybind11/stl.h>
 #include <type_traits>
 
 namespace py = pybind11;