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 due to type mismatch/ODR violations #1783

Open
eli-schwartz opened this issue Aug 20, 2024 · 6 comments
Open

Build fails with LTO due to type mismatch/ODR violations #1783

eli-schwartz opened this issue Aug 20, 2024 · 6 comments

Comments

@eli-schwartz
Copy link

Problem description

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

Link-Time Optimization is a massively global compiler optimization pass which is pretty handy for producing faster executables. It also has the interesting property that because the compiler does whole-program analysis using bytecode, it can save type information and perform error checks that it normally doesn’t have enough insight for. In particular, checking for ODR issues and checking function type signature mismatches.

Note that all 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:

x86_64-pc-linux-gnu-g++ -o build/python/cantera/_cantera.cpython-312-x86_64-linux-gnu.so -Wl,-O1 -Wl,--as-needed -Wl,-z,pack-relative-relocs -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wl,--defsym=__gentoo_check_ldflags__=0 -pthread -shared -Wl,-rpath=/usr/lib64/cantera -Wl,-rpath=/usr/lib build/temp-py/_cantera.os build/temp-py/_onedim.os build/temp-py/_utils.os build/temp-py/constants.os build/temp-py/delegator.os build/temp-py/func1.os build/temp-py/kinetics.os build/temp-py/mixture.os build/temp-py/preconditioners.os build/temp-py/reaction.os build/temp-py/reactionpath.os build/temp-py/reactor.os build/temp-py/solutionbase.os build/temp-py/speciesthermo.os build/temp-py/thermo.os build/temp-py/transport.os build/temp-py/units.os build/temp-py/yamlwriter.os src/extensions/PythonExtensionManager.os -Lbuild/lib -L/usr/lib64/cantera -lcantera -lfmt -lgomp
src/fortran/cantera_thermo.f90:49:54: error: type of ‘th_newfromfile’ does not match original declaration [-Werror=lto-type-mismatch]
   49 |          self%thermo_id = th_newfromfile(filename, id)
      |                                                      ^
src/fortran/fct.cpp:368:13: note: type mismatch in parameter 3
  368 |     integer th_newfromfile_(char* filename, char* phasename, ftnlen lenf, ftnlen lenp)
      |             ^
src/fortran/fct.cpp:368:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:368:13: note: ‘th_newfromfile_’ was previously declared here
src/fortran/fct.cpp:368:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:66:46: error: type of ‘phase_getname’ does not match original declaration [-Werror=lto-type-mismatch]
   66 |       call phase_getname(self%thermo_id, name)
      |                                              ^
