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

GCC 9.1 fixes #5614

Merged

Conversation

Projects
None yet
5 participants
@moneromooo-monero
Copy link
Contributor

commented Jun 8, 2019

No description provided.

@@ -90,6 +90,18 @@ namespace hw {
AKout = keys.AKout;
}

ABPkeys &ABPkeys::operator=(const ABPkeys& keys) {

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jun 9, 2019

Contributor

Does everything get copied? Because this looks like a ABPkeys operator=(const ABPkeys&) = default; case (the copy constructor too).

If this style is preferred since the copy constructor is already explicit:

if (std::addressof(keys) != this)
{
   ...
}

will be useful in skipping work in the one edge case.

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jun 9, 2019

Author Contributor

I've not checked, it's a straight copy of what the ctor does. I'll add the address check.

@@ -134,10 +134,11 @@ namespace boost
a & port;
a & length;

if (length > net::tor_address::buffer_size())
const size_t buffer_size = net::tor_address::buffer_size();
if (length > buffer_size)

This comment has been minimized.

Copy link
@vtnerd

vtnerd Jun 9, 2019

Contributor

How does this fix a crash with GCC 9.1? The value returned by buffer_size() is a constant, so storing the return value on the stack locally should not have any change in behavior. I might need to install GCC 9 early on my Gentoo devbox ...

This comment has been minimized.

Copy link
@hyc

hyc Jun 9, 2019

Contributor

gcc 9.1 definitely crashes without this patch.

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Jun 9, 2019

Author Contributor

It is a GCC ICE, the code was correct AFAICT.

This comment has been minimized.

Copy link
@hyc

hyc Jun 11, 2019

Contributor

Btw, we reported the bug upstream https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90825

Fix GCC 9.1 build warnings
GCC wants operator= aand copy ctor to be both defined, or neither

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:gcc-9.1-0.14 branch from 59730a4 to f47488c Jun 9, 2019

cmake: do not use -mmitigate-rop on GCC >= 9.1
It was removed, but it still accepted by the compiler, which warns
for every file
@hyc

hyc approved these changes Jun 11, 2019

@luigi1111 luigi1111 merged commit ce13a98 into monero-project:release-v0.14 Jun 11, 2019

8 of 10 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
buildbot/monero-linux-armv8 Build done.
Details
buildbot/monero-linux-armv7 Build done.
Details
buildbot/monero-static-osx-10.11 Build done.
Details
buildbot/monero-static-osx-10.12 Build done.
Details
buildbot/monero-static-osx-10.13 Build done.
Details
buildbot/monero-static-ubuntu-amd64 Build done.
Details
buildbot/monero-static-ubuntu-i686 Build done.
Details
buildbot/monero-static-win32 Build done.
Details
buildbot/monero-static-win64 Build done.
Details

luigi1111 added a commit that referenced this pull request Jun 11, 2019

Merge pull request #5614
4cff925 p2p: fix GCC 9.1 crash (monermooo-monero)
f47488c Fix GCC 9.1 build warnings (moneromooo-monero)
ce13a98 cmake: do not use -mmitigate-rop on GCC >= 9.1 (moneromooo-monero)
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.