build: Allow export of environ symbols and work around rv64 toolchain issue #17569
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ever since statically linking Qt, we've been linking the C++ library statically too (-static-libstdc++). Take this into account in the symbol checker.
Debian 8 (Jessie) has: - g++ version 4.9.2 - libc version 2.19 Ubuntu 16.04.4 (Xenial) has: - g++ version 5.3.1 - libc version 2.23.0 CentOS 7 has: - g++ version 4.8.5 - libc version 2.17 Taking the minimum of these as our target. According to the GNU ABI document this corresponds to: - GCC 4.8.5: GCC_4.8.0 - (glibc) GLIBC_2_17 Co-Authored-By: fanquake <fanquake@gmail.com>
This export was introduced in bitcoin#17270 which added ``` //! Necessary on some platforms extern char** environ; ```
|
I also think it'd be OK to remove the exports check completely. It was introduced back in the day because of symbol collisions between dynamically-linked Qt and the built-in statically built OpenSSL which could have scary implications. This hasn't been the case for ages—ever since Qt5 and before, we've statically linked Qt—and we don't even use OpenSSL anymore. I don't think a binary that exports symbols can usually cause problems (exporting excessive numbers of symbols would be an indication that something is going wrong in the link, but besides that). The check is already being skipped for some architectures. |
|
It looks like there is a third problem that needs to be fixed before gitian passes again, the RISC-V binaries fail on the NX (non-executable stack) check. I'm really confused as to what could cause this. Will try to bisect. I suspect it's another unexpected by-effect of #17270, because it passes with 1baf7d1. Edit: no, it's not #17270, I don't have the issue with 0bb37e4 Edit: bisected it to b49b6b0 . Seems that linking to OpenSSL was the only thing that made it set noexecstack, on RISC-V. I'm not sure why yet, although we do explicitly pass |
7ebe0bd to
b1b4c3e
Compare
|
Added a workaround to force # Workaround for Ubuntu's g++ compiler for RV64, which
# doesn't add .note.GNU-stack sections automatically unlike other platforms,
# resulting in a NX failure of the binary in security-check.py
# TODO: remove this when no longer needed
if [ "${i}" = "riscv64-linux-gnu" ]; then
HOST_LDFLAGS="${HOST_LDFLAGS_BASE} -Wl,-z,noexecstack"
else
HOST_LDFLAGS="${HOST_LDFLAGS_BASE}"
fiThis is necessary now that the flag is no longer "inherited" through OpenSSL's build settings. I think this should be it. |
|
Issue (hopefully correctly) reported upstream: https://bugs.launchpad.net/ubuntu/+source/gcc-8-cross-ports/+bug/1853740 |
b1b4c3e to
eafd259
Compare
Gitian builds
|
maflcko
pushed a commit
that referenced
this pull request
Nov 25, 2019
…rv64 toolchain issue eafd259 build: Add NX workaround for RV64 (Wladimir J. van der Laan) f6e4225 build: Allow export of environ symbols (Wladimir J. van der Laan) Pull request description: This export was introduced in #17270 which added ``` //! Necessary on some platforms extern char** environ; ``` This should (finally) make the gitian build pass again (fix issue #17525.). Built on top of #17538 which should be merged first. Top commit has no ACKs. Tree-SHA512: 5c2054d52d0957aec3dc945b76d8e219187d22dc03889e7a88fb76049bf8e4a3e9f4da00dd1e9dd0351211f8e70d1a1b8ad7244f0348dab698e9d14b9d0c0bd4
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Nov 25, 2019
…around rv64 toolchain issue eafd259 build: Add NX workaround for RV64 (Wladimir J. van der Laan) f6e4225 build: Allow export of environ symbols (Wladimir J. van der Laan) Pull request description: This export was introduced in bitcoin#17270 which added ``` //! Necessary on some platforms extern char** environ; ``` This should (finally) make the gitian build pass again (fix issue bitcoin#17525.). Built on top of bitcoin#17538 which should be merged first. Top commit has no ACKs. Tree-SHA512: 5c2054d52d0957aec3dc945b76d8e219187d22dc03889e7a88fb76049bf8e4a3e9f4da00dd1e9dd0351211f8e70d1a1b8ad7244f0348dab698e9d14b9d0c0bd4
1 task
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
Feb 18, 2020
This export was introduced in bitcoin#17270 which added ``` //! Necessary on some platforms extern char** environ; ``` Github-Pull: bitcoin#17569 Rebased-From: f6e4225
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
Feb 18, 2020
Work around https://bugs.launchpad.net/ubuntu/+source/gcc-8-cross-ports/+bug/1853740. Github-Pull: bitcoin#17569 Rebased-From: eafd259
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
Feb 18, 2020
This export was introduced in bitcoin#17270 which added ``` //! Necessary on some platforms extern char** environ; ``` Github-Pull: bitcoin#17569 Rebased-From: f6e4225
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
Feb 18, 2020
Work around https://bugs.launchpad.net/ubuntu/+source/gcc-8-cross-ports/+bug/1853740. Github-Pull: bitcoin#17569 Rebased-From: eafd259
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
Feb 18, 2020
Github-Pull: bitcoin#17569 Rebased-From: f6e4225
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
Feb 18, 2020
Work around https://bugs.launchpad.net/ubuntu/+source/gcc-8-cross-ports/+bug/1853740. Github-Pull: bitcoin#17569 Rebased-From: eafd259
|
I'm going to untag this as needing backport to
|
sidhujag
pushed a commit
to syscoin-core/syscoin
that referenced
this pull request
Nov 10, 2020
…around rv64 toolchain issue eafd259 build: Add NX workaround for RV64 (Wladimir J. van der Laan) f6e4225 build: Allow export of environ symbols (Wladimir J. van der Laan) Pull request description: This export was introduced in bitcoin#17270 which added ``` //! Necessary on some platforms extern char** environ; ``` This should (finally) make the gitian build pass again (fix issue bitcoin#17525.). Built on top of bitcoin#17538 which should be merged first. Top commit has no ACKs. Tree-SHA512: 5c2054d52d0957aec3dc945b76d8e219187d22dc03889e7a88fb76049bf8e4a3e9f4da00dd1e9dd0351211f8e70d1a1b8ad7244f0348dab698e9d14b9d0c0bd4
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Sep 17, 2021
…around rv64 toolchain issue eafd259 build: Add NX workaround for RV64 (Wladimir J. van der Laan) f6e4225 build: Allow export of environ symbols (Wladimir J. van der Laan) Pull request description: This export was introduced in bitcoin#17270 which added ``` //! Necessary on some platforms extern char** environ; ``` This should (finally) make the gitian build pass again (fix issue bitcoin#17525.). Built on top of bitcoin#17538 which should be merged first. Top commit has no ACKs. Tree-SHA512: 5c2054d52d0957aec3dc945b76d8e219187d22dc03889e7a88fb76049bf8e4a3e9f4da00dd1e9dd0351211f8e70d1a1b8ad7244f0348dab698e9d14b9d0c0bd4
thelazier
pushed a commit
to thelazier/dash
that referenced
this pull request
Sep 25, 2021
…around rv64 toolchain issue eafd259 build: Add NX workaround for RV64 (Wladimir J. van der Laan) f6e4225 build: Allow export of environ symbols (Wladimir J. van der Laan) Pull request description: This export was introduced in bitcoin#17270 which added ``` //! Necessary on some platforms extern char** environ; ``` This should (finally) make the gitian build pass again (fix issue bitcoin#17525.). Built on top of bitcoin#17538 which should be merged first. Top commit has no ACKs. Tree-SHA512: 5c2054d52d0957aec3dc945b76d8e219187d22dc03889e7a88fb76049bf8e4a3e9f4da00dd1e9dd0351211f8e70d1a1b8ad7244f0348dab698e9d14b9d0c0bd4
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This export was introduced in #17270 which added
This should (finally) make the gitian build pass again (fix issue #17525.).
Built on top of #17538 which should be merged first.