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

Build fails with LTO #976

Closed
eli-schwartz opened this issue Mar 11, 2024 · 13 comments · Fixed by #985 or #1028
Closed

Build fails with LTO #976

eli-schwartz opened this issue Mar 11, 2024 · 13 comments · Fixed by #985 or #1028

Comments

@eli-schwartz
Copy link

I tried to build with the following *FLAGS to optimize the build: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing

Note the -Werror=* flags are used to help detect cases where the compiler tries to optimize by assuming UB cannot exist in the source code -- if it does exist, ordinarily the code would be miscompiled, and this says to make the miscompilation a fatal error.

I got this error:

FAILED: _core.cpython-311-x86_64-linux-gnu.so 
: && /usr/bin/x86_64-pc-linux-gnu-g++ -fPIC -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-clash-protection -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -O3 -DNDEBUG  -Wl,-O1 -Wl,--as-needed -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wl,--defsym=__gentoo_check_ldflags__=0   -flto -shared  -o _core.cpython-311-x86_64-linux-gnu.so CMakeFiles/_core.dir/src/application.cpp.o CMakeFiles/_core.dir/src/contours.cpp.o CMakeFiles/_core.dir/src/equal.cpp.o CMakeFiles/_core.dir/src/fcn.cpp.o CMakeFiles/_core.dir/src/functionminimum.cpp.o CMakeFiles/_core.dir/src/functionminimum_extra.cpp.o CMakeFiles/_core.dir/src/functionminimum_pickle.cpp.o CMakeFiles/_core.dir/src/hesse.cpp.o CMakeFiles/_core.dir/src/lasymmatrix.cpp.o CMakeFiles/_core.dir/src/lavector.cpp.o CMakeFiles/_core.dir/src/machineprecision.cpp.o CMakeFiles/_core.dir/src/main.cpp.o CMakeFiles/_core.dir/src/migrad.cpp.o CMakeFiles/_core.dir/src/minimumstate.cpp.o CMakeFiles/_core.dir/src/minos.cpp.o CMakeFiles/_core.dir/src/minuitparameter.cpp.o CMakeFiles/_core.dir/src/print.cpp.o CMakeFiles/_core.dir/src/printimpl.cpp.o CMakeFiles/_core.dir/src/scan.cpp.o CMakeFiles/_core.dir/src/simplex.cpp.o CMakeFiles/_core.dir/src/strategy.cpp.o CMakeFiles/_core.dir/src/usercovariance.cpp.o CMakeFiles/_core.dir/src/userparameterstate.cpp.o CMakeFiles/_core.dir/src/usertransformation.cpp.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/AnalyticalGradientCalculator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/BFGSErrorUpdator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/DavidonErrorUpdator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/HessianGradientCalculator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/ExternalInternalGradientCalculator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/InitialGradientCalculator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/LaEigenValues.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/LaInnerProduct.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/LaInverse.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/LaOuterProduct.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/LaSumOfElements.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/LaVtMVSimilarity.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MPIProcess.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MinimumBuilder.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnApplication.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnContours.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnCovarianceSqueeze.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnFcn.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnFunctionCross.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnGlobalCorrelationCoeff.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnHesse.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnLineSearch.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnMachinePrecision.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnMinos.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnParabolaFactory.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnParameterScan.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnPlot.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnPosDef.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnPrint.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnSeedGenerator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnStrategy.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnScan.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnTiny.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnTraceObject.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnUserFcn.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnUserParameterState.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnUserParameters.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/MnUserTransformation.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/ModularFunctionMinimizer.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/NegativeG2LineSearch.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/Numerical2PGradientCalculator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/SimplexSeedGenerator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/SimplexBuilder.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/SimplexParameters.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/SinParameterTransformation.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/ScanBuilder.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/SqrtLowParameterTransformation.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/SqrtUpParameterTransformation.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/VariableMetricBuilder.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/VariableMetricEDMEstimator.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mnbins.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mndasum.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mndaxpy.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mnddot.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mndscal.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mndspmv.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mndspr.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mnlsame.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mnteigen.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mntplot.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mnvert.cxx.o CMakeFiles/_core.dir/extern/root/math/minuit2/src/mnxerbla.cxx.o   && cd /var/tmp/portage/dev-python/iminuit-2.25.2/work/iminuit-2.25.2/build/cp311-cp311-manylinux_2_38_x86_64 && /usr/bin/x86_64-pc-linux-gnu-strip /var/tmp/portage/dev-python/iminuit-2.25.2/work/iminuit-2.25.2/build/cp311-cp311-manylinux_2_38_x86_64/_core.cpython-311-x86_64-linux-gnu.so
/var/tmp/portage/dev-python/iminuit-2.25.2/work/iminuit-2.25.2/src/type_caster.hpp:10:8: error: type ‘struct type_caster’ violates the C++ One Definition Rule [-Werror=odr]
   10 | struct type_caster<std::vector<Value>> {
      |        ^
/usr/lib/python3.11/site-packages/pybind11/include/pybind11/stl.h:215:8: note: a type with different bases is defined in another translation unit
  215 | struct type_caster<std::vector<Type, Alloc>> : list_caster<std::vector<Type, Alloc>, Type> {};
      |        ^
lto1: some warnings being treated as errors
lto-wrapper: fatal error: /usr/bin/x86_64-pc-linux-gnu-g++ returned 1 exit status
compilation terminated.
/usr/lib/gcc/x86_64-pc-linux-gnu/13/../../../../x86_64-pc-linux-gnu/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

Downstream bug report: https://bugs.gentoo.org/856778
Full build log: build.log

@eli-schwartz
Copy link
Author

Unfortunately it appears that against my will, cmake is forcing it to build with LTO even when I remove it from my *FLAGS.

Which means that the resulting build artifacts are still broken, and since I didn't remove -Werror=odr, I just removed -flto, the build still fails -- as it should.

@HDembinski
Copy link
Member

This seems like an issue in pybind11, maybe you can report it upstream?

@eli-schwartz
Copy link
Author

The issue is a type in iminuit that clashes with a type in pybind11.

It's not immediately obvious to me why that is the responsibility of pybind11. Can you clarify?

@HDembinski
Copy link
Member

Ok, you are right, I missed that in the error message. I am working on a fix.

HDembinski added a commit that referenced this issue Apr 15, 2024
Closes #976 

This fixes an ODR violation and adds more warnings regarding ODR
violations to the standard build flags (which are treated as errors).
@eli-schwartz
Copy link
Author

Sorry for the lateness of the reply... this didn't actually fix it for me at the time.

2.26

/var/tmp/portage/dev-python/iminuit-2.26.0/work/iminuit-2.26.0/src/type_caster.hpp:9:8: error: type ‘struct type_caster’ violates the C++ One Definition Rule [-Werror=odr]
    9 | struct type_caster<std::vector<Value, Alloc>> {
      |        ^
/usr/lib/python3.12/site-packages/pybind11/include/pybind11/stl.h:215:8: note: a type with different bases is defined in another translation unit
  215 | struct type_caster<std::vector<Type, Alloc>> : list_caster<std::vector<Type, Alloc>, Type> {};
      |        ^
lto1: some warnings being treated as errors

2.28

/var/tmp/portage/dev-python/iminuit-2.28.0/work/iminuit-2.28.0/src/type_caster.hpp:9:8: error: type ‘struct type_caster’ violates the C++ One Definition Rule [-Werror=odr]
    9 | struct type_caster<std::vector<double>> {
      |        ^
/usr/lib/python3.12/site-packages/pybind11/include/pybind11/stl.h:215:8: note: a type with different bases is defined in another translation unit
  215 | struct type_caster<std::vector<Type, Alloc>> : list_caster<std::vector<Type, Alloc>, Type> {};
      |        ^
lto1: some warnings being treated as errors

I cannot tell from CI whether gcc or clang is being used -- uv pip install provides no output other than the names of packages installed, usually having compile logs for at least your own project is handy for debugging and introspection.

@HDembinski HDembinski reopened this Aug 12, 2024
@HDembinski
Copy link
Member

This is strange. In my patch, I activated strong warnings and errors in the GNU build and then fixed the issue. The issue occurs because I am overriding a template specialization of pybind11 in some translation units, which to my understanding is not UB and not a ODR violation. The compiler seems to agree.

@ArsenArsen
Copy link

ArsenArsen commented Aug 12, 2024

you need LTO for -Wodr and -Wlto-type-mismatch to produce a diagnostic

The issue occurs because I am overriding a template specialization of pybind11 in some translation units, which to my understanding is not UB and not a ODR violation.

I think this is a case of [temp.expl.spec]p7 (an instantiation of type_caster<std::vector<double>> happens in TU X, and in TU Y you use your explicit specialization - please correct me if I'm wrong)

@eli-schwartz
Copy link
Author

In my patch, I activated strong warnings and errors in the GNU build and then fixed the issue.

I only see it in compile opts, not in link opts. It is the linker stage that compares LTO bytecode across multiple TUs to perform whole-program optimization, and in the process checks whether the resulting multi-TU analysis has ODR issues or type mismatches between implementations in one file and callers in another file.

(LTO gives the compiler driver another chance to detect several types of warnings that are easier to detect at link time than at compile time. -Wmaybe-uninitialized is another warning that can start showing up at link time when not originally found at compile time.)

Also:

I cannot tell from CI whether gcc or clang is being used -- uv pip install provides no output other than the names of packages installed, usually having compile logs for at least your own project is handy for debugging and introspection.

The reason I mention this is because clang has weaker diagnostics for odr and doesn't support lto-type-mismatch at all, so I don't like to assume it will even notice the problem. My tests used GCC.

@HDembinski
Copy link
Member

Thank you for this insightful comment, that's very useful for me. I will come back to this issue, right now I am working on completing another patch.

@HDembinski
Copy link
Member

I cannot tell from CI whether gcc or clang is being used -- uv pip install provides no output other than the names of packages installed, usually having compile logs for at least your own project is handy for debugging and introspection.

@henryiii You introduced uv to this repo. How can we get the compiler messages back during the package build?

HDembinski added a commit that referenced this issue Aug 25, 2024
… 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`
@eli-schwartz
Copy link
Author

Thanks, I confirm it works for me now.

@HDembinski
Copy link
Member

Excellent, glad to hear it.

@henryiii
Copy link
Member

henryiii commented Sep 5, 2024

You introduced uv to this repo. How can we get the compiler messages back during the package build?

The latest versions of uv (from a day or two ago) should now include streaming build output in the --verbose logs, or by default with the new uv build command. There's a bit of a formatting issue, but it's there now.

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