Build with -O2 by default. #2988

This is cfields' preferred stopgap until we sort out how we want debug vs release setup.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/e00d03efc3cea1aa92da6287ffb167ef8a51af4f for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

ACK

Why do we need a stopgap?

@gavinandresen this was a bug that slipped in, -O2 was supposed to be in the original autotools PR. Apparently some of the hardening options create strange warning/errors when combined with no optims.

I proposed that now was a good time to evaluate how debug/release usage should work (or if the distinction even needs to be available via configure options) now that we're beginning to deviate from the old build. A good example is OSX, which is currently (as it was before) vastly different between debug/release builds, meaning that devs rarely see what end users will see.

Stopgap is just to put the old behavior back rather than having fishy builds in the meantime.

"some of the hardening options create strange warnings/errors with no -O" : that makes me nervous. Is that documented behavior? A bug in some version of gcc? Are we using that gcc for pull-tester or gitian?

Sorry for being grumpy, but that's why I don't like stopgap solutions; they tend to become "Oh, we fixed that" when really it is "Oh, we swept that problem under the rug so we didn't have to figure out what is really broken."

https://isisblogs.poly.edu/2011/04/11/fortify_source-semantics/ -> YOU MUST TURN ON OPTIMIZATION -01 OR GREATER FOR FORTIFY_SOURCE TO WORK.

stopgap is really not the right description here. This returns us to the previous working behavior. It's only stopgap in that I have a desire to change that previous behavior.

From gcc man pages:

NOTE: In Ubuntu 8.10 and later versions, -D_FORTIFY_SOURCE=2 is set by default, and is activated when -O is set to 2 or higher. This enables additional compile-time and run-time checks for several libc functions. To disable, specify either -U_FORTIFY_SOURCE or -D_FORTIFY_SOURCE=0.

Seems wrong to add -O2 to ./configure --enable-debug. I know I've had issues in the past trying to step through -g -O2 compiled code in the debugger.

Is the real issue that --enable-debug and --enable-hardening are incompatible? It'd be fine with me if the default was release builds, and if you want a debug build you must --enable-debug --disable-hardening.

@gavinandresen I would agree with that, yes. Hardening and debugging pretty much conflict as concepts.

But if we go that route, I would go further as to say that there's no longer any need for the hardening option, it's simply always on in release mode and never in debug.

ACK from me on always-harden-release and removing the --enable-hardening option. Good Idea.

@gavinandresen ok, then the last hurdle is figuring out what to do with OSX, since debug/release vary so much. I was hoping to bite this off in different chunks, but I suppose it's better worth doing as a whole.

OSX is currently back-compat to 10.5. As you've mentioned, this should change to 10.6. Unfortunately, 10.6 still supports 32bit cpus, so release builds will need to remain 32bit. And in that case, debug should be 32bit as well.

So that means that the following move from release flags to always-on flags: -mmacosx-version-min=10.5 -arch i386 -> -mmacosx-version-min=10.6 -arch i386

Additionally, osx has historically built with -O3 for release. For the sake of consistency, I'd prefer to move all release to -O2 or -O3 (no preference which).

I think it would be better to drop -arch and -mmacosx-version-min entirely from the mac builds.

I'll still build releases on my 32-bit OSX 10.6 machine. Eventually maybe we'll cross-compile releases in a gitian VM. (both of those cases will likely need their own wacky flags).

In any case, I don't want to make it difficult for OSX developers by having them either constantly overriding the default ./configure settings or recompiling all of their dependencies -arch i386.

I disagree very strongly with that. That means that every dev is targeting different architectures and sdks with their builds, meaning that the release binary will be completely different (read: different compiler, min sdk, target sdk, and architecture) than what devs have tested. That is a huge amount of trust to be putting into toolchains and compliant code, doubly so given Apple's whimsical replace-the-build-stack-for-each-release nature.

If the reason for not building with the release-platform by default is because it's too inconvenient for devs to be bothered, then in my opinion the correct question to ask is: should we support that platform? If so, then I think it's crucial for devs to be targetting it, otherwise it would be a stretch to call it supported.

For a bit of perspective, this would be equivalent to most devs working on win7/win8 building for native 64bit with msvc++ and backwards compatibility off, but releasing relatively untested 32bit binaries built by mingw in XP.

I realize that our hand is somewhat forced with compilers, but I think we should mitigate as much as possible with regards to arch and sdk.

I don't think this is the best place to discuss alternative toolchains on OSX.

Please just make the default release builds. Hardening enabled by default, I don't even care if there is a disable hardening but since there has to be detection to make sure the options don't break the build it's probably easy to keep a disable flag.

A make debug should build a binary the same as release but with -O0 (and -g cranked up) by default. If people want other optimization options in a debug build they can override the cflags. Sound good?

@gmaxwell: So your suggestion is:

• Debug is "-O0 -g2 -ggdb"
• Release is "-O2 -g0 + hardening"
• Default is release Or do I miss something?