src/fortran/fct.cpp:101:14: note: return value type mismatch
  101 |     status_t phase_getname_(const integer* n, char* nm,
      |              ^
src/fortran/fct.cpp:101:14: note: type ‘status_t’ should match type ‘void’
src/fortran/fct.cpp:101:14: note: ‘phase_getname_’ was previously declared here
src/fortran/fct.cpp:101:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:123:68: error: type of ‘phase_elementindex’ does not match original declaration [-Werror=lto-type-mismatch]
  123 |       ctthermo_elementindex = phase_elementindex(self%thermo_id, nm)
      |                                                                    ^
src/fortran/fct.cpp:191:13: note: type mismatch in parameter 3
  191 |     integer phase_elementindex_(const integer* n, char* nm, ftnlen lennm)
      |             ^
src/fortran/fct.cpp:191:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:191:13: note: ‘phase_elementindex_’ was previously declared here
src/fortran/fct.cpp:191:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:130:68: error: type of ‘phase_speciesindex’ does not match original declaration [-Werror=lto-type-mismatch]
  130 |       ctthermo_speciesindex = phase_speciesindex(self%thermo_id, nm)
      |                                                                    ^
src/fortran/fct.cpp:201:13: note: type mismatch in parameter 3
  201 |     integer phase_speciesindex_(const integer* n, char* nm, ftnlen lennm)
      |             ^
src/fortran/fct.cpp:201:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:201:13: note: ‘phase_speciesindex_’ was previously declared here
src/fortran/fct.cpp:201:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:179:64: error: type of ‘phase_setmolefractionsbyname’ does not match original declaration [-Werror=lto-type-mismatch]
  179 |       self%err = phase_setmolefractionsbyname(self%thermo_id, x)
      |                                                                ^
src/fortran/fct.cpp:266:14: note: type mismatch in parameter 3
  266 |     status_t phase_setmolefractionsbyname_(const integer* n, char* x, ftnlen lx)
      |              ^
src/fortran/fct.cpp:266:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:266:14: note: ‘phase_setmolefractionsbyname_’ was previously declared here
src/fortran/fct.cpp:266:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:200:64: error: type of ‘phase_setmassfractionsbyname’ does not match original declaration [-Werror=lto-type-mismatch]
  200 |       self%err = phase_setmassfractionsbyname(self%thermo_id, y)
      |                                                                ^
src/fortran/fct.cpp:292:14: note: type mismatch in parameter 3
  292 |     status_t phase_setmassfractionsbyname_(const integer* n, char* y, ftnlen leny)
      |              ^
src/fortran/fct.cpp:292:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:292:14: note: ‘phase_setmassfractionsbyname_’ was previously declared here
src/fortran/fct.cpp:292:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:222:60: error: type of ‘phase_getspeciesname’ does not match original declaration [-Werror=lto-type-mismatch]
  222 |       self%err = phase_getspeciesname(self%thermo_id, k, nm)
      |                                                            ^
src/fortran/fct.cpp:327:14: note: type mismatch in parameter 4
  327 |     status_t phase_getspeciesname_(const integer* n, integer* k, char* nm, ftnlen lennm)
      |              ^
src/fortran/fct.cpp:327:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:327:14: note: ‘phase_getspeciesname_’ was previously declared here
src/fortran/fct.cpp:327:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:230:60: error: type of ‘phase_getelementname’ does not match original declaration [-Werror=lto-type-mismatch]
  230 |       self%err = phase_getelementname(self%thermo_id, m, nm)
      |                                                            ^
src/fortran/fct.cpp:342:14: note: type mismatch in parameter 4
  342 |     status_t phase_getelementname_(const integer* n, integer* m, char* nm, ftnlen lennm)
      |              ^
src/fortran/fct.cpp:342:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:342:14: note: ‘phase_getelementname_’ was previously declared here
src/fortran/fct.cpp:342:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:245:50: error: type of ‘th_geteostype’ does not match original declaration [-Werror=lto-type-mismatch]
  245 |       self%err = th_geteostype(self%thermo_id, nm)
      |                                                  ^
src/fortran/fct.cpp:379:13: note: type mismatch in parameter 3
  379 |     integer th_geteostype_(const integer* n, char* buf, ftnlen lenbuf)
      |             ^
src/fortran/fct.cpp:379:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:379:13: note: ‘th_geteostype_’ was previously declared here
src/fortran/fct.cpp:379:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_thermo.f90:501:119: error: type of ‘th_equil’ does not match original declaration [-Werror=lto-type-mismatch]
  501 |       self%err = th_equil(self%thermo_id, XY, trim(solver_), rtol_, max_steps_, max_iter_, estimate_equil_, log_level_)
      |                                                                                                                       ^
src/fortran/fct.cpp:566:14: note: type mismatch in parameter 9
  566 |     status_t th_equil_(const integer* n, char* XY, char* solver, double* rtol, int* max_steps, int* max_iter, int* estimate_equil, int* log_level, ftnlen lenxy, ftnlen lensolver)
      |              ^
src/fortran/fct.cpp:566:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:566:14: note: ‘th_equil_’ was previously declared here
src/fortran/fct.cpp:566:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_kinetics.f90:46:56: error: type of ‘kin_newfromfile’ does not match original declaration [-Werror=lto-type-mismatch]
   46 |                                          n1, n2, n3, n4)
      |                                                        ^
