build: Enable C++11 in build, require C++11 compiler #7165

laanwj · 2015-12-03T14:17:43Z

Implements #6211.

Just meant for testing for now. Probably doesn't pass Travis.

I'm running into an issues with boost 1.55 locally: https://svn.boost.org/trac/boost/ticket/10038

/usr/include/boost/filesystem/operations.hpp:381: undefined reference to `boost::filesystem::detail::copy_file(boost::filesystem::path const&, boost::filesystem::path const&, boost::filesystem::copy_option, boost::system::error_code*)'

TODO:

Travis fails on "This is not a C++11 compiler"

Possibly TODO:

Make sure Boost/Qt/BDB++ is compiled with c++11 in depends?

jtimon · 2015-12-04T00:55:37Z

Concept requete-ACK, of course, I don't think anybody will dare to nack this (besides tested nits [and making travis happy]).

dcousens · 2015-12-04T01:17:43Z

concept ACK

droark · 2015-12-14T23:37:14Z

Concept ACK.

luke-jr · 2015-12-15T01:31:51Z

Concept ACK, but only when it's determined to be safe (especially against possibly invisible consensus failures) and can be supported on all stable distros with standard OS libraries...

dcousens · 2015-12-15T02:05:04Z

on all stable distros

For reference, that list of distros:

Those ticked have native support of GCC ^5.1.y in the latest stable release (AFAIK). Please let me know if any corrections need to be made.

This doens't necessarily act as a block, assuming #6211 (comment)

droark · 2015-12-15T02:14:14Z

@dcousens - Don't forget Windows (well, MinGW)! I guess this one's a little tricky since the Windows build is cross-compiled. I think Ubuntu 14.04 will be fine, although 16.04 (LTS) will be necessary if GCC 5 features are required, I suppose, due to Gitian requirements.

dcousens · 2015-12-15T02:16:06Z

Maybe throw the switch once 16.04 (LTS) comes out and somebody can get Gitian to play nice with it?

That sounds OK to me, it seems like a realistic timeline too.

droark · 2015-12-15T02:19:27Z

That sounds OK to me, it seems like a realistic timeline too.

@dcousens - Sorry for editing what you replied to. :) But yeah, maybe aim for throwing the switch around the time 16.04 comes out if, for whatever reasons, it's decided that GCC 5 support is required/highly desirable. 15.10 has it but I think Gitian officially supports only LTS releases.

luke-jr · 2015-12-15T04:03:40Z

~~Fedora can't be fully supported due to the ECDSA mess they've made. Disagree with it not being a blocker.~~ Is GCC 5 now known to solve all the ABI issues, or at least the ones that could potentially affect Core?

Edit: Forgot we switched to libsecp256k1, nevermind re Fedora.

laanwj · 2015-12-17T14:23:11Z

Ok, getting kind of tired of this, but postponing this another major release, I'm not going to push for this again.

jgarzik · 2015-12-17T14:24:43Z

@luke-jr No reason why Fedora cannot be supported. A fedora programmer added the following commit to picocoin, illustrating a Fedora-compatible approach: jgarzik/picocoin#44

sipa · 2015-12-17T15:50:55Z

@jgarzik That seems unrelated to C++11?

nathan-at-least · 2015-12-17T16:16:29Z

I'm working on a fork off of Bitcoin core to implement zerocash. We require C++11 due to dependencies (eg libsnark).

It's not much effort to make C++11 compile, link, run, and pass make check. Hidden changes to consensus are another matter which we don't have a solid grasp on.

We've been working on a fork off of both v0.10.0 and v0.11.2 with g++ -std=c++11. This has required a few modifications to the upstream bitcoin code, different in each of these branches. Here are some notes (though I may be forgetting details).

First, some strong caveats:

Our only testing of consensus changes has been make check, so the following notes are only a starting point for C++11 support in bitcoin core. (Edit: -because we haven't done rigorous consensus regression testing. Any advice here would be greatly appreciated.)
On top of this, on our branch atop v0.10.0, we disabled many tests that had hard-coded test vectors (such as serialized structures from production main-net) rather than correctly fixing the tests.
On v0.11.2 we have not disabled any upstream tests, and have only added C++11 mode and linked in our extra dependencies. (The v0.10.0 is a messy proof of concept. Now we're starting fresh with more rigour.)
We only test/develop on debian or arch linux with non-"default' packages installed. Furthermore we only build with the depends system, something close to make -C ./depends NO_QT=1 && ./autogen.sh && ./configure --prefix=./depends/… --with-gui=no CXXFLAGS='-O0 -g' && make V=1
No one on our team has a strong C++ background. :-<

So, I scanned through our deltas from upstream to hunt down changes I believe are necessitated by C++11 support:

We've found that upgrading to boost 1.57.0 atop v0.10.0, or to either boost 1.57.0 or 1.59.0 atop bitcoin v0.11.2 resolves the linker error in @laanwj 's original comment.
- The patches in ./depends/patches/boost do not apply to boost 1.57.0 (and we haven't tried them 1.59.0). I examined the 1.57.0 code to see if I could update the patches, but couldn't find relevant target code after about an hour of effort, so I gave up and deleted these patches! (I asked in #bitcoin-dev about this and at least one person told me that's probably ok because upstream probably addressed the same patched issue.)
Boost's list_of(x)(y)(z) syntax didn't work (for boost 1.57.0 atop bitcoin v0.10.0), so we switched to C++11 initializer syntax { x, y, z }.
In bitcoin v0.11.2 there is one unit test that relies on exceptions propagating out of a dtor: src/test/reverselock_tests.cpp L52-L58 at 7e278929. In C++11 this causes a process to abort unless you explicitly add an attribute to the dtor as in ~reverse_lock() noexcept(false) {…}.

I hope this helps. If I had time I'd work on this PR directly, but I can't promise anything.

jgarzik · 2015-12-17T16:18:01Z

@sipa It is directly related to luke's comment.

sipa · 2015-12-17T16:21:21Z

@jgarzik Oops, I missed that. @luke-jr That's irrelevant. We don't use OpenSSL's EC code anywhere anymore. I am really in favor of C++11 and don't understand the fuss at this point. There are potential problems when mixing C++ code compiled against different versions of the std library, but as far as I can tell, those are things that result in link-time errors? I would like to see this reopened.

jgarzik · 2015-12-17T19:11:23Z

Agree - urge re-opening this - this is not a big risk.

droark · 2015-12-17T21:25:24Z

Thanks for reopening, @laanwj.

One thing I would like to say is that some IBLT simulation code (non-Core) was written in C++11. I believe the plan is to try to recycle some of it for the eventual Core commit. It would be nice to know if C++11 will be acceptable so that everybody working on the Core patch(es) can use or modify code as necessary.

Also, I agree with @sipa. AFAIK, any library issues would come up as linker errors. I'm not aware offhand of any massive gotchas that would come with a switch to GCC 5 and a bit of time for distros to recompile everything. (AFAIK, Ubuntu did most of that before releasing 15.10, although it sounds like more work is needed for 16.04.) If anybody's aware of any specific issues, I'm all ears.

sipa · 2015-12-17T21:50:58Z

This was discussed in the weekly IRC meeting. It seems we'll aim for C++11 in 0.13, by first adding build configuration and testing for C++11 on some targets (but not all), and if no problems appear, switch everything to C++11 and start using C++11 features.

theuni · 2015-12-17T21:51:23Z

Looks like the only remaining issue in that list should be the throwing dtor.

nathan-at-least · 2015-12-18T00:10:04Z

The patch for the dtor "fix" is:

diff --git a/src/reverselock.h b/src/reverselock.h
index 567636e..db5c626 100644
--- a/src/reverselock.h
+++ b/src/reverselock.h
@@ -17,7 +17,7 @@ public:
         lock.unlock();
     }

-    ~reverse_lock() {
+    ~reverse_lock() noexcept(false) {
         lock.lock();
     }

This allows reverselock_error test to pass (link in my prior comment). However, it makes me uncomfortable because I don't understand if both enabling c++11 and adding this attribute will lead to identical behavior as before.

This patch is enabling a mechanism used for test verification, but we may not want that behavior in the application code. The only use I see is in src/scheduler.cpp L73 at cd3f12c6 (latest on branch master).

jtimon · 2015-12-18T05:21:37Z

c++11 for 0.13? I should miss more meetings so things go more my way...

jtimon · 2016-03-16T18:47:27Z

what's the latest status on this?

laanwj · 2016-03-16T19:20:12Z

what's the latest status on this?

@theuni has been chasing Travis about dependency caching support on the VMs that support c++11 (Ubuntu 14.04+)
We could in principle do without it, disabling qt builds - or using the distro's native qt development headers, but if possible it'd be preferable to keep using the current system.
That's what holding it up.

maflcko · 2016-04-17T12:20:26Z

Wouldn't you need to bump travis to trusty as part of this pull?

laanwj · 2016-04-18T10:43:32Z

@theuni is working on that

paveljanik · 2016-04-27T17:45:05Z

ACK a398549

configure output diff on OS X:

+checking whether g++ supports C++11 features by default... no
+checking whether g++ supports C++11 features with -std=c++11... yes

and the build log contains millions of added -std=c++11 as expected.

TheBlueMatt · 2016-04-27T19:58:01Z

No longer the latest version of the C++11 macro (see http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=history;f=m4/ax_cxx_compile_stdcxx.m4). Do we want to update?

droark · 2016-04-27T21:09:50Z

Tested ACK with the latest C++11 macro, as pointed out by Matt. Compiles fine on the latest OS X (10.11.4) and Xcode (7.3), make check passes, and I get the same output as Janik.

randy-waterhouse · 2016-04-28T00:21:40Z

ACK. builds, tests, runs.

Throws a new build warning
"httpserver.cpp:255:36: warning: ‘auto_ptr’ is deprecated ..."

paveljanik · 2016-04-28T05:07:28Z

@randy-waterhouse What compiler do you use please?

Edit: Travis has the warning here: https://travis-ci.org/bitcoin/bitcoin/jobs/126071750#L3267

theuni · 2016-04-28T05:20:51Z

@paveljanik That's a legitimate warning. We'll need to auto_ptr -> unique_ptr. But that's a hard fork :p

fanquake · 2016-04-28T07:12:49Z

Tested on OSX 10.11.4 & Xcode 7.3. compiles fine, make check passes. Same configure diff as above.

laanwj · 2016-04-28T08:18:49Z

removed WIP: tag, added commit that updates ax_cxx_compile_stdcxx (thanks @TheBlueMatt for noticing)

@randy-waterhouse Indeed, the auto_ptr should be replaced, let's do that in a later pull.

paveljanik · 2016-04-28T08:39:12Z

reACK 2aacc72

[skip ci]

7df9224 doc: Add note about new build/test requirements to release notes (Wladimir J. van der Laan) 2aacc72 build: update ax_cxx_compile_stdcxx to serial 4 (Wladimir J. van der Laan) a398549 depends: use c++11 (Cory Fields) 67969af build: Enable C++11 build, require C++11 compiler (Wladimir J. van der Laan)

Darwin build fixes Includes fixes cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7136 - Only the third commit (to avoid a merge conflict) - bitcoin/bitcoin#7302 - Excluding the first commit, which is unnecessary (we use Boost 1.62) - bitcoin/bitcoin#7487 - bitcoin/bitcoin#7606 - bitcoin/bitcoin#7711 - bitcoin/bitcoin#7165 - bitcoin/bitcoin#8002 - bitcoin/bitcoin#8210 - Only the second commit - bitcoin/bitcoin#9114

0ae8f18 build: add -Wgnu to compile flags (fanquake) 3a0fd77 Remove use of non-standard zero variadic macros (Ben Woosley) 49f6178 Drop unused LOG_TIME_MICROS helper (Ben Woosley) 5d49999 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes) Pull request description: Since we [started using](#7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile". However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former. #### anonymous structs ```bash ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct] struct { ``` This is fixed in b849212. #### variadic macros ```bash ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments] ::Unserialize(s, VARINT(nVersionDummy)); ``` This is taken care of in #18087. The `LOG_TIME_*` macros introduced in #16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html). ```bash In file included from validation.cpp:22: ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ 6 warnings generated. ``` This is fixed in 081a0ab and 612e8e1. #### prevention To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions. This would close #14130. Also related to #17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute. ACKs for top commit: practicalswift: ACK 0ae8f18 -- diff looks correct MarcoFalke: ACK 0ae8f18 vasild: utACK 0ae8f18 dongcarl: ACK 0ae8f18 Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f

0ae8f18 build: add -Wgnu to compile flags (fanquake) 3a0fd77 Remove use of non-standard zero variadic macros (Ben Woosley) 49f6178 Drop unused LOG_TIME_MICROS helper (Ben Woosley) 5d49999 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes) Pull request description: Since we [started using](bitcoin#7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile". However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former. #### anonymous structs ```bash ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct] struct { ``` This is fixed in bitcoin@b849212. #### variadic macros ```bash ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments] ::Unserialize(s, VARINT(nVersionDummy)); ``` This is taken care of in bitcoin#18087. The `LOG_TIME_*` macros introduced in bitcoin#16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html). ```bash In file included from validation.cpp:22: ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ 6 warnings generated. ``` This is fixed in 081a0ab and 612e8e1. #### prevention To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions. This would close bitcoin#14130. Also related to bitcoin#17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute. ACKs for top commit: practicalswift: ACK 0ae8f18 -- diff looks correct MarcoFalke: ACK 0ae8f18 vasild: utACK 0ae8f18 dongcarl: ACK 0ae8f18 Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f

76f031b build: fix unoptimized libraries in depends (fanquake) Pull request description: We need to append-to rather than set CXXFLAGS, otherwise we loose `-O2` & `-pipe` from our defaults. Currently this results in zeromq being built without optimizations at all (or whatever the compiler would default too, essentially always `-O0`). Bdb is the same, for the CXX portion of its code. C code has been built with `-O2`. Boost has actually been unaffected because it receives `-O3` from it's own build flags. Noticed while reworking #22380. For bdb & zeromq, I assume (haven't checked) this has been the case since #7165. You can inspect the effcts in bitcoind comparing a function from a unoptimised library, i.e libzmq. Build bitcoind with zeromq from depends (7be143a): ``` gmake -C depends NO_QT=1 NO_UPNP=1 NO_NATPMP=1 NO_WALLET=1 -j9 ./autogen.sh CONFIG_SITE=/path/to/share/config.site ./configure gmake -C src bitcoind -j9 ``` Find a zeromq function: ```bash nm -C src/bitcoind | rg zmq ... 000000010053a7e0 T _zmq_ctx_new ``` Disassemble it: ```bash lldb src/bitcoind disassemble -a 000000010053a7e0 ... bitcoind`zmq_ctx_new: bitcoind[0x10053a7e0] <+0>: pushq %rbp bitcoind[0x10053a7e1] <+1>: movq %rsp, %rbp bitcoind[0x10053a7e4] <+4>: subq $0x50, %rsp bitcoind[0x10053a7e8] <+8>: callq 0x1004b2ee0 ; zmq::initialize_network() bitcoind[0x10053a7ed] <+13>: testb $0x1, %al bitcoind[0x10053a7ef] <+15>: jne 0x10053a802 ; <+34> bitcoind[0x10053a7f5] <+21>: movq $0x0, -0x8(%rbp) bitcoind[0x10053a7fd] <+29>: jmp 0x10053a8e8 ; <+264> bitcoind[0x10053a802] <+34>: movq 0xadab7(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x10053a809] <+41>: movl $0x2b8, %edi ; imm = 0x2B8 bitcoind[0x10053a80e] <+46>: callq 0x100556c58 ; symbol stub for: operator new(unsigned long, std::nothrow_t const&) bitcoind[0x10053a813] <+51>: xorl %ecx, %ecx bitcoind[0x10053a815] <+53>: movl %ecx, %edx bitcoind[0x10053a817] <+55>: movb $0x0, -0x19(%rbp) bitcoind[0x10053a81b] <+59>: cmpq $0x0, %rax bitcoind[0x10053a81f] <+63>: movq %rax, -0x38(%rbp) bitcoind[0x10053a823] <+67>: movq %rdx, -0x40(%rbp) bitcoind[0x10053a827] <+71>: je 0x10053a858 ; <+120> bitcoind[0x10053a82d] <+77>: movq -0x38(%rbp), %rax bitcoind[0x10053a831] <+81>: movq %rax, -0x18(%rbp) bitcoind[0x10053a835] <+85>: movb $0x1, -0x19(%rbp) bitcoind[0x10053a839] <+89>: movq -0x38(%rbp), %rdi bitcoind[0x10053a83d] <+93>: movq %rax, -0x48(%rbp) bitcoind[0x10053a841] <+97>: callq 0x100497aa0 ; zmq::ctx_t::ctx_t() bitcoind[0x10053a846] <+102>: jmp 0x10053a84b ; <+107> bitcoind[0x10053a84b] <+107>: movq -0x48(%rbp), %rax bitcoind[0x10053a84f] <+111>: movq %rax, -0x40(%rbp) bitcoind[0x10053a853] <+115>: jmp 0x10053a858 ; <+120> bitcoind[0x10053a858] <+120>: movq -0x40(%rbp), %rax bitcoind[0x10053a85c] <+124>: movq %rax, -0x10(%rbp) bitcoind[0x10053a860] <+128>: cmpq $0x0, -0x10(%rbp) bitcoind[0x10053a865] <+133>: je 0x10053a8e0 ; <+256> bitcoind[0x10053a86b] <+139>: movq -0x10(%rbp), %rdi bitcoind[0x10053a86f] <+143>: callq 0x100497ee0 ; zmq::ctx_t::valid() const bitcoind[0x10053a874] <+148>: testb $0x1, %al bitcoind[0x10053a876] <+150>: jne 0x10053a8db ; <+251> bitcoind[0x10053a87c] <+156>: movq -0x10(%rbp), %rax bitcoind[0x10053a880] <+160>: cmpq $0x0, %rax bitcoind[0x10053a884] <+164>: movq %rax, -0x50(%rbp) bitcoind[0x10053a888] <+168>: je 0x10053a8a3 ; <+195> bitcoind[0x10053a88e] <+174>: movq -0x50(%rbp), %rdi bitcoind[0x10053a892] <+178>: callq 0x100497ec0 ; zmq::ctx_t::~ctx_t() bitcoind[0x10053a897] <+183>: movq -0x50(%rbp), %rax bitcoind[0x10053a89b] <+187>: movq %rax, %rdi bitcoind[0x10053a89e] <+190>: callq 0x100556c3a ; symbol stub for: operator delete(void*) bitcoind[0x10053a8a3] <+195>: movq $0x0, -0x8(%rbp) bitcoind[0x10053a8ab] <+203>: jmp 0x10053a8e8 ; <+264> bitcoind[0x10053a8b0] <+208>: movq %rax, -0x28(%rbp) bitcoind[0x10053a8b4] <+212>: movl %edx, -0x2c(%rbp) bitcoind[0x10053a8b7] <+215>: testb $0x1, -0x19(%rbp) bitcoind[0x10053a8bb] <+219>: jne 0x10053a8c6 ; <+230> bitcoind[0x10053a8c1] <+225>: jmp 0x10053a8d6 ; <+246> bitcoind[0x10053a8c6] <+230>: movq 0xad9f3(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x10053a8cd] <+237>: movq -0x18(%rbp), %rdi bitcoind[0x10053a8d1] <+241>: callq 0x100556c40 ; symbol stub for: operator delete(void*, std::nothrow_t const&) bitcoind[0x10053a8d6] <+246>: jmp 0x10053a8f2 ; <+274> bitcoind[0x10053a8db] <+251>: jmp 0x10053a8e0 ; <+256> bitcoind[0x10053a8e0] <+256>: movq -0x10(%rbp), %rax bitcoind[0x10053a8e4] <+260>: movq %rax, -0x8(%rbp) bitcoind[0x10053a8e8] <+264>: movq -0x8(%rbp), %rax bitcoind[0x10053a8ec] <+268>: addq $0x50, %rsp bitcoind[0x10053a8f0] <+272>: popq %rbp bitcoind[0x10053a8f1] <+273>: retq bitcoind[0x10053a8f2] <+274>: movq -0x28(%rbp), %rdi bitcoind[0x10053a8f6] <+278>: callq 0x100556856 ; symbol stub for: _Unwind_Resume bitcoind[0x10053a8fb] <+283>: ud2 bitcoind[0x10053a8fd] <+285>: nopl (%rax) ``` Cleanup and repeat after changing the zeromq cxxflags to be `$(package)_cxxflags+=-std=c++17`: ```bash gmake clean gmake -C depends NO_QT=1 NO_UPNP=1 NO_NATPMP=1 NO_WALLET=1 -j9 gmake -C src bitcoind -j9 nm -C src/bitcoind | rg zmq ... 00000001004d5170 T _zmq_ctx_new ``` Disassemble the same function which has now been built with `-O2`: ```bash lldb src/bitcoind disassemble -a 00000001004d5170 ... bitcoind`zmq_ctx_new: bitcoind[0x1004d5170] <+0>: pushq %rbp bitcoind[0x1004d5171] <+1>: movq %rsp, %rbp bitcoind[0x1004d5174] <+4>: pushq %r14 bitcoind[0x1004d5176] <+6>: pushq %rbx bitcoind[0x1004d5177] <+7>: callq 0x10049cbc0 ; zmq::initialize_network() bitcoind[0x1004d517c] <+12>: testb %al, %al bitcoind[0x1004d517e] <+14>: je 0x1004d51bd ; <+77> bitcoind[0x1004d5180] <+16>: movq 0xab139(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x1004d5187] <+23>: movl $0x2b8, %edi ; imm = 0x2B8 bitcoind[0x1004d518c] <+28>: callq 0x1004f0e5a ; symbol stub for: operator new(unsigned long, std::nothrow_t const&) bitcoind[0x1004d5191] <+33>: testq %rax, %rax bitcoind[0x1004d5194] <+36>: je 0x1004d51bd ; <+77> bitcoind[0x1004d5196] <+38>: movq %rax, %rbx bitcoind[0x1004d5199] <+41>: movq %rax, %rdi bitcoind[0x1004d519c] <+44>: callq 0x100493400 ; zmq::ctx_t::ctx_t() bitcoind[0x1004d51a1] <+49>: movq %rbx, %rdi bitcoind[0x1004d51a4] <+52>: callq 0x1004936e0 ; zmq::ctx_t::valid() const bitcoind[0x1004d51a9] <+57>: testb %al, %al bitcoind[0x1004d51ab] <+59>: jne 0x1004d51bf ; <+79> bitcoind[0x1004d51ad] <+61>: movq %rbx, %rdi bitcoind[0x1004d51b0] <+64>: callq 0x1004936d0 ; zmq::ctx_t::~ctx_t() bitcoind[0x1004d51b5] <+69>: movq %rbx, %rdi bitcoind[0x1004d51b8] <+72>: callq 0x1004f0e42 ; symbol stub for: operator delete(void*) bitcoind[0x1004d51bd] <+77>: xorl %ebx, %ebx bitcoind[0x1004d51bf] <+79>: movq %rbx, %rax bitcoind[0x1004d51c2] <+82>: popq %rbx bitcoind[0x1004d51c3] <+83>: popq %r14 bitcoind[0x1004d51c5] <+85>: popq %rbp bitcoind[0x1004d51c6] <+86>: retq bitcoind[0x1004d51c7] <+87>: movq %rax, %r14 bitcoind[0x1004d51ca] <+90>: movq 0xab0ef(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x1004d51d1] <+97>: movq %rbx, %rdi bitcoind[0x1004d51d4] <+100>: callq 0x1004f0e48 ; symbol stub for: operator delete(void*, std::nothrow_t const&) bitcoind[0x1004d51d9] <+105>: movq %r14, %rdi bitcoind[0x1004d51dc] <+108>: callq 0x1004f0a6a ; symbol stub for: _Unwind_Resume bitcoind[0x1004d51e1] <+113>: ud2 bitcoind[0x1004d51e3] <+115>: nopw %cs:(%rax,%rax) bitcoind[0x1004d51ed] <+125>: nopl (%rax) ``` ACKs for top commit: theuni: ACK 76f031b. Tree-SHA512: 0f71d98387d88f36bd22fd4204f8116efc6d540b45a722281483f1f19f36a26daa197458006af6a35d80a52dd8f13c714c4c816ad6c279d6e52872c948fab987

76f031b build: fix unoptimized libraries in depends (fanquake) Pull request description: We need to append-to rather than set CXXFLAGS, otherwise we loose `-O2` & `-pipe` from our defaults. Currently this results in zeromq being built without optimizations at all (or whatever the compiler would default too, essentially always `-O0`). Bdb is the same, for the CXX portion of its code. C code has been built with `-O2`. Boost has actually been unaffected because it receives `-O3` from it's own build flags. Noticed while reworking bitcoin#22380. For bdb & zeromq, I assume (haven't checked) this has been the case since bitcoin#7165. You can inspect the effcts in bitcoind comparing a function from a unoptimised library, i.e libzmq. Build bitcoind with zeromq from depends (7be143a): ``` gmake -C depends NO_QT=1 NO_UPNP=1 NO_NATPMP=1 NO_WALLET=1 -j9 ./autogen.sh CONFIG_SITE=/path/to/share/config.site ./configure gmake -C src bitcoind -j9 ``` Find a zeromq function: ```bash nm -C src/bitcoind | rg zmq ... 000000010053a7e0 T _zmq_ctx_new ``` Disassemble it: ```bash lldb src/bitcoind disassemble -a 000000010053a7e0 ... bitcoind`zmq_ctx_new: bitcoind[0x10053a7e0] <+0>: pushq %rbp bitcoind[0x10053a7e1] <+1>: movq %rsp, %rbp bitcoind[0x10053a7e4] <+4>: subq $0x50, %rsp bitcoind[0x10053a7e8] <+8>: callq 0x1004b2ee0 ; zmq::initialize_network() bitcoind[0x10053a7ed] <+13>: testb $0x1, %al bitcoind[0x10053a7ef] <+15>: jne 0x10053a802 ; <+34> bitcoind[0x10053a7f5] <+21>: movq $0x0, -0x8(%rbp) bitcoind[0x10053a7fd] <+29>: jmp 0x10053a8e8 ; <+264> bitcoind[0x10053a802] <+34>: movq 0xadab7(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x10053a809] <+41>: movl $0x2b8, %edi ; imm = 0x2B8 bitcoind[0x10053a80e] <+46>: callq 0x100556c58 ; symbol stub for: operator new(unsigned long, std::nothrow_t const&) bitcoind[0x10053a813] <+51>: xorl %ecx, %ecx bitcoind[0x10053a815] <+53>: movl %ecx, %edx bitcoind[0x10053a817] <+55>: movb $0x0, -0x19(%rbp) bitcoind[0x10053a81b] <+59>: cmpq $0x0, %rax bitcoind[0x10053a81f] <+63>: movq %rax, -0x38(%rbp) bitcoind[0x10053a823] <+67>: movq %rdx, -0x40(%rbp) bitcoind[0x10053a827] <+71>: je 0x10053a858 ; <+120> bitcoind[0x10053a82d] <+77>: movq -0x38(%rbp), %rax bitcoind[0x10053a831] <+81>: movq %rax, -0x18(%rbp) bitcoind[0x10053a835] <+85>: movb $0x1, -0x19(%rbp) bitcoind[0x10053a839] <+89>: movq -0x38(%rbp), %rdi bitcoind[0x10053a83d] <+93>: movq %rax, -0x48(%rbp) bitcoind[0x10053a841] <+97>: callq 0x100497aa0 ; zmq::ctx_t::ctx_t() bitcoind[0x10053a846] <+102>: jmp 0x10053a84b ; <+107> bitcoind[0x10053a84b] <+107>: movq -0x48(%rbp), %rax bitcoind[0x10053a84f] <+111>: movq %rax, -0x40(%rbp) bitcoind[0x10053a853] <+115>: jmp 0x10053a858 ; <+120> bitcoind[0x10053a858] <+120>: movq -0x40(%rbp), %rax bitcoind[0x10053a85c] <+124>: movq %rax, -0x10(%rbp) bitcoind[0x10053a860] <+128>: cmpq $0x0, -0x10(%rbp) bitcoind[0x10053a865] <+133>: je 0x10053a8e0 ; <+256> bitcoind[0x10053a86b] <+139>: movq -0x10(%rbp), %rdi bitcoind[0x10053a86f] <+143>: callq 0x100497ee0 ; zmq::ctx_t::valid() const bitcoind[0x10053a874] <+148>: testb $0x1, %al bitcoind[0x10053a876] <+150>: jne 0x10053a8db ; <+251> bitcoind[0x10053a87c] <+156>: movq -0x10(%rbp), %rax bitcoind[0x10053a880] <+160>: cmpq $0x0, %rax bitcoind[0x10053a884] <+164>: movq %rax, -0x50(%rbp) bitcoind[0x10053a888] <+168>: je 0x10053a8a3 ; <+195> bitcoind[0x10053a88e] <+174>: movq -0x50(%rbp), %rdi bitcoind[0x10053a892] <+178>: callq 0x100497ec0 ; zmq::ctx_t::~ctx_t() bitcoind[0x10053a897] <+183>: movq -0x50(%rbp), %rax bitcoind[0x10053a89b] <+187>: movq %rax, %rdi bitcoind[0x10053a89e] <+190>: callq 0x100556c3a ; symbol stub for: operator delete(void*) bitcoind[0x10053a8a3] <+195>: movq $0x0, -0x8(%rbp) bitcoind[0x10053a8ab] <+203>: jmp 0x10053a8e8 ; <+264> bitcoind[0x10053a8b0] <+208>: movq %rax, -0x28(%rbp) bitcoind[0x10053a8b4] <+212>: movl %edx, -0x2c(%rbp) bitcoind[0x10053a8b7] <+215>: testb $0x1, -0x19(%rbp) bitcoind[0x10053a8bb] <+219>: jne 0x10053a8c6 ; <+230> bitcoind[0x10053a8c1] <+225>: jmp 0x10053a8d6 ; <+246> bitcoind[0x10053a8c6] <+230>: movq 0xad9f3(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x10053a8cd] <+237>: movq -0x18(%rbp), %rdi bitcoind[0x10053a8d1] <+241>: callq 0x100556c40 ; symbol stub for: operator delete(void*, std::nothrow_t const&) bitcoind[0x10053a8d6] <+246>: jmp 0x10053a8f2 ; <+274> bitcoind[0x10053a8db] <+251>: jmp 0x10053a8e0 ; <+256> bitcoind[0x10053a8e0] <+256>: movq -0x10(%rbp), %rax bitcoind[0x10053a8e4] <+260>: movq %rax, -0x8(%rbp) bitcoind[0x10053a8e8] <+264>: movq -0x8(%rbp), %rax bitcoind[0x10053a8ec] <+268>: addq $0x50, %rsp bitcoind[0x10053a8f0] <+272>: popq %rbp bitcoind[0x10053a8f1] <+273>: retq bitcoind[0x10053a8f2] <+274>: movq -0x28(%rbp), %rdi bitcoind[0x10053a8f6] <+278>: callq 0x100556856 ; symbol stub for: _Unwind_Resume bitcoind[0x10053a8fb] <+283>: ud2 bitcoind[0x10053a8fd] <+285>: nopl (%rax) ``` Cleanup and repeat after changing the zeromq cxxflags to be `$(package)_cxxflags+=-std=c++17`: ```bash gmake clean gmake -C depends NO_QT=1 NO_UPNP=1 NO_NATPMP=1 NO_WALLET=1 -j9 gmake -C src bitcoind -j9 nm -C src/bitcoind | rg zmq ... 00000001004d5170 T _zmq_ctx_new ``` Disassemble the same function which has now been built with `-O2`: ```bash lldb src/bitcoind disassemble -a 00000001004d5170 ... bitcoind`zmq_ctx_new: bitcoind[0x1004d5170] <+0>: pushq %rbp bitcoind[0x1004d5171] <+1>: movq %rsp, %rbp bitcoind[0x1004d5174] <+4>: pushq %r14 bitcoind[0x1004d5176] <+6>: pushq %rbx bitcoind[0x1004d5177] <+7>: callq 0x10049cbc0 ; zmq::initialize_network() bitcoind[0x1004d517c] <+12>: testb %al, %al bitcoind[0x1004d517e] <+14>: je 0x1004d51bd ; <+77> bitcoind[0x1004d5180] <+16>: movq 0xab139(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x1004d5187] <+23>: movl $0x2b8, %edi ; imm = 0x2B8 bitcoind[0x1004d518c] <+28>: callq 0x1004f0e5a ; symbol stub for: operator new(unsigned long, std::nothrow_t const&) bitcoind[0x1004d5191] <+33>: testq %rax, %rax bitcoind[0x1004d5194] <+36>: je 0x1004d51bd ; <+77> bitcoind[0x1004d5196] <+38>: movq %rax, %rbx bitcoind[0x1004d5199] <+41>: movq %rax, %rdi bitcoind[0x1004d519c] <+44>: callq 0x100493400 ; zmq::ctx_t::ctx_t() bitcoind[0x1004d51a1] <+49>: movq %rbx, %rdi bitcoind[0x1004d51a4] <+52>: callq 0x1004936e0 ; zmq::ctx_t::valid() const bitcoind[0x1004d51a9] <+57>: testb %al, %al bitcoind[0x1004d51ab] <+59>: jne 0x1004d51bf ; <+79> bitcoind[0x1004d51ad] <+61>: movq %rbx, %rdi bitcoind[0x1004d51b0] <+64>: callq 0x1004936d0 ; zmq::ctx_t::~ctx_t() bitcoind[0x1004d51b5] <+69>: movq %rbx, %rdi bitcoind[0x1004d51b8] <+72>: callq 0x1004f0e42 ; symbol stub for: operator delete(void*) bitcoind[0x1004d51bd] <+77>: xorl %ebx, %ebx bitcoind[0x1004d51bf] <+79>: movq %rbx, %rax bitcoind[0x1004d51c2] <+82>: popq %rbx bitcoind[0x1004d51c3] <+83>: popq %r14 bitcoind[0x1004d51c5] <+85>: popq %rbp bitcoind[0x1004d51c6] <+86>: retq bitcoind[0x1004d51c7] <+87>: movq %rax, %r14 bitcoind[0x1004d51ca] <+90>: movq 0xab0ef(%rip), %rsi ; (void *)0x0000000000000000 bitcoind[0x1004d51d1] <+97>: movq %rbx, %rdi bitcoind[0x1004d51d4] <+100>: callq 0x1004f0e48 ; symbol stub for: operator delete(void*, std::nothrow_t const&) bitcoind[0x1004d51d9] <+105>: movq %r14, %rdi bitcoind[0x1004d51dc] <+108>: callq 0x1004f0a6a ; symbol stub for: _Unwind_Resume bitcoind[0x1004d51e1] <+113>: ud2 bitcoind[0x1004d51e3] <+115>: nopw %cs:(%rax,%rax) bitcoind[0x1004d51ed] <+125>: nopl (%rax) ``` ACKs for top commit: theuni: ACK 76f031b. Tree-SHA512: 0f71d98387d88f36bd22fd4204f8116efc6d540b45a722281483f1f19f36a26daa197458006af6a35d80a52dd8f13c714c4c816ad6c279d6e52872c948fab987

laanwj added the Build system label Dec 3, 2015

laanwj closed this Dec 17, 2015

laanwj reopened this Dec 17, 2015

laanwj added the Refactoring label Feb 16, 2016

maflcko mentioned this pull request Apr 16, 2016

[qa] Switch to py3 #7814

Merged

1 task

build: update ax_cxx_compile_stdcxx to serial 4

2aacc72

laanwj changed the title ~~WIP: build: Enable C++11 in build, require C++11 compiler~~ build: Enable C++11 in build, require C++11 compiler Apr 28, 2016

doc: Add note about new build/test requirements to release notes

7df9224

[skip ci]

laanwj force-pushed the 2015_12_c++11 branch from 45ea7d8 to 7df9224 Compare April 28, 2016 08:47

laanwj merged commit 7df9224 into bitcoin:master Apr 28, 2016

daira mentioned this pull request May 9, 2016

Enable compiler warnings as errors. zcash/zcash#457

Closed

str4d mentioned this pull request Oct 29, 2017

Darwin build fixes zcash/zcash#2697

Merged

maflcko mentioned this pull request Jan 25, 2019

RFC: C++14 Requirement #13356

Closed

hebasto mentioned this pull request Nov 30, 2019

qt: Fix comparison function signature #17634

Merged

fanquake mentioned this pull request Feb 7, 2020

build: ensure we aren't using GNU extensions #18088

Merged

fanquake mentioned this pull request Aug 31, 2021

build: fix unoptimized libraries in depends #22840

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

build: Enable C++11 in build, require C++11 compiler #7165

build: Enable C++11 in build, require C++11 compiler #7165

Uh oh!

Conversation

laanwj commented Dec 3, 2015

Uh oh!

jtimon commented Dec 4, 2015

Uh oh!

dcousens commented Dec 4, 2015

Uh oh!

droark commented Dec 14, 2015

Uh oh!

luke-jr commented Dec 15, 2015

Uh oh!

dcousens commented Dec 15, 2015

Uh oh!

droark commented Dec 15, 2015

Uh oh!

dcousens commented Dec 15, 2015

Uh oh!

droark commented Dec 15, 2015

Uh oh!

luke-jr commented Dec 15, 2015

Uh oh!

laanwj commented Dec 17, 2015

Uh oh!

jgarzik commented Dec 17, 2015

Uh oh!

sipa commented Dec 17, 2015 via email

Uh oh!

nathan-at-least commented Dec 17, 2015

Uh oh!

jgarzik commented Dec 17, 2015

Uh oh!

sipa commented Dec 17, 2015 via email

Uh oh!

jgarzik commented Dec 17, 2015

Uh oh!

droark commented Dec 17, 2015

Uh oh!

sipa commented Dec 17, 2015

Uh oh!

theuni commented Dec 17, 2015

Uh oh!

nathan-at-least commented Dec 18, 2015

Uh oh!

jtimon commented Dec 18, 2015

Uh oh!

jtimon commented Mar 16, 2016

Uh oh!

laanwj commented Mar 16, 2016

Uh oh!

maflcko commented Apr 17, 2016

Uh oh!

laanwj commented Apr 18, 2016

Uh oh!

paveljanik commented Apr 27, 2016

Uh oh!

TheBlueMatt commented Apr 27, 2016

Uh oh!

droark commented Apr 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

randy-waterhouse commented Apr 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paveljanik commented Apr 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theuni commented Apr 28, 2016

Uh oh!

fanquake commented Apr 28, 2016

Uh oh!

laanwj commented Apr 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paveljanik commented Apr 28, 2016

Uh oh!

Uh oh!

droark commented Apr 27, 2016 •

edited

Loading

randy-waterhouse commented Apr 28, 2016 •

edited

Loading

paveljanik commented Apr 28, 2016 •

edited

Loading

laanwj commented Apr 28, 2016 •

edited

Loading