Menu
▾
▴
#40 ibmtss is compiled with -O0
It is generally good idea to let gcc or the user to pick optimization level.
If your code does not work with optimization it is usually broken (unless you hit a gcc bug).
-O0 conflicts with hardening features leading to
[ 53s] /bin/sh ../libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I.. -fPIC -DTPM_INTERFACE_TYPE_DEFAULT="\"dev\"" -DTPM_DEVICE_DEFAULT="\"/dev/tpmrm0\"" -DTPM_TPM20 -DTPM_TPM12 -DTPM_POSIX -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g -g -ggdb -O0 -c -o libibmtss_la-tsscrypto.lo test -f 'tsscrypto.c' || echo './'tsscrypto.c
[ 53s] libtool: compile: gcc -DHAVE_CONFIG_H -I. -I.. -fPIC -DTPM_TPM20 -DTPM_TPM12 -DTPM_POSIX -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g -g -ggdb -O0 -c eventlib.c -fPIC -DPIC -o .libs/libibmtssutils_la-eventlib.o
[ 53s] In file included from /usr/include/bits/libc-header-start.h:33,
[ 53s] from /usr/include/stdio.h:27,
[ 53s] from eventlib.c:39:
[ 53s] /usr/include/features.h:397:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp]
[ 53s] 397 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
[ 53s] | ^~~~~~~
patch
I would like to understand the rationale for this.
The intent is that, without --enable-debug, it compiles for optimization, -O2, This is the default and is regression tested.
With the --enable-debug option, it is compiled with -O0. In my experience, -O0 works better with source level debuggers. Why would that be an issue? This was done so that end users could step into the TSS with a debugger, not because anything is broken with optimization.
yes, -O0 works better with debuggers. But it generates code different from default and from other flags specified by the user. If the compiler does the code generation correctly the semantic should be the same but the disassembly will not. Also the timing will change which metters for some bugs. So it may be desirable to biuld with -O0 at some times and not others.
I can understand that you can think --enable-debug as a debug profile similar to what Visual Studio has which applies a complete set of instrumentation options and compiler flags which can be modified only by creating another modified profile.
The configure script genereated by autotools is more flexible, though. It can pick up compiler flags set by the user in the environment. In fact you can see in the log snippet that the user set -O2 but the configure code you added overrides it to -O0. You do not want that.
Also in the same log snippet you can see that compiler complains that the semantic of FORTIFY_SOURCE cannot be upheld with -O0 which means that even the semantics of the code will be different from a version without --enable-debug. You do not want that for sure.
Finally --enable-debug is the default for distribution builds because the packaging scripts then separate the binaries and the debug data and package them separately. Then the user installs the binaries and in case of a crash or malfunction can also install debug data to provide intelligible bug reports. You want --enable-debug to do whatever is necessary to produce debug data for the binaries in your project and nothing more. If additional instrumentation options are available they should have separate configure script options that can be enabled and disabled separately.
I understand gcc optimizations, but I don't understand the bug report. If we always turn off -O0, the user can't get a non-optimized build that can be used with a source level debugger. So I don't think the patch by itself does what we want.
Perhaps you're saying that --enable-debug was the wrong autotools flag, and that there was a better way to do this. Let me know.
In the end, (I think) we want the default to be -O2, with an option for -O0.
My thought was that distros would just use the defaults, but users could build differently.
To make it completely clear: the use is free to set
CFLAGS=-O0
or
CFLAGS="-O6 -funroll-loops"
and the configure script has no business overriding that.
yes, the way to set -O0 is to set CFLAGS=-O0 which is completely independent of --enable-debug
Alternative patch