src/fortran/fct.cpp:679:13: note: type mismatch in parameter 8
  679 |     integer kin_newfromfile_(const char* filename, const char* phasename,
      |             ^
src/fortran/fct.cpp:679:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:679:13: note: ‘kin_newfromfile_’ was previously declared here
src/fortran/fct.cpp:679:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_kinetics.f90:58:45: error: type of ‘kin_gettype’ does not match original declaration [-Werror=lto-type-mismatch]
   58 |       self%err = kin_gettype(self%kin_id, nm)
      |                                             ^
src/fortran/fct.cpp:708:13: note: type mismatch in parameter 3
  708 |     integer kin_gettype_(const integer* n, char* buf, ftnlen buflen)
      |             ^
src/fortran/fct.cpp:708:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:708:13: note: ‘kin_gettype_’ was previously declared here
src/fortran/fct.cpp:708:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_kinetics.f90:73:77: error: type of ‘kin_speciesindex’ does not match original declaration [-Werror=lto-type-mismatch]
   73 |       ctkin_kineticsSpeciesIndex = kin_speciesindex(self%kin_id, name, phase)
      |                                                                             ^
src/fortran/fct.cpp:726:13: note: type mismatch in parameter 4
  726 |     integer kin_speciesindex_(const integer* n, const char* nm, const char* ph,
      |             ^
src/fortran/fct.cpp:726:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:726:13: note: ‘kin_speciesindex_’ was previously declared here
src/fortran/fct.cpp:726:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_kinetics.f90:98:58: error: type of ‘kin_phaseindex’ does not match original declaration [-Werror=lto-type-mismatch]
   98 |       ctkin_phaseindex = kin_phaseindex(self%kin_id, name)
      |                                                          ^
src/fortran/fct.cpp:766:13: note: type mismatch in parameter 3
  766 |     integer kin_phaseindex_(const integer* n, const char* ph, ftnlen lenph)
      |             ^
src/fortran/fct.cpp:766:13: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:766:13: note: ‘kin_phaseindex_’ was previously declared here
src/fortran/fct.cpp:766:13: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_kinetics.f90:122:57: error: type of ‘kin_getreactiontype’ does not match original declaration [-Werror=lto-type-mismatch]
  122 |       self%err = kin_getreactiontype(self%kin_id, i, buf)
      |                                                         ^
src/fortran/fct.cpp:793:14: note: type mismatch in parameter 4
  793 |     status_t kin_getreactiontype_(const integer* n, integer* i, char* buf, ftnlen lenbuf)
      |              ^
src/fortran/fct.cpp:793:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:793:14: note: ‘kin_getreactiontype_’ was previously declared here
src/fortran/fct.cpp:793:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_kinetics.f90:193:59: error: type of ‘kin_getreactionstring’ does not match original declaration [-Werror=lto-type-mismatch]
  193 |       self%err = kin_getreactionstring(self%kin_id, i, buf)
      |                                                           ^
src/fortran/fct.cpp:903:14: note: type mismatch in parameter 4
  903 |     status_t kin_getreactionstring_(const integer* n, integer* i, char* buf, ftnlen lenbuf)
      |              ^
src/fortran/fct.cpp:903:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:903:14: note: ‘kin_getreactionstring_’ was previously declared here
src/fortran/fct.cpp:903:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_funcs.f90:30:66: error: type of ‘trans_newdefault’ does not match original declaration [-Werror=lto-type-mismatch]
   30 |          self%tran_id = trans_newdefault(self%thermo_id, loglevel)
      |                                                                  ^
src/fortran/fct.cpp:960:13: note: type mismatch in parameter 3
  960 |     integer trans_newdefault_(integer* ith, integer* loglevel, ftnlen lenmodel)
      |             ^
src/fortran/fct.cpp:960:13: note: type ‘ftnlen’ should match type ‘void’
src/fortran/fct.cpp:960:13: note: ‘trans_newdefault_’ was previously declared here
src/fortran/cantera_funcs.f90:78:35: error: type of ‘ctgetcanteraerror’ does not match original declaration [-Werror=lto-type-mismatch]
   78 |       ierr = ctgetCanteraError(buf)
      |                                   ^
src/fortran/fct.cpp:1089:14: note: type mismatch in parameter 2
 1089 |     status_t ctgetcanteraerror_(char* buf, ftnlen buflen)
      |              ^
src/fortran/fct.cpp:1089:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:1089:14: note: ‘ctgetcanteraerror_’ was previously declared here
src/fortran/fct.cpp:1089:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
src/fortran/cantera_funcs.f90:90:59: error: type of ‘ctaddcanteradirectory’ does not match original declaration [-Werror=lto-type-mismatch]
   90 |       self%err = ctaddCanteraDirectory(self%thermo_id, buf)
      |                                                           ^
src/fortran/fct.cpp:1105:14: note: type mismatch in parameter 3
 1105 |     status_t ctaddcanteradirectory_(integer* buflen, char* buf)
      |              ^
src/fortran/fct.cpp:1105:14: note: type ‘void’ should match type ‘long int’
src/fortran/fct.cpp:1105:14: note: ‘ctaddcanteradirectory_’ was previously declared here
src/fortran/cantera_funcs.f90:68:68: error: type of ‘ctphase_report’ does not match original declaration [-Werror=lto-type-mismatch]
   68 |          self%err = ctphase_report(self%thermo_id, buf, show_thermo)
      |                                                                    ^
src/fortran/fct.cpp:1070:14: note: type mismatch in parameter 4
 1070 |     status_t ctphase_report_(const integer* nth,
      |              ^
src/fortran/fct.cpp:1070:14: note: type ‘ftnlen’ should match type ‘long int’
src/fortran/fct.cpp:1070:14: note: ‘ctphase_report_’ was previously declared here
src/fortran/fct.cpp:1070:14: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used
lto1: some warnings being treated as errors
build/python/cantera/_cantera.cpp:2340:3: error: type ‘struct __pyx_mstate’ violates the C++ One Definition Rule [-Werror=odr]
 2340 | } __pyx_mstate;
      |   ^
lto-wrapper: fatal error: x86_64-pc-linux-gnu-gfortran returned 1 exit status
compilation terminated.
/usr/libexec/gcc/x86_64-pc-linux-gnu/ld: error: lto-wrapper failed
build/python/cantera/_onedim.cpp:5467:3: note: a different type is defined in another translation unit
 5467 | } __pyx_mstate;
      |   ^
build/python/cantera/_cantera.cpp:2265:13: note: the first difference of corresponding definitions is field ‘__pyx_n_s_’
 2265 |   PyObject *__pyx_n_s_;
      |             ^
build/python/cantera/_onedim.cpp:4614:17: note: a field with different name is defined in another translation unit
 4614 |   PyTypeObject *__pyx_GeneratorType;
      |                 ^
collect2: error: ld returned 1 exit status
scons: *** [build/lib/libcantera_fortran.so.3.0.1] Error 1
lto1: some warnings being treated as errors
lto-wrapper: fatal error: x86_64-pc-linux-gnu-g++ returned 1 exit status
compilation terminated.
/usr/libexec/gcc/x86_64-pc-linux-gnu/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
scons: *** [build/python/cantera/_cantera.cpython-312-x86_64-linux-gnu.so] Error 1
scons: done building targets (errors occurred during build).

Steps to reproduce

Compile with GCC.

Include the following CFLAGS / CXXFLAGS / LDFLAGS: -flto -Werror=odr -Werror=lto-type-mismatch

System information

  • Cantera version: 3.0.1
  • OS: Gentoo Linux
  • Python/MATLAB/other software versions:
    gcc (Gentoo 13.3.1_p20240614 p17) 13.3.1 20240614
    Python 3.12.3

Attachments

Full build log of the failure: build.log

@ischoegl
Copy link
Member

ischoegl commented Aug 20, 2024

Thanks for the issue report. All the examples above (which one exception for Python) appear to be from the Fortran interface. Cantera using mismatched types for integers in Fortran is something that is known (it may actually be a deliberate hack). Can you confirm that the

build/python/cantera/_onedim.cpp:5467:3: note: a different type is defined in another translation unit
 5467 | } __pyx_mstate;
      |   ^
