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 strict-aliasing violations. #60

Closed
eli-schwartz opened this issue Mar 20, 2024 · 3 comments · Fixed by #61
Closed

Build fails with strict-aliasing violations. #60

eli-schwartz opened this issue Mar 20, 2024 · 3 comments · Fixed by #61

Comments

@eli-schwartz
Copy link

I tried to compile with LTO: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing

The -Werror=* flags are important to detect cases where the compiler can try to optimize based on assuming UB cannot happen, and miscompile code that has UB in it. strict-aliasing issues are always bad but LTO can make them even worse.

I got this error:

x86_64-pc-linux-gnu-g++ -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 -Wno-write-strings -Wno-unused-result  -I. -MD -MF dp_lib/evaluate_dirichlet.deps -c -o dp_lib/evaluate_dirichlet.o dp_lib/evaluate_dirichlet.c
In file included from ./io_lib_header.h:33,
                 from dp_lib/evaluate_dirichlet.c:33:
./io_lib/tree_util.h:30:1: warning: ‘typedef’ was ignored in this declaration
   30 | typedef struct ALNcol
      | ^~~~~~~
dp_lib/evaluate_dirichlet.c: In function ‘double sin_pi(double)’:
dp_lib/evaluate_dirichlet.c:499:14: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  499 |         ix=(*(long long *)&x)>>32;
      |              ^~~~~~~~~~~~~~~
dp_lib/evaluate_dirichlet.c:519:29: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  519 |                         n=(*(long long *)&x);
      |                             ^~~~~~~~~~~~~~~
dp_lib/evaluate_dirichlet.c: In function ‘double lgamma_r(double, int*)’:
dp_lib/evaluate_dirichlet.c:548:14: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  548 |         hx=(*(long long *)&x)>>32;
      |              ^~~~~~~~~~~~~~~
dp_lib/evaluate_dirichlet.c:549:14: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  549 |         lx=(*(long long *)&x);
      |              ^~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors
make: *** [makefile:9: dp_lib/evaluate_dirichlet.o] Error 1

Downstream report: https://bugs.gentoo.org/862327 -- note that I updated to 13.46.0 and still got the same issue.
Full build log: build.log

@cnotred
Copy link
Contributor

cnotred commented Mar 20, 2024 via email

@eli-schwartz
Copy link
Author

eli-schwartz commented Mar 20, 2024

Excellent, thank you for the speedy update.

Now a bit of background for all the clanking things around. What you just stumbled upon is really is legacy code, that made its way in T-Coffee a long time ago (in the 90s I think) and is not really used any more. Much more than a production tool, T-Coffee is a framework through which many things have been tested over time, and can be rapidly re-tested when needed. These things are sometimes implemented in a sub-optimal way, or a deprecated way. I do not think the dirichelet mixtures are used any more in any default mode.

Yup, makes sense. :) Most of the issues we've been uncovering (Gentoo is trying to get all packages to be LTO-clean, since it's something a lot of people want to add to their default *FLAGS, and that involves reporting a lot of bugs) come from legacy code or legacy portions of a codebase. Sadly, it has a tendency to break any code it gets linked with even if it never gets used...

@JoseEspinosa JoseEspinosa mentioned this issue Mar 22, 2024
Merged
@JoseEspinosa
Copy link
Member

Fixed in #61

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 a pull request may close this issue.

3 participants