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 #61

Open
eli-schwartz opened this issue Jul 22, 2024 · 2 comments
Open

Build fails with strict-aliasing violations #61

eli-schwartz opened this issue Jul 22, 2024 · 2 comments
Assignees

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

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:

Making all in pcp
make[3]: Entering directory '/var/tmp/portage/dev-db/pgpool2-4.4.7/work/pgpool-II-4.4.7-16/src/libs/pcp'
/bin/sh ../../../libtool  --tag=CC   --mode=compile x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I. -I../../../src/include  -D_GNU_SOURCE -DPOOL_PRIVATE -I /usr/include/postgresql-16   -pipe -march=native -fstack-protector-all -O2 -fdiagnostics-color=always -frecord-gcc-switches -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types -pthread -Wall -Wmissing-prototypes -Wmissing-declarations -Wno-format-truncation -Wno-stringop-truncation  -c -o json.lo json.c
libtool: compile:  x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I. -I../../../src/include -D_GNU_SOURCE -DPOOL_PRIVATE -I /usr/include/postgresql-16 -pipe -march=native -fstack-protector-all -O2 -fdiagnostics-color=always -frecord-gcc-switches -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types -pthread -Wall -Wmissing-prototypes -Wmissing-declarations -Wno-format-truncation -Wno-stringop-truncation -c json.c  -fPIC -DPIC -o .libs/json.o
json.c: In function 'new_value':
json.c:197:65: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  197 |                                 value->_reserved.object_mem = (*(char **) &value->u.object.values) + values_size;
      |                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
json.c: In function 'json_parse_ex':
json.c:487:67: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  487 |                                                                 (*(json_char * *) & top->u.object.values) += string_length + 1;
      |                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [Makefile:483: json.lo] Error 1
make[3]: Leaving directory '/var/tmp/portage/dev-db/pgpool2-4.4.7/work/pgpool-II-4.4.7-16/src/libs/pcp'
make[3]: Target 'all' not remade because of errors.
make[3]: Nothing to be done for 'all-am'.
make[2]: *** [Makefile:386: all-recursive] Error 1
make[2]: Target 'all' not remade because of errors.
Making all in watchdog
make[2]: Nothing to be done for 'all'.
Making all in .
make[2]: Entering directory '/var/tmp/portage/dev-db/pgpool2-4.4.7/work/pgpool-II-4.4.7-16/src'
x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -DDEFAULT_CONFIGDIR=\"/etc/pgpool2\" -DPGSQL_BIN_DIR=\"/usr/lib64/postgresql-16/bin\" -I. -I../src/include  -D_GNU_SOURCE -I /usr/include/postgresql-16   -pipe -march=native -fstack-protector-all -O2 -fdiagnostics-color=always -frecord-gcc-switches -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types -pthread -Wall -Wmissing-prototypes -Wmissing-declarations -Wno-format-truncation -Wno-stringop-truncation  -c -o utils/json.o utils/json.c
utils/json.c: In function ‘new_value’:
utils/json.c:197:65: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  197 |                                 value->_reserved.object_mem = (*(char **) &value->u.object.values) + values_size;
      |                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
utils/json.c: In function ‘json_parse_ex’:
utils/json.c:487:67: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  487 |                                                                 (*(json_char * *) & top->u.object.values) += string_length + 1;
      |                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [Makefile:884: utils/json.o] Error 1
make[2]: Leaving directory '/var/tmp/portage/dev-db/pgpool2-4.4.7/work/pgpool-II-4.4.7-16/src'
make[2]: Target 'all-am' not remade because of errors.
Making all in tools
Making all in pcp
make[3]: *** No rule to make target '../../../src/libs/pcp/libpcp.la', needed by 'pcp_stop_pgpool'.
make[3]: Target 'all' not remade because of errors.
Making all in pgmd5
make[3]: Nothing to be done for 'all'.
Making all in pgenc
make[3]: Nothing to be done for 'all'.
Making all in pgproto
make[3]: Nothing to be done for 'all'.
Making all in watchdog
make[3]: Entering directory '/var/tmp/portage/dev-db/pgpool2-4.4.7/work/pgpool-II-4.4.7-16/src/tools/watchdog'
x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -DDEFAULT_CONFIGDIR=\"/etc/pgpool2\" -DPOOL_TOOLS -I. -I../../../src/include  -D_GNU_SOURCE -DPOOL_PRIVATE -I ../../../src/include/parser -I /usr/include/postgresql-16    -pipe -march=native -fstack-protector-all -O2 -fdiagnostics-color=always -frecord-gcc-switches -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types -pthread -Wall -Wmissing-prototypes -Wmissing-declarations -Wno-format-truncation -Wno-stringop-truncation  -c -o json.o json.c
json.c: In function ‘new_value’:
json.c:197:65: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  197 |                                 value->_reserved.object_mem = (*(char **) &value->u.object.values) + values_size;
      |                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
json.c: In function ‘json_parse_ex’:
json.c:487:67: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  487 |                                                                 (*(json_char * *) & top->u.object.values) += string_length + 1;
      |                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [Makefile:522: json.o] Error 1

Full build log: build.log

Relevant: b412da2

In 2016, -fno-strict-aliasing was added to the entire project with this logic:

flatten_set_variable_args() was imported from PostgreSQL in Pgpool-II 3.5. To make the code work, a compiler flag "-fno-strict-aliasing" is necessary (PostgreSQL does so). Unfortunately when the function was imported, the compiler flag was not added. To fix this, configure.ac was modified.

"It is necessary" is something I doubt. If it's necessary to make the code work, it indicates underlying issues in the code that need to be fixed. But the compiler doesn't warn about it, so it may possibly have been refactored in the past and is no longer relevant.

It would be good to fix the json.c issues independent of flatten_set_variable_args, and ideally drop -fno-strict-aliasing altogether -- it prevents the compiler from engaging in various optimization opportunities -- if it is possible to use conforming C.

@tatsuo-ishii tatsuo-ishii self-assigned this Jul 25, 2024
@tatsuo-ishii
Copy link
Collaborator

I agree that strict-aliasing rules violation should be fixed. I believe utils/json.c was brought in pgpool by @codeforall. Usama, can you look into this?

But the compiler doesn't warn about it, so it may possibly have been refactored in the past and is no longer relevant.

Maybe. Or, the compiler does not warn false positives any more.

BTW, what's "UB"?

@eli-schwartz
Copy link
Author

BTW, what's "UB"?

An acronym for Undefined Behavior -- a term used in the ISO C standard text to describe code which is incompatible with the requirements and constraints of the language, but which is explicitly permitted for the compiler to do "whatever it wants, including to cause demons to fly out of the programmer's nose" rather than diagnosing it as an error.

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

2 participants