build/python/cantera/_cantera.cpp:2265:13: note: the first difference of corresponding definitions is field ‘__pyx_n_s_’
 2265 |   PyObject *__pyx_n_s_;
      |             ^
build/python/cantera/_onedim.cpp:4614:17: note: a field with different name is defined in another translation unit
 4614 |   PyTypeObject *__pyx_GeneratorType;
      |       

block is the only one remaining if you compile without Fortran support? I.e. using the f90_interface=n option?

@eli-schwartz
Copy link
Author

All the examples above (which one exception for Python) appear to be from the Fortran interface. Cantera using mismatched types for integers in Fortran is something that is known (it may actually be a deliberate hack).

Yeah, it does seem to show up in a lot of packages. :)

Sometimes they do get fixed after my report though -- in at least one case, that involved adopting the use of Fortran 2003 "BIND(C)". I don't really understand the details very well since I don't know Fortran. :( But e.g. the GCC docs talk about it as "the" facility for ensuring interoperability between C and Fortran interfaces when producing shared libraries. Maybe that's an option here?

@eli-schwartz
Copy link
Author

$ USE=-fortran emerge -1 cantera

[...]

x86_64-pc-linux-gnu-g++ -o build/python/cantera/_cantera.cpython-312-x86_64-linux-gnu.so -Wl,-O1 -Wl,--as-needed -Wl,-z,pack-relative-relocs -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wl,--defsym=__gentoo_check_ldflags__=0 -pthread -shared -Wl,-rpath=/usr/lib64/cantera -Wl,-rpath=/usr/lib build/temp-py/_cantera.os build/temp-py/_onedim.os build/temp-py/_utils.os build/temp-py/constants.os build/temp-py/delegator.os build/temp-py/func1.os build/temp-py/kinetics.os build/temp-py/mixture.os build/temp-py/preconditioners.os build/temp-py/reaction.os build/temp-py/reactionpath.os build/temp-py/reactor.os build/temp-py/solutionbase.os build/temp-py/speciesthermo.os build/temp-py/thermo.os build/temp-py/transport.os build/temp-py/units.os build/temp-py/yamlwriter.os src/extensions/PythonExtensionManager.os -Lbuild/lib -L/usr/lib64/cantera -lcantera -lfmt -lgomp
build/python/cantera/_cantera.cpp:2340:3: error: type ‘struct __pyx_mstate’ violates the C++ One Definition Rule [-Werror=odr]
 2340 | } __pyx_mstate;
      |   ^
