Conversation
0c85ae4 to
6bcc532
Compare
| esac | ||
|
|
||
| CC="${HOST:+$HOST-}gcc" | ||
| CC="${HOST:+$HOST-}gcc${cc_flags}" |
There was a problem hiding this comment.
Little worried that "$CC" foo will now break in non-obvious ways and only when building x86 binaries. :/
There was a problem hiding this comment.
I dunno, info autoconf says (in CFLAGS description):
If it affects only the compiler proper, ‘CFLAGS’ is the natural home for it. If an option affects multiple phases of the compiler, though, matters get tricky:
• If an option selects a 32-bit or 64-bit build on a bi-arch system, it must be put direcly into ‘CC’, e.g., ‘CC='gcc -m64'’. This is necessary for ‘config.guess’ to work right.
• Otherwise one approach is to put the option into ‘CC’. Another is to put it into both ‘CPPFLAGS’ and ‘LDFLAGS’, but not into ‘CFLAGS’.
this is exactly what we do here.
There was a problem hiding this comment.
Ah if you mean if we'll quote $CC and it will fail in a non-obvious way -- I think it will fail in a very obvious way, saying something like:
bash: gcc -m32 -march=i686: command not found...
There was a problem hiding this comment.
I mean, if autoconf says to do that then who am I to argue?
When cross-building libseccomp, build_libseccomp resets CFLAGS to an empty value (if it was unset) before each architecture, and set_cross_vars exports it. Autoconf only applies its default CFLAGS (-g -O2) when CFLAGS is unset, so the per-architecture libseccomp builds (which are what release binaries link against, including for the native architecture) end up unoptimized. The only reason CFLAGS was touched at all is that set_cross_vars puts -m32 -march=... there for 386, which is why callers had to save and restore it. Pass these flags via CC instead (as autoconf documentation suggests for multilib builds), so set_cross_vars no longer modifies CFLAGS, and remove the now-unneeded save/restore dance from both build-seccomp.sh and release_build.sh. This way, a user-provided CFLAGS is passed as is, and if it is unset, configure uses its defaults for all architectures, including 386. Note that go build does not use CFLAGS (cgo uses CGO_CFLAGS), and it handles CC with arguments just fine. Reported-by: Ciprian Hacman <ciprian@hakman.dev> Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
6bcc532 to
78c3b60
Compare
This is an alternative to #5456.
When cross-building libseccomp,
build_libseccompresetsCFLAGSto an empty value (if it was unset) before each architecture, andset_cross_varsexports it. Autoconf only applies its defaultCFLAGS(-g -O2) whenCFLAGSis unset, so the per-architecture libseccomp builds (which release binaries link against, including for the native architecture) end up unoptimized. See #5456 for performance numbers.The only reason
CFLAGSwas touched at all is thatset_cross_varsputs-m32 -march=...there for 386, which is why callers had to save and restore it. Pass these flags viaCCinstead (as autoconf documentation suggests for multilib builds), soset_cross_varsno longer modifiesCFLAGS, and remove the now-unneeded save/restore from bothbuild-seccomp.shandrelease_build.sh.This way, a user-provided
CFLAGSis passed as is, and if it is unset, configure uses its defaults for all architectures, including 386 (which #5456 does not fix, sinceset_cross_varsmakesCFLAGSnon-empty there).Tested libseccomp 2.6.0 configure (on Debian trixie) with the new
set_cross_vars:CC='x86_64-linux-gnu-gcc -m32 -march=i686',CFLAGS='-g -O2';CC='x86_64-linux-gnu-gcc',CFLAGS='-g -O2'.