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 failure with -Werror=strict-aliasing: ic_predict.c:58:16: error: dereferencing type-punned pointer will break strict-aliasing rules #163

Closed
kostadinsh opened this issue Jul 1, 2023 · 3 comments

Comments

@kostadinsh
Copy link

kostadinsh commented Jul 1, 2023

faad2 fails to build with -Werror=strict-aliasing added to CFLAGS and CXXFLAGS

gcc version: (Gentoo 13.1.1_p20230527 p3) 13.1.1 20230527

[  6%] Building C object CMakeFiles/faad.dir/libfaad/ic_predict.c.o
/usr/bin/cc -DAPPLY_DRC -DHAVE_INTTYPES_H=1 -DHAVE_MEMCPY=1 -DHAVE_STRINGS_H=1 -DHAVE_STRING_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_SYS_TYPES_H=1 -DPACKAGE_VERSION=\"2.10.1\" -DSTDC_HEADERS=1 -I/home/kostadin/faad2/build/include -I/home/kostadin/faad2/libfaad -O2 -Werror=strict-aliasing -Wall -pedantic -ffloat-store -MD -MT CMakeFiles/faad.dir/libfaad/ic_predict.c.o -MF CMakeFiles/faad.dir/libfaad/ic_predict.c.o.d -o CMakeFiles/faad.dir/libfaad/ic_predict.c.o -c /home/kostadin/faad2/libfaad/ic_predict.c
/home/kostadin/faad2/libfaad/ic_predict.c: In function ‘flt_round’:
/home/kostadin/faad2/libfaad/ic_predict.c:58:16: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   58 |         *pf = *(float32_t*)&tmp1 + *(float32_t*)&tmp2 - *(float32_t*)&tmp;
      |                ^~~~~~~~~~~~~~~~~
/home/kostadin/faad2/libfaad/ic_predict.c:58:37: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   58 |         *pf = *(float32_t*)&tmp1 + *(float32_t*)&tmp2 - *(float32_t*)&tmp;
      |                                     ^~~~~~~~~~~~~~~~~
/home/kostadin/faad2/libfaad/ic_predict.c:58:58: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   58 |         *pf = *(float32_t*)&tmp1 + *(float32_t*)&tmp2 - *(float32_t*)&tmp;
      |                                                          ^~~~~~~~~~~~~~~~
/home/kostadin/faad2/libfaad/ic_predict.c:60:16: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   60 |         *pf = *(float32_t*)&tmp;
      |                ^~~~~~~~~~~~~~~~
/home/kostadin/faad2/libfaad/ic_predict.c: In function ‘inv_quant_pred’:
/home/kostadin/faad2/libfaad/ic_predict.c:80:12: warning: ‘x’ is used uninitialized [-Wuninitialized]
   80 |     return x;
      |            ^
/home/kostadin/faad2/libfaad/ic_predict.c:76:15: note: ‘x’ declared here
   76 |     float32_t x;
      |               ^
cc1: some warnings being treated as errors

Steps to reproduce:

  1. clone the repo
  2. cd into the faad2 folder
  3. mkdir build && cd build
  4. run CFLAGS="-O2 -Werror=strict-aliasing" CXXFLAGS="-O2 -Werror=strict-aliasing" cmake ..
  5. run make VERBOSE=1
  6. Build failure

Gentoo bug: https://bugs.gentoo.org/859844

@thesamesam
Copy link

FWIW, the getdword_n bits in a6ed518 look like aliasing volations too. I suspect https://github.com/projg2/portable-endianness can help there.

@eustas
Copy link
Contributor

eustas commented Jul 6, 2023

In that change I've just moved getdword_n in source code, but agree, it looks horrible at least - both type-punning and fall-through... Going to rewrite it. And take care of flt_round soon.

eustas added a commit to eustas/faad2 that referenced this issue Jul 7, 2023
Use memcpy instead of type-punning. Compiler knows that this is just a cast.
@thesamesam
Copy link

thesamesam commented Jul 9, 2023

Thank you! (Yes, sorry, I was lazy with the link. Oops.)

Much cleaner now and I think some of the big-endian paths weren't right before either (indices looked possibly a bit off), so a double win! Many thanks.

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

No branches or pull requests

3 participants