build/python/cantera/_onedim.cpp:5467:3: note: a different type is defined in another translation unit
 5467 | } __pyx_mstate;
      |   ^
build/python/cantera/_cantera.cpp:2265:13: note: the first difference of corresponding definitions is field ‘__pyx_n_s_’
 2265 |   PyObject *__pyx_n_s_;
      |             ^
build/python/cantera/_onedim.cpp:4614:17: note: a field with different name is defined in another translation unit
 4614 |   PyTypeObject *__pyx_GeneratorType;
      |                 ^
lto1: some warnings being treated as errors
lto-wrapper: fatal error: x86_64-pc-linux-gnu-g++ returned 1 exit status
compilation terminated.
/usr/libexec/gcc/x86_64-pc-linux-gnu/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
scons: *** [build/python/cantera/_cantera.cpython-312-x86_64-linux-gnu.so] Error 1
scons: building terminated because of errors.

Can you confirm that the [...] block is the only one remaining if you compile without Fortran support? I.e. using the f90_interface=n option?

Yes, as long as the build is without fortran support it is just the one error.

@ischoegl
Copy link
Member

Thanks for confirming!

@speth
Copy link
Member

speth commented Aug 20, 2024

The warnings about __pyx_mstate are related to the somewhat unconventional way that we use Cython. To reduce compilation time, we compile multiple separate .pyx files, which Cython expects will end up as their own separate extension module .so, but we instead combine everything into a single extension module.

All usage of this struct seems to be guarded by the CYTHON_USE_MODULE_STATE macro, which is by default defined as 0, and is described in the documentation as an experimental feature. I don't know if this option will perhaps become a default at some point in the future, or what it would take to support our use case. Perhaps that's a question for the Cython devs.

@eli-schwartz
Copy link
Author

The warnings about __pyx_mstate are related to the somewhat unconventional way that we use Cython. To reduce compilation time, we compile multiple separate .pyx files, which Cython expects will end up as their own separate extension module .so, but we instead combine everything into a single extension module.

That is a very interesting use case which does seem valuable. Another possibility from the cython side could be if they offered the ability to simply allow producing "_part1.cpp", "_part2.cpp" files when processing pyx files that produce especially large generated outputs. I would probably open a feature request if I were you. :)

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

No branches or pull requests

3 participants