A set of both big and little endian binaries, should be compatible with PowerPC 970 (Apple G5) and newer.
Also splits libpng out of Qt (since Qt's bundled copy is broken on POWER) and disables JPEG (since we don't use it).
Tested only the little endian variant, on Gentoo.
(Based on #14065)
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Concept ACK
Not convinced, though, that we need a set of both big and little-endian binaries. What is the default on this platform? If you can only test little-endian, I think it's better to leave it at that. (looks like both Debian and Fedora support LE, or are moving to it)
Also (not for this PR): what would be really interesting is running the gitian build (for all platforms) on the secure workstation. This would remove the single point of failure of dependency Intel/AMD platforms for the build.
There is no default. Some older systems can only boot to big endian. Newer systems come in either BE and LE variant. Unfortunately, one cannot run even static BE binaries on LE or vice-versa (on Linux, anyway).
At least RHEL, SuSE, and Adélie ship big-endian images.
Big endian is simple to run in KVM paravirt on the little-endian systems with a performance hit of less than 1%. This should be very easy to test, especially with Gentoo, since Gentoo can make a cross-root somewhere and then you can just point KVM at that disk.
Any POWER7 or older system, including most workstations before the Talos (including IBM's post-RS and Apple's 970), are exclusively big endian.
Any system (of any generation) running openSuSE Tumbleweed or Adélie is exclusively big endian.
Any POWER7 or older system, including most workstations before the Talos (including IBM's post-RS and Apple's 970), are exclusively big endian.
Those are outside the scope of this PR, though, OP mentions this is POWER8+.
Rebased
118 | - (sym, _, version) = line[7].partition('@') 119 | - is_import = line[6] == 'UND' 120 | + words = line.split() 121 | + if 'Machine:' in words: 122 | + arch = words[-1] 123 | + m = re.match(r'^\s*\d+:\s*[\da-f]+\s+\d+\s(?:(?:\S+\s+){3})(?:\[.*\]\s+)?(\S+)\s+(\S+).*$', line)
I have no clue what this is suppose to do.
Perhaps a different tool like objdump or nm has output that's more easily parsed?
ahhh even parsing the ELF directly would be easier here than this
This is a fairly straightforward regex... Do we really need to ban regexs? :/
Looking at alternatives:
I don't think it's that complex now that you've explained what it does on IRC, and why the old parsing doesn't work for POWER (due to [<localentry>: 8])—please add a comment in that regard (and thanks for investigating that readelf is apparently still the right tool for the job)
673 | @@ -674,6 +674,11 @@ if test x$use_glibc_compat != xno; then 674 | AC_DEFINE_UNQUOTED(FDELT_TYPE, $fdelt_type,[parameter and return value type for __fdelt_chk]) 675 | AX_CHECK_LINK_FLAG([[-Wl,--wrap=__divmoddi4]], [COMPAT_LDFLAGS="$COMPAT_LDFLAGS -Wl,--wrap=__divmoddi4"]) 676 | AX_CHECK_LINK_FLAG([[-Wl,--wrap=log2f]], [COMPAT_LDFLAGS="$COMPAT_LDFLAGS -Wl,--wrap=log2f"]) 677 | + case $host in 678 | + powerpc64* | ppc64*) 679 | + AX_CHECK_LINK_FLAG([[-Wl,--no-tls-get-addr-optimize]], [COMPAT_LDFLAGS="$COMPAT_LDFLAGS -Wl,--no-tls-get-addr-optimize"])
If this were a gcc switch, we would test against --tls-get-addr-optimize, then add --no-tls-get-addr-optimize to COMPAT_LDFLAGS if it succeeded. That is because gcc issues warnings for --no-foo if unsupported, but errors for --foo.
I'm thinking we should do the same here. I don't know how most linkers handle this, but I can't imagine that causing any problems. That would also mean that we could safely run this check for all platforms without constraining it to specific ppc's.
It should work without the no- prefix, but upon further research, I'm not sure if this is the correct solution at all.
According to the documentation, systems without the glibc-side part can still explicitly enable the option and produce binaries that work (with a performance hit) on older systems. I'm not sure how this is technically implemented.
Managed to get an old glibc running and confirmed --no-tls-get-addr-optimize is in fact needed. No clue why the docs imply otherwise. :/
As for the original comment:
12 | +define $(package)_config_cmds 13 | + $($(package)_autoconf) 14 | +endef 15 | + 16 | +define $(package)_build_cmds 17 | + $(MAKE) $($(package)_build_opts) PNG_COPTS='-fPIC'
Why this instead of doing it the usual way via configure?
No idea why, but it doesn't seem to work the usual way via configure.
/usr/lib/gcc-cross/riscv64-linux-gnu/8/../../../../riscv64-linux-gnu/bin/ld: /home/ubuntu/build/bitcoin/depends/riscv64-linux-gnu/lib/libpng16.a(png.o): relocation R_RISCV_HI20 against `a local symbol' can not be used when making a shared object; recompile with -fPIC
/usr/lib/gcc-cross/riscv64-linux-gnu/8/../../../../riscv64-linux-gnu/bin/ld: /home/ubuntu/build/bitcoin/depends/riscv64-linux-gnu/lib/libpng16.a(pngerror.o): relocation R_RISCV_HI20 against `a local symbol' can not be used when making a shared object; recompile with -fPIC
/usr/lib/gcc-cross/riscv64-linux-gnu/8/../../../../riscv64-linux-gnu/bin/ld: /home/ubuntu/build/bitcoin/depends/riscv64-linux-gnu/lib/libpng16.a(pngget.o): relocation R_RISCV_HI20 against `__stack_chk_guard@@GLIBC_2.27' can not be used when making a shared object; recompile with -fPIC
...
84 | @@ -79,7 +85,11 @@ script: | 85 | echo "REAL=\`which -a ${i}-${prog}-8 | grep -v ${WRAP_DIR}/${i}-${prog} | head -1\`" >> ${WRAP_DIR}/${i}-${prog} 86 | echo 'export LD_PRELOAD=/usr/lib/x86_64-linux-gnu/faketime/libfaketime.so.1' >> ${WRAP_DIR}/${i}-${prog} 87 | echo "export FAKETIME=\"$1\"" >> ${WRAP_DIR}/${i}-${prog} 88 | - echo "exec \"\$REAL\" \"\$@\"" >> $WRAP_DIR/${i}-${prog} 89 | + if [ "${i:0:9}" = "powerpc64" ]; then 90 | + echo "exec \"\$REAL\" -mcpu=power8 -mtune=power9 \"\$@\"" >> $WRAP_DIR/${i}-${prog}
This looks like something that should be handled in depends/hosts/linux.mk
What if someone else wants to build for POWER7 (outside of gitian)?
Is there a reason for specifying -mcpu=power8 here at all? VSX goes back to P7 and VMX goes back to P4…
vec_vsx_ld is POWER7; but the SHA sigma instructions are exclusive to POWER8 and newer. However, it looks like the merged code does check the hardware cap before using it, so it depends on compiler support, not build hardware support. (That is, if my understanding is correct, one could build all the way down to -mcpu=generic and you'd still get speed on P8+.)
572 | @@ -573,7 +573,7 @@ clean-local: 573 | check-symbols: $(bin_PROGRAMS) 574 | if GLIBC_BACK_COMPAT 575 | @echo "Checking glibc back compat..." 576 | - $(AM_V_at) READELF=$(READELF) CPPFILT=$(CPPFILT) $(top_srcdir)/contrib/devtools/symbol-check.py < $(bin_PROGRAMS) 577 | + $(AM_V_at) READELF=$(READELF) CPPFILT=$(CPPFILT) $(top_srcdir)/contrib/devtools/symbol-check.py $(BITCOIN_SYMBOL_CHECK_OPTIONS) < $(bin_PROGRAMS)
What is BITCOIN_SYMBOL_CHECK_OPTIONS ? I don't see any references to it, is this just intended to be an optional env var for local testing?
Should have been dropped in rebase; will remove next revision.
Concept ACK, a few questions though.
Concept ACK
@luke-jr What setup would you suggest for reviewers who want to test those binaries?
You could spin up an IntegriCloud VPS, but I don't know of any big endian distros that use glibc. All of them that I am aware of (Adelie, Void, etc) went to musl since it is easier to support BE on musl.
So you'd need an older Debian or Fedora image to test the BE binary.
Rebased, and extended support back to the PowerPC 970 (Apple G5) since it didn't seem to have a notable performance hit on POWER9 (it actually performed better!).
@luke-jr What setup would you suggest for reviewers who want to test those binaries?
@awilfox answered that above. If you prefer to buy real hardware, I'm using https://www.raptorcs.com/content/base/products.html
Fails with:
> make[1]: Leaving directory '/home/ubuntu/build/bitcoin/depends/work/build/powerpc64-linux-gnu/qt/5.9.7-72625d07ee6/qtbase/config.tests/xcb_xlib'
=> source accepted.
test config.gui.libraries.xcb_xlib succeeded
Checking for XKB config root...
test config.gui.tests.xkbconfigroot gave result /usr/share/X11/xkb
Checking for xkbcommon...
Trying source 0 (type pkgConfig) of library xkbcommon ...
+ PKG_CONFIG_SYSROOT_DIR=/ PKG_CONFIG_LIBDIR=/home/ubuntu/build/bitcoin/depends/powerpc64-linux-gnu/lib/pkgconfig /usr/bin/pkg-config --exists --silence-errors xkbcommon
pkg-config did not find package.
=> source produced no result.
test config.gui.libraries.xkbcommon FAILED
Done running configuration tests.
Configure summary:
Building on: linux-g++ (x86_64, CPU features: mmx sse sse2)
Building for: bitcoin-linux-g++ (power64, CPU features: altivec)
Configuration: cross_compile enable_new_dtags largefile precompile_header silent release c++11 dbus no-qml-debug reduce_exports release_tools static
Build options:
Mode ................................... release; optimized tools
Optimize release build for size ........ no
Building shared libraries .............. no
Using C++ standard ..................... C++11
Using ccache ........................... no
Using gold linker ...................... no
Using new DTAGS ........................ yes
Using precompiled headers .............. yes
Using LTCG ............................. no
Target compiler supports:
Build parts ............................ libs
Qt modules and options:
Qt Concurrent .......................... no
Qt D-Bus ............................... yes
Qt D-Bus directly linked to libdbus .... no
Qt Gui ................................. yes
Qt Network ............................. yes
Qt Sql ................................. no
Qt Testlib ............................. yes
Qt Widgets ............................. yes
Qt Xml ................................. no
Support enabled for:
Using pkg-config ....................... yes
QML debugging .......................... no
udev ................................... no
Using system zlib ...................... yes
Qt Core:
DoubleConversion ....................... yes
Using system DoubleConversion ........ no
GLib ................................... no
iconv .................................. no
ICU .................................... no
Logging backends:
journald ............................. no
syslog ............................... no
slog2 ................................ no
Using system PCRE2 ..................... no
Qt Network:
getaddrinfo() .......................... no
getifaddrs() ........................... yes
IPv6 ifname ............................ yes
libproxy ............................... no
OpenSSL ................................ no
Qt directly linked to OpenSSL ........ no
SCTP ................................... no
Use system proxies ..................... yes
Qt Gui:
Accessibility .......................... yes
FreeType ............................... yes
Using system FreeType ................ no
HarfBuzz ............................... yes
Using system HarfBuzz ................ no
Fontconfig ............................. no
Image formats:
GIF .................................. no
ICO .................................. yes
JPEG ................................. no
Using system libjpeg ............... no
PNG .................................. yes
Using system libpng ................ yes
EGL .................................... no
OpenVG ................................. no
OpenGL:
Desktop OpenGL ....................... no
OpenGL ES 2.0 ........................ no
OpenGL ES 3.0 ........................ no
OpenGL ES 3.1 ........................ no
Session Management ..................... no
Features used by QPA backends:
evdev .................................. yes
libinput ............................... no
INTEGRITY HID .......................... no
mtdev .................................. no
tslib .................................. no
xkbcommon-evdev ........................ no
QPA backends:
DirectFB ............................... no
EGLFS .................................. no
LinuxFB ................................ no
VNC .................................... yes
Mir client ............................. no
X11:
Using system-provided XCB libraries .. no
EGL on X11 ........................... no
Xinput2 .............................. no
XCB XKB .............................. yes
XLib ................................. yes
XCB render ........................... yes
XCB GLX .............................. yes
XCB Xlib ............................. yes
Using system-provided xkbcommon ...... no
Qt Widgets:
GTK+ ................................... no
Styles ................................. Fusion Windows
Qt PrintSupport:
CUPS ................................... no
Qt Sql:
DB2 (IBM) .............................. no
InterBase .............................. no
MySql .................................. no
OCI (Oracle) ........................... no
ODBC ................................... no
PostgreSQL ............................. no
SQLite2 ................................ no
SQLite ................................. no
Using system provided SQLite ......... no
TDS (Sybase) ........................... no
Note: Using static linking will disable the use of dynamically
loaded plugins. Make sure to import all needed static plugins,
or compile needed modules into the library.
Note: -optimized-tools is not useful in -release mode.
Note: Disabling X11 Accessibility Bridge: D-Bus or AT-SPI is missing.
ERROR: Qt requires a compliant STL library.
ERROR: detected a std::atomic implementation that fails for function pointers.
Please apply the patch corresponding to your Standard Library vendor, found in
qtbase/config.tests/atomicfptr
ERROR: Feature 'openssl-linked' was enabled, but the pre-condition '!features.securetransport && libs.openssl' failed.
ERROR: Feature 'openssl' was enabled, but the pre-condition '!features.securetransport && (features.openssl-linked || libs.openssl_headers)' failed.
ERROR: Feature 'system-freetype' was enabled, but the pre-condition 'features.freetype && libs.freetype' failed.
ERROR: Feature 'fontconfig' was enabled, but the pre-condition '!config.win32 && !config.darwin && features.system-freetype && libs.fontconfig' failed.
funcs.mk:243: recipe for target '/home/ubuntu/build/bitcoin/depends/work/build/powerpc64-linux-gnu/qt/5.9.7-72625d07ee6/qtbase/.stamp_configured' failed
make: *** [/home/ubuntu/build/bitcoin/depends/work/build/powerpc64-linux-gnu/qt/5.9.7-72625d07ee6/qtbase/.stamp_configured] Error 3
make: Leaving directory '/home/ubuntu/build/bitcoin/depends'
Worked for me and the bot? :/
55 | @@ -54,6 +56,10 @@ For linux AARCH64 cross compilation: 56 | 57 | sudo apt-get install g++-aarch64-linux-gnu binutils-aarch64-linux-gnu 58 | 59 | +For linux POWER 64-bit cross compilation (there are no packages for 32-bit): 60 | + 61 | + sudo apt-get install curl g++-7-powerpc64-linux-gnu binutils-powerpc64-linux-gnu g++-7-powerpc64le-linux-gnu binutils-powerpc64le-linux-gnu
sudo apt-get install g++-powerpc64-linux-gnu binutils-powerpc64-linux-gnu g++-powerpc64le-linux-gnu binutils-powerpc64le-linux-gnu
(Same as the other instructions)
Worked for me and the bot? :/
Oh, the failure is from the bot. But it passes for me locally on a fresh bionic vm:
apt update && apt install -y git vim htop make automake cmake curl g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 && apt install -y g++-powerpc64-linux-gnu binutils-powerpc64-linux-gnu g++-powerpc64le-linux-gnu binutils-powerpc64le-linux-gnu
git clone https://github.com/luke-jr/bitcoin --depth=1 --branch gitian_power64 && cd bitcoin/depends && make -j 9 HOST=powerpc64-linux-gnu qt
Tested that Gitian building works on my own machine.
Concept ACK
Diversity in testing is good. Thanks for working on this!
Would be nice to have this for 0.20.
Also splits libpng out of Qt (since Qt's bundled copy is broken on POWER) and disables JPEG (since we don't use it).
Could this be solved by changing to a new version of Qt? If so, I'd prefer that.
One of the commits has been cherry-picked and merged as 989fd539d5bf590c5f6070ee2a4a9e2d3018df2c
84 | @@ -79,7 +85,11 @@ script: | 85 | echo "REAL=\`which -a ${i}-${prog}-8 | grep -v ${WRAP_DIR}/${i}-${prog} | head -1\`" >> ${WRAP_DIR}/${i}-${prog} 86 | echo 'export LD_PRELOAD=/usr/lib/x86_64-linux-gnu/faketime/libfaketime.so.1' >> ${WRAP_DIR}/${i}-${prog} 87 | echo "export FAKETIME=\"$1\"" >> ${WRAP_DIR}/${i}-${prog} 88 | - echo "exec \"\$REAL\" \"\$@\"" >> $WRAP_DIR/${i}-${prog} 89 | + if [ "${i:0:9}" = "powerpc64" ]; then 90 | + echo "exec \"\$REAL\" -mcpu=970 -mtune=power9 \"\$@\"" >> $WRAP_DIR/${i}-${prog}
Could we set these as CFLAGS/CPPFLAGS in depends instead of in the wrapper?
I don't know how reliably those get passed everywhere. Also, you mean CXXFLAGS...
:sweat_smile: yeah I meant CXXFLAGS...
with depends you can just modify hosts/linux.mk... I'm also not sure about tuning for a specific cpu here (perhaps I'm understanding incorrectly), especially because this will be activated for both big and small endian HOSTs? Do you have sources for why these flags are needed?
We don't want to force these CFLAGS on everyone... just use them for gitian builds so they perform well.
Sure, then we can just supply powerpc64_linux_CFLAGS='-mcpu=970 -mtune=power9' powerpc64le_linux_CFLAGS='-mcpu=970 -mtune=power9', but let's do that in a followup :-)
Wondering about the choice of -mcpu here... Why PowerPC 970 instead of a more generic powerpc64 or powerpc64le? My reference: https://gcc.gnu.org/onlinedocs/gcc/RS_002f6000-and-PowerPC-Options.html#RS_002f6000-and-PowerPC-Options
Wondering about the choice of -mcpu here... Why PowerPC 970 instead of a more generic powerpc64 or powerpc64le?
This still atleast needs answering.
I don't know. GCC doesn't let me compare -mcpu values AFAICT. Presumably generic options wouldn't be optimal for modern CPUs? (even 970 is pretty ancient)
I'm told -mcpu=970 is just powerpc64 + altivec.
This has needed rebase for a while (if we still want this for 0.20).
Also splits libpng out of Qt (since Qt's bundled copy is broken on POWER)
Just for reference:
Could this be solved by changing to a new version of Qt? If so, I'd prefer that.
Fixed since Qt 5.12.0 (according to the bugtracker).
Would be nice to have this for 0.20.
Rebased and fixed a bug (GCC 8 miscompiles for LE targets with -mcpu=970)
Considering the fragility of using the wrong -mcpu, I kept it in the wrapper...
Excerpt:
+ make -j1 -C src check-security
make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-powerpc64-linux-gnu/src'
Checking binary security...
bitcoind: failed NX
bitcoin-cli: failed NX
bitcoin-tx: failed NX
bitcoin-wallet: failed NX
test/test_bitcoin: failed NX
bench/bench_bitcoin: failed NX
qt/bitcoin-qt: failed NX
Makefile:18486: recipe for target 'check-security' failed
make: *** [check-security] Error 1
make: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-powerpc64-linux-gnu/src'
Fixed by making noexecstack explicit on all platforms (it doesn't make sense to rely on defaults when we require it...)
Rebased and re-tested.
<!--9cd9c72976c961c55c7acef8f6ba82cd-->
<!--a722867cd34abeea1fadc8d60700f111-->
| File | commit d67883d01e507dd22d1281f4a4860e79d6a46a47<br>(master) | commit f8cb73189115a63d45256b9340816bfce96393f5<br>(master and this pull) |
|---|---|---|
| bitcoin-core-linux-0.21-res.yml | fb2670f2f8ab0d0a... |
57d59a44bffa740a... |
| bitcoin-core-osx-0.21-res.yml | d24523299fb03e93... |
15dbc11fa63ccda4... |
| bitcoin-core-win-0.21-res.yml | 56c065d51ffa9fb6... |
250e9b116781e91c... |
| *-aarch64-linux-gnu-debug.tar.gz | b4d05463fda0adac... |
05b2ee6295f2e033... |
| *-aarch64-linux-gnu.tar.gz | 3a43937219f4fbef... |
c00e5233a0c93442... |
| *-arm-linux-gnueabihf-debug.tar.gz | 54e6c6f85955de26... |
e24bf2d02a8198de... |
| *-arm-linux-gnueabihf.tar.gz | 76dee1ea73dd31b7... |
feb4e7976ee0ee1d... |
| *-osx-unsigned.dmg | 6d0b6c8274d37dbb... |
1c58228bce6b6118... |
| *-osx64.tar.gz | 902a69555d82322d... |
b13444feb8a9c24a... |
| *-riscv64-linux-gnu-debug.tar.gz | e87c71666376fe9a... |
d1bf41dc3cabb2b9... |
| *-riscv64-linux-gnu.tar.gz | 38c0ca912040a8b4... |
0e4832f29b4f2219... |
| *-win64-debug.zip | f815299d5d2100f2... |
2e88232c4e5ea247... |
| *-win64-setup-unsigned.exe | 854105d5cef6ab9c... |
cb5d93c039b3c21e... |
| *-win64.zip | e01a2d84b4fb5653... |
78516aed31be5d0a... |
| *-x86_64-linux-gnu-debug.tar.gz | f75d8220b59ed46e... |
52936ad4725853d2... |
| *-x86_64-linux-gnu.tar.gz | a6d8292cf44c959e... |
8bdba6613953e49f... |
| *.tar.gz | 70f224763adead92... |
b0942820765c35a9... |
| linux-build.log | ceaaa58ad271c807... |
c067489d8a64916f... |
| osx-build.log | e48082cd327972e1... |
21a106676aff913c... |
| win-build.log | 4c0c68075af70f26... |
98e8b6f1fac30696... |
| bitcoin-core-linux-0.21-res.yml.diff | 52bd17632c91381e... |
|
| bitcoin-core-osx-0.21-res.yml.diff | 10814c03d2f02a18... |
|
| bitcoin-core-win-0.21-res.yml.diff | 05c123318fddccb1... |
|
| *-powerpc64-linux-gnu-debug.tar.gz | 49050c85b7776ebc... |
|
| *-powerpc64-linux-gnu.tar.gz | 76908c390ae80531... |
|
| *-powerpc64le-linux-gnu-debug.tar.gz | c84351a042c4aa31... |
|
| *-powerpc64le-linux-gnu.tar.gz | 766246b8a94227f4... |
|
| linux-build.log.diff | 6b78b10de4a501ce... |
|
| osx-build.log.diff | 89499c0a4d99986e... |
|
| win-build.log.diff | e68c80d64b8709db... |
144 | @@ -145,7 +145,6 @@ def check_ELF_separate_code(executable): 145 | EXPECTED_FLAGS = { 146 | # Read + execute 147 | '.init': 'R E', 148 | - '.plt': 'R E',
Why remove this? Is
We should make this specific on architecture. If POWER9 is an exception here that needs the jump table to be writable (which would be kind of insecure in itself, so this is an important security check), let's make a specific exception for that..plt writable?
Edit: writability of the .plt section is not determined by separate-code. It's part of "early binding". E.g. -z,relro or -z,now.
Fixed
50 | FAKETIME_HOST_PROGS="gcc g++" 51 | FAKETIME_PROGS="date ar ranlib nm" 52 | HOST_CFLAGS="-O2 -g" 53 | HOST_CXXFLAGS="-O2 -g" 54 | - HOST_LDFLAGS_BASE="-static-libstdc++ -Wl,-O2" 55 | + HOST_LDFLAGS="-static-libstdc++ -Wl,-O2 -Wl,-z,noexecstack"
If you're going to add -z,noexecstack in the gitian descriptor, it should have a comment explaining why it's here, rather than along with all of the other hardening options in configure, otherwise it's not going to be clear why this flag is special cased.
I don't know the answer to that myself. This PR is merely making explicit what was already enforced by our scripts for other platforms.
<!--cf906140f33d8803c4a75a2196329ecb-->
🐙 This pull request conflicts with the target branch and needs rebase.
<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>
Milestone
22.0