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

C++11 build/runtime fixes #7302

Merged
merged 4 commits into from Jan 7, 2016

Conversation

Projects
None yet
9 participants
@theuni
Member

theuni commented Jan 5, 2016

In preparation for switching to the c++11 requirement, I've done depends builds on all supported OSs and fixed up any snags along the way.

Note that this does not enable c++11 anywhere, it simply enables c++11 builds to pass.

Once merged, we can begin moving forward with enabling.

theuni added some commits Jan 5, 2016

c++11: detect and correct for boost builds with an incompatible abi
This is ugly, but temporary. boost::filesystem will likely be dropped soon
after c++11 is enabled. Otherwise, we could simply roll our own copy_file. I've
fixed this at the buildsystem level for now in order to avoid mixing in
functional changes.

Explanation:
If boost (prior to 1.57) was built without c++11, it emulated scoped enums
using c++98 constructs. Unfortunately, this implementation detail leaked into
the abi. This was fixed in 1.57.

When building against that installed version using c++11, the headers pick up
on the native c++11 scoped enum support and enable it, however it will fail to
link. This can be worked around by disabling c++11 scoped enums if linking will
fail.

Add an autoconf test to determine incompatibility. At build-time, if native
enums are being used (a c++11 build), and force-disabling them causes a
successful link, we can be sure that there's an incompatibility and enable the
work-around.
c++11: don't throw from the reverselock destructor
noexcept is default for destructors as of c++11. By throwing in reverselock's
destructor if it's lock has been tampered with, the likely result is
std::terminate being called. Indeed that happened before this change.

Once reverselock has taken another lock (its ctor didn't throw), it makes no
sense to try to grab or lock the parent lock. That is be broken/undefined
behavior depending on the parent lock's implementation, but it shouldn't cause
the reverselock to fail to re-lock when destroyed.

To avoid those problems, simply swap the parent lock's contents with a dummy
for the duration of the lock. That will ensure that any undefined behavior is
caught at the call-site rather than the reverse lock's destruction.

Barring a failed mutex unlock which would be indicative of a larger problem,
the destructor should now never throw.
c++11: CAccountingEntry must be defined before use in a list
c++11ism. This fixes builds against libc++.
@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jan 5, 2016

Contributor

utACK 3968922

Contributor

dcousens commented Jan 5, 2016

utACK 3968922

@fanquake

This comment has been minimized.

Show comment
Hide comment
@fanquake

fanquake Jan 5, 2016

Member

conceptACK, will start testing shortly.

On Wednesday, 6 January 2016, Daniel Cousens notifications@github.com
wrote:

utACK


Reply to this email directly or view it on GitHub
#7302 (comment).

Member

fanquake commented Jan 5, 2016

conceptACK, will start testing shortly.

On Wednesday, 6 January 2016, Daniel Cousens notifications@github.com
wrote:

utACK


Reply to this email directly or view it on GitHub
#7302 (comment).

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jan 6, 2016

Member

code review ACK.

Member

jonasschnelli commented Jan 6, 2016

code review ACK.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke
Member

MarcoFalke commented Jan 6, 2016

utACK 3968922

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Jan 6, 2016

Contributor

utACK

Contributor

jgarzik commented Jan 6, 2016

utACK

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jan 6, 2016

Member

utACK

Member

sipa commented Jan 6, 2016

utACK

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Jan 6, 2016

Contributor

minor nits: new configure line output is:

checking for mismatched boost c++11 scoped enums... ok

Will we use C++11 like we use C++? Ie. capital C? Or is small c ok?

Can you please add a comment to configure.ac (e.g. reuse the commit message) above the new part?

Otherwise ACK
Thanks!

Contributor

paveljanik commented Jan 6, 2016

minor nits: new configure line output is:

checking for mismatched boost c++11 scoped enums... ok

Will we use C++11 like we use C++? Ie. capital C? Or is small c ok?

Can you please add a comment to configure.ac (e.g. reuse the commit message) above the new part?

Otherwise ACK
Thanks!

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jan 7, 2016

Member

utACK

Member

laanwj commented Jan 7, 2016

utACK

@laanwj laanwj merged commit 3968922 into bitcoin:master Jan 7, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jan 7, 2016

Merge pull request #7302
3968922 c++11: fix libbdb build against libc++ in c++11 mode (Cory Fields)
57d2f62 c++11: CAccountingEntry must be defined before use in a list (Cory Fields)
89f71c6 c++11: don't throw from the reverselock destructor (Cory Fields)
76ac35f c++11: detect and correct for boost builds with an incompatible abi (Cory Fields)

@str4d str4d referenced this pull request Oct 29, 2017

Merged

Darwin build fixes #2697

zkbot added a commit to zcash/zcash that referenced this pull request Nov 29, 2017

Auto merge of #2697 - str4d:darwin-build, r=<try>
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

zkbot added a commit to zcash/zcash that referenced this pull request Nov 30, 2017

Auto merge of #2697 - str4d:darwin-build, r=<try>
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

zkbot added a commit to zcash/zcash that referenced this pull request Nov 30, 2017

Auto merge of #2697 - str4d:darwin-build, r=str4d
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

zkbot added a commit to zcash/zcash that referenced this pull request Nov 30, 2017

Auto merge of #2697 - str4d:darwin-build, r=str4d
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment