Skip to content
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

random: fix crash on some 64bit platforms #10614

Merged
merged 1 commit into from Jun 17, 2017

Conversation

Projects
None yet
4 participants
@theuni
Copy link
Member

theuni commented Jun 16, 2017

Fixes #10611. Credit @sipa.

rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash in particular, it was holding a stack canary which was not properly restored after the cpuid.

Split out the x86+PIC case so that x86_64 doesn't have to worry about it.

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Jun 16, 2017

utACK b9b87481fd69e5caa15f580a19a5e0a58ad5f6a6

src/random.cpp Outdated
@@ -73,9 +73,15 @@ static constexpr uint32_t CPUID_F1_ECX_RDRAND = 0x40000000;
static void RDRandInit()
{
//! When calling cpuid function #1, ecx register will have this set if RDRAND is available.

This comment has been minimized.

@sipa

sipa Jun 16, 2017

Member

While you're at it, can you move this comment down to after the #endif? It's intended to clarify the CPUID_F1_ECX_RDRAND constant.

This comment has been minimized.

@theuni

theuni Jun 16, 2017

Author Member

Done

random: fix crash on some 64bit platforms
rbx needs to be stashed in a 64bit register on 64bit platforms. With this crash
in particular, it was holding a stack canary which was not properly restored
after the cpuid.

Split out the x86+PIC case so that x86_64 doesn't have to worry about it.

@theuni theuni force-pushed the theuni:fix-osx-crash branch to 9af207c Jun 16, 2017

@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented Jun 16, 2017

tested ACK 9af207c (OSX 10.12 only) full static build via gitian: https://bitcoin.jonasschnelli.ch/build/181
Confirmed that it fixes #10611

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Jun 16, 2017

utACK 9af207c

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Jun 16, 2017

Fixes #10611. Credit @sipa.

You can also add a blame for introducing the bug in the first place.

@gmaxwell
Copy link
Member

gmaxwell left a comment

utACK

@sipa sipa merged commit 9af207c into bitcoin:master Jun 17, 2017

1 check passed

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

sipa added a commit that referenced this pull request Jun 17, 2017

Merge #10614: random: fix crash on some 64bit platforms
9af207c random: fix crash on some 64bit platforms (Cory Fields)

Tree-SHA512: c9516b69bec224c7e650dfc7c50f04cdd93a1006d515699bc64a311a03662d4ad33a834861194a1649ed212b37fb50aadfb004954cdf8b9bc1dc82f3ea962897
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.