I think I'd prefer -O1 for debug builds, and having hardening enabled there too, as it doesn't interfere that much with debugging, but has a large performance impact (which matters in particular when running under valgrind).

NACK from me on -O1 debug builds. Just today I ran into odd issues with lldb I suspect are caused by -O1 debug builds... (might not be, but "principle of least surprise" would be no optimizations at all for debug builds).

Many programs have a "release with debug symbols" mode for building, maybe that's a useful compromise for the people that want to debug/valgrind a hardened and optimized build.

A lot of work has gone into making "-O2 -g" a sane and workable default, in the upstream compiler and packaging world. Obviously it is not perfect for debugging, as the compiler may transform a statement you wish to debug, but literal man-years have gone into making it as usable as possible. Fedora builds debug info for all packages, and uses special tools to gather the debug info into a separately-installable, optional "foo-debuginfo" package.

Further, I really see no point in separate "debug build" and "release build" concepts. We should always build with a default, -O2 -g + hardening. If the default is not suitable -- I often disable -O for serious debugging too -- then we can add "--disable-opt" or "--enable-debug" if the developer really cannot be bothered to override on the configure command line.

IMO overridding CXXFLAGS on configure is easy enough. I also find -O2 makes debugging impractical, but not a huge deal if it can be overridden.

@sipa I have never seen hardening interfere with debugging, so I think it should always be on. (though perhaps we turn off the couple things that cause warnings at O0 just to remove the noise.

Jeff is right that a lot of work has gone into making debugging work at O2 -g. make debug is for when that fails.

I too would prefer O1 for make debug, but since I consider make debug for the case where release breaks debugging so O0 is safer and it can be overridden easily.

Yes, release should be default.

Does anyone here know how the split debugging stuff works? I'd like it if we could ship production binaries that have the debugging removed (I think it makes them a zillion mb smaller for us) but still have the symbols around to correlate when we get reports.

Some numbers, on x86-64/Linux:

CXXFLAGS="-O2 -Wall -g" ./configure --without-qt --with-incompatible-bdb

jgarzik@hum:~/repo/bitcoin/src$ ls -l bitcoind
-rwxrwxr-x 1 jgarzik jgarzik 63680406 Sep 13 12:52 bitcoind
jgarzik@hum:~/repo/bitcoin/src$ strip bitcoind
jgarzik@hum:~/repo/bitcoin/src$ ls -l bitcoind
-rwxrwxr-x 1 jgarzik jgarzik 3777696 Sep 13 12:54 bitcoind

CXXFLAGS="-O2" ./configure --without-qt --with-incompatible-bdb --disable-debug

jgarzik@hum:~/repo/bitcoin/src$ ls -l bitcoind
-rwxrwxr-x 1 jgarzik jgarzik 5227662 Sep 13 12:59 bitcoind
jgarzik@hum:~/repo/bitcoin/src$ strip bitcoind
jgarzik@hum:~/repo/bitcoin/src$ ls -l bitcoind
-rwxrwxr-x 1 jgarzik jgarzik 3777696 Sep 13 13:00 bitcoind

@gmaxwell Starter links, https://fedoraproject.org/wiki/Packaging:Debuginfo?rd=Packaging/Debuginfo https://fedorahosted.org/elfutils/wiki/DebugInfo

Though I think that the size of our executables is a low priority [unless it's a fun project you just want to obsess over ;p] Shipping debug info in production executables should be just fine. The compiler puts the debug info in separate sections, so it should not pollute instruction/L1/L2 caches if unused.

FWIW, if we ship with debug symbols, I also have a backtrace.dll (mostly written be other people) that can be used with a simple LoadLibrary call and dumps a backtrace to stderr (which we could reopen to debug.log if desired) on crash. The only downside (although my users thing it's a good thing) is that Windows no longer displays a crash dialog (ie, the program "silently" exits).

Another note too, let's examine the problem a bit at a higher level: work with autotools not against it. Remember that many platforms will go ahead and override CFLAGS / CXXFLAGS with the platform standard during the Red Hat / SuSE / Debian package build process.

All this work trying to find the perfect build flags for various releases will be scrapped, ignored or worked around by many, regardless of what you pick. Use the autotools default.

1. Scrap the concept of "debug version" or "release version" entirely.
2. Stop messing around with non-standard "--enable-debug" flags. The standard is to provide anything not already provided by default via environment variables passed at configure time: CFLAGS, CXXFLAGS, LIBS, LDFLAGS, ...
3. Stop messing around with -O2 in configure.ac, and just go with the default.
4. Only modify flags when required, such as in the hardening case (an obvious exception to case 3).

@gmaxwell Split debug stuff is no problem, I can create a target for those once the dust settles here.

@jgarzik fwiw, "-g -O2" is the autoconf default. That's what most would expect to see as a result of a vanilla "./configure".

Could we please restore the default behavior of pre-autotools (-O2) then proceed with the bikeshedding? I don't care how -O2 is restored, just do it and decide how to do do it "proper" later?

ACK - this or just reverting to autotools' default. We can bikeshed later.

Upstream now does -O2, superceded, closing.