Skip to content

Conversation

@dongcarl
Copy link
Contributor

@dongcarl dongcarl commented Oct 1, 2021

Fixes #23111

It would seem that I got the wrong default glibc-dynamic-linker path for PowerPC platforms. This means that for our currently released v22.0 binaries to be run on powerpc platforms, users would have to either:

  1. Move /lib64/ld64.so.? to /lib, or
  2. Invoke their linker-loader directly to start our binaries, e.g. /lib64/ld64.so.? bitcoind

This is my bad.

I've fixed the paths in this patchset, and also added a test to symbol-check.py so that this does not ever slip past our checks again.

@dongcarl dongcarl changed the title build: Fix guix linker-loader path and add symbol-checks build: Fix guix linker-loader path and add check_ELF_interpreter Oct 1, 2021
@dongcarl dongcarl force-pushed the 2021-09-fix-loader-quick branch from 9f35cd9 to e95b5a9 Compare October 1, 2021 01:05
@laanwj
Copy link
Member

laanwj commented Oct 1, 2021

Concept ACK, this is a good check to have.

@dongcarl dongcarl force-pushed the 2021-09-fix-loader-quick branch from e95b5a9 to 2261b18 Compare October 2, 2021 00:06
@dongcarl
Copy link
Contributor Author

dongcarl commented Oct 2, 2021

Pushed e95b5a9ca9040cab826f041ab7cf348696a15ba3 -> 2261b186ebae38362eaceeed5ac1991b578375fa

  • Cherry-picked 2261b186ebae38362eaceeed5ac1991b578375fa from scripts: use LIEF for ELF security & symbol checks #22392 as e759330d003099a8acacb1e2b7da00e1f3f1d1ea
    • Modifications:
      • Use lief.ELF.ARCH(243) instead of 'RISCV' special value
      • Update assertEqual expected string in test-symbol-check
  • Stop using pixie for the new ELF interpreter check

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 2, 2021

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23212 (lint: enable mypy import checking by fanquake)
  • #22392 (scripts: use LIEF for ELF security & symbol checks by fanquake)

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.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 3, 2021

Guix builds

File commit 35a31d5
(master)
commit a9f8ab3120b1c278798ad0e3d746dbda4f9f2586
(master and this pull)
SHA256SUMS.part 492a0384086452ac... 48c7ddfd9756018e...
*-aarch64-linux-gnu-debug.tar.gz 16867c76796473c4... c1740f5ed1829d8a...
*-aarch64-linux-gnu.tar.gz 84f0fc972786971e... 2c8effb6e5e9a853...
*-arm-linux-gnueabihf-debug.tar.gz 5b359cfd87fe6818... 228693ce8a43b5fe...
*-arm-linux-gnueabihf.tar.gz 9e96ecfbf8c48873... 1e0025ae3b9ba785...
*-osx-unsigned.dmg e5800e67dc17c84a... 13c4d2ae32a19ec8...
*-osx-unsigned.tar.gz 1a6e9cc4e80d90f8... c11b8f8b2fdee29c...
*-osx64.tar.gz 8ecd00cb3ff5b3d8... db9db6a8df4e8d86...
*-powerpc64-linux-gnu-debug.tar.gz 764ebaf4c6b477f9... a9e5f262832452e7...
*-powerpc64-linux-gnu.tar.gz a7edc8f7e6b99b6b... 52fb7f3f0fa30bb3...
*-powerpc64le-linux-gnu-debug.tar.gz 6c33e5a013404a04... cc6f3683811f66ce...
*-powerpc64le-linux-gnu.tar.gz d6325b26303e2ebe... 50e8b5e990e66349...
*-riscv64-linux-gnu-debug.tar.gz 16c387740dbb267a... 8f8a89fc1a2d72e4...
*-riscv64-linux-gnu.tar.gz 4e7af7c1951b26c8... aa4ea526f0ac523a...
*-win-unsigned.tar.gz 48b2aa0938825646... 63c6a919b0daaa95...
*-win64-debug.zip 06a1c171366eab17... 703712c1ca29e2ef...
*-win64-setup-unsigned.exe 18ce9615047ca3d8... 99a75af77b73157d...
*-win64.zip fb2c616343319e1c... d9adf6baa6c2b092...
*-x86_64-linux-gnu-debug.tar.gz 0f4f1b9674caec9d... d7f9cdb4527fa0af...
*-x86_64-linux-gnu.tar.gz 50c45e817dac2f49... a31c2912123b172c...
*.tar.gz 9108535d941ed527... ebc77c1c594cf89c...
guix_build.log 4abd25ecdb41ec97... 836766d09c5b6f99...
guix_build.log.diff 1772738928fa333a...

@laanwj
Copy link
Member

laanwj commented Oct 4, 2021

I missed this before, but I think it can now be removed from the commit message of 2261b186ebae38362eaceeed5ac1991b578375fa, as only lief is used now?

    Code reviewers: You may see that both lief and pixie are used for
    check_ELF_interpreter. That's because at the time of writing:
      - lief does not recognize riscv64 yet (fixed by fanquake in e83f812,
        not yet in a release)
      - pixie does not know how to extract the interpreter

Code review ACK otherwise.

@dongcarl dongcarl force-pushed the 2021-09-fix-loader-quick branch from 2261b18 to 6071282 Compare October 4, 2021 19:58
@dongcarl
Copy link
Contributor Author

dongcarl commented Oct 4, 2021

can now be removed from the commit message

Yes, done in latest push!

@laanwj
Copy link
Member

laanwj commented Oct 5, 2021

Thank you
Code review ACK 6071282e0af299ebc90ebc8d1b4662a081574bf5

@fanquake
Copy link
Member

Cherry-picked 2261b18 from scripts: use LIEF for ELF security & symbol checks #22392 as e759330

I've now cherry-picked this back into #22392

@dongcarl
Copy link
Contributor Author

Pushed 6071282e0af299ebc90ebc8d1b4662a081574bf5 -> b74a251e042b8d09c34fd452613736edc339ed30

I used Guix's values for the powerpc64(le) dynamic linkers, and the
/lib-prefix seems to be a Guix-ism rather than standard. The standard
path for the linker-loaders start with /lib64.

I've taken the new loader values from SYSDEP_KNOWN_INTERPRETER_NAMES in
glibc's sysdeps/unix/sysv/linux/powerpc/ldconfig.h file.

For future reference, loader path values can also be found on glibc's
website: https://sourceware.org/glibc/wiki/ABIList?action=recall&rev=16
@dongcarl dongcarl force-pushed the 2021-09-fix-loader-quick branch from b74a251 to c698045 Compare October 13, 2021 12:38
It is important that binaries request a standard interpreter location
where most distros would place the linker-loader. Otherwise, the user
would be met with a very confusing message:

    bash: <path>/<to>/bitcoind: No such file or directory

When really it's the interpreter that's not found.
@dongcarl dongcarl force-pushed the 2021-09-fix-loader-quick branch from c698045 to 1527b7e Compare October 13, 2021 12:40
@laanwj
Copy link
Member

laanwj commented Oct 13, 2021

Code review ACK 1527b7e

@dongcarl
Copy link
Contributor Author

Pushed b74a251e042b8d09c34fd452613736edc339ed30 -> 1527b7e

@laanwj laanwj merged commit 71a85fb into bitcoin:master Oct 13, 2021
@DrahtBot
Copy link
Contributor

Guix builds

File commit 5b7210c
(master)
commit 87de097f653076cb4574a8f55d96349cc1b13bf2
(master and this pull)
SHA256SUMS.part c79a21dc3da6e71b... 7ffec0abcb217c9d...
*-aarch64-linux-gnu-debug.tar.gz 108b2ef989bd0d35... 4212498cf999d15b...
*-aarch64-linux-gnu.tar.gz a5bacdca50e2c3f0... 6ca590b7d17b6e19...
*-arm-linux-gnueabihf-debug.tar.gz b21f70b81f75b1f6... 1b05d01dc8de08be...
*-arm-linux-gnueabihf.tar.gz 6c79c07aa1d2df22... a3beb72a91614007...
*-osx-unsigned.dmg e15275439c514402... 40085619b35e6122...
*-osx-unsigned.tar.gz 23080798bd4790d0... 1c0f88b31bbbaf61...
*-osx64.tar.gz f4afcac2e217616d... 78c1176c5f5feb9d...
*-powerpc64-linux-gnu-debug.tar.gz 7bb92bb01f337a75... f010bf4504626b0b...
*-powerpc64-linux-gnu.tar.gz 085787f59f3c1a58... abd6fd9146624239...
*-powerpc64le-linux-gnu-debug.tar.gz 2732fa036e1ba621... 090615a0dca94193...
*-powerpc64le-linux-gnu.tar.gz 495e1c10d8950c1a... a86689cc58e5f701...
*-riscv64-linux-gnu-debug.tar.gz 82c896f2e942879b... 110d9556341dd474...
*-riscv64-linux-gnu.tar.gz ebc9d158e7fe286e... ddb11327a627bd1a...
*-win-unsigned.tar.gz 4a871e218e240ccb... 615244f2bacbadfb...
*-win64-debug.zip 0ac6d7beea9e7a67... b393b85d15fea97e...
*-win64-setup-unsigned.exe 73d5ba8460a67851... 9a59d7fadecfc365...
*-win64.zip 104adfb4cd021131... 08f7e2e58321af3d...
*-x86_64-linux-gnu-debug.tar.gz 896c0ecaefff84eb... c8e094bd2ca04e69...
*-x86_64-linux-gnu.tar.gz 9cc9de13c910b2a4... 99cc8f6323418e6b...
*.tar.gz 0d51a99bd81f38ec... 30645bafa9e9af2a...
guix_build.log 463c2d4bbaa53638... fa57a9121c752c9e...
guix_build.log.diff e379947a692a777f...

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 13, 2021
…_ELF_interpreter

1527b7e symbol-check: Check requested ELF interpreter (Carl Dong)
b96adcb guix: Fix powerpc64(le) dynamic linker name (Carl Dong)

Pull request description:

  Fixes bitcoin#23111

  It would seem that I got the wrong default glibc-dynamic-linker path for PowerPC platforms. This means that for our currently released v22.0 binaries to be run on powerpc platforms, users would have to either:
  1. Move `/lib64/ld64.so.?` to `/lib`, or
  2. Invoke their linker-loader directly to start our binaries, e.g. `/lib64/ld64.so.? bitcoind`

  This is my bad.

  I've fixed the paths in this patchset, and also added a test to `symbol-check.py` so that this does not ever slip past our checks again.

ACKs for top commit:
  laanwj:
    Code review ACK 1527b7e

Tree-SHA512: bc520c35f72a9d4a3804b53d211138724560bd2405bf2f592ef755d19073e72f114fc4b8a3747e0c8724ac46a60b6ca86ea7766d66acb88eed1ebe2abc2678b8
@fanquake
Copy link
Member

Being backported to 22.x in #23276.

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Oct 17, 2021
I used Guix's values for the powerpc64(le) dynamic linkers, and the
/lib-prefix seems to be a Guix-ism rather than standard. The standard
path for the linker-loaders start with /lib64.

I've taken the new loader values from SYSDEP_KNOWN_INTERPRETER_NAMES in
glibc's sysdeps/unix/sysv/linux/powerpc/ldconfig.h file.

For future reference, loader path values can also be found on glibc's
website: https://sourceware.org/glibc/wiki/ABIList?action=recall&rev=16

Github-Pull: bitcoin#23148
Rebased-From: b96adcb
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Oct 21, 2021
I used Guix's values for the powerpc64(le) dynamic linkers, and the
/lib-prefix seems to be a Guix-ism rather than standard. The standard
path for the linker-loaders start with /lib64.

I've taken the new loader values from SYSDEP_KNOWN_INTERPRETER_NAMES in
glibc's sysdeps/unix/sysv/linux/powerpc/ldconfig.h file.

For future reference, loader path values can also be found on glibc's
website: https://sourceware.org/glibc/wiki/ABIList?action=recall&rev=16

Github-Pull: bitcoin#23148
Rebased-From: b96adcb
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Feb 15, 2022
I used Guix's values for the powerpc64(le) dynamic linkers, and the
/lib-prefix seems to be a Guix-ism rather than standard. The standard
path for the linker-loaders start with /lib64.

I've taken the new loader values from SYSDEP_KNOWN_INTERPRETER_NAMES in
glibc's sysdeps/unix/sysv/linux/powerpc/ldconfig.h file.

For future reference, loader path values can also be found on glibc's
website: https://sourceware.org/glibc/wiki/ABIList?action=recall&rev=16

Github-Pull: bitcoin#23148
Rebased-From: b96adcb
fanquake added a commit that referenced this pull request Mar 1, 2022
269553f test: Call ceildiv helper with integer (Martin Zumsande)
2f60fc6 ci: Replace soon EOL hirsute with jammy (MarcoFalke)
801b0f0 build: patch qt to explicitly define previously implicit header include (Kittywhiskers Van Gogh)
c768bfa tests: Calculate fees more similarly to CFeeRate::GetFee (Andrew Chow)
f66bc42 tests: Test for assertion when feerate is rounded down (Andrew Chow)
bd7e08e fees: Always round up fee calculated from a feerate (Andrew Chow)
227ae65 wallet: fix segfault by avoiding invalid default-ctored `external_spk_managers` entry (Sebastian Falbesoner)
282863a refactor: include a missing <limits> header in fs.cpp (Joan Karadimov)
7febe4f consensus: don't call GetBlockPos in ReadBlockFromDisk without lock (Jon Atack)
c671c6f the result of CWallet::IsHDEnabled() was initialized with true. (Saibato)
a5a1538 build, qt: Fix typo in QtInputSupport check (Hennadii Stepanov)
c95b188 system: skip trying to set the locale on NetBSD (fanquake)
c1cdedd guix: Fix powerpc64(le) dynamic linker name (Carl Dong)
92d44ff doc: Add 23061 release notes (MarcoFalke)
db76db7 Fix (inverse) meaning of -persistmempool (MarcoFalke)
85c78e0 build: Restrict check for CRC32C intrinsic to aarch64 (W. J. van der Laan)

Pull request description:

  Collecting backports for the 22.1 release. Currently:
  * #23045
  * #23061
  * #23148
  * #22390
  * #22820
  * #22781
  * #22895
  * #23335
  * #23333
  * #22949
  * #23580
  * #23504
  * #24239

ACKs for top commit:
  achow101:
    ACK 269553f

Tree-SHA512: b3a57ea241be7a83488eeb032276f4cf82a0987aada906a82f94a20c4acf9f2397708249dcecbe1c7575e70d09c60b835233d4718af4013c7bc58896c618274c
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bitcoin-22.0-powerpc64le-linux-gnu tarball does not execute

5 participants