build: Fix guix linker-loader path and add check_ELF

dongcarl commented at 0:58 am on October 1, 2021: member

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:

Move /lib64/ld64.so.? to /lib, or
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 added the label Needs backport (22.x) on Oct 1, 2021

dongcarl renamed this:
~~build: Fix guix linker-loader path and add symbol-checks~~
build: Fix guix linker-loader path and add check_ELF_interpreter
on Oct 1, 2021

fanquake added the label DrahtBot Guix build requested on Oct 1, 2021

dongcarl force-pushed on Oct 1, 2021

in contrib/devtools/symbol-check.py:264 in e95b5a9ca9 outdated

258@@ -244,11 +259,23 @@ def check_PE_subsystem_version(filename) -> bool:
259         return True
260     return False
261 
262+def check_ELF_interpreter(filename) -> bool:
263+    binary = lief.parse(filename)
264+    elf = pixie.load(filename)

laanwj commented at 7:09 am on October 1, 2021:

using both lief and pixie seems overkill here, please choose one :smile:

fanquake commented at 7:13 am on October 1, 2021:

I would opt for lief as we are moving that direction #22392.

dongcarl commented at 4:23 pm on October 1, 2021:

I explained this in the commit message:

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

~~Let me know if I’m missing some functionality here!~~

Oh! I see that fanquake’s PR has a workaround for lief riscv stuff, I’ll see if I can use that!

laanwj commented at 5:16 pm on October 4, 2021:

Oops, I missed that commit message, sorry.

laanwj commented at 7:09 am on October 1, 2021: member

Concept ACK, this is a good check to have.

dongcarl force-pushed on Oct 2, 2021

dongcarl commented at 0:11 am on October 2, 2021: member

Pushed e95b5a9ca9040cab826f041ab7cf348696a15ba3 -> 2261b186ebae38362eaceeed5ac1991b578375fa

Cherry-picked 2261b186ebae38362eaceeed5ac1991b578375fa from #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 commented at 5:05 pm on October 2, 2021: member

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 commented at 11:04 am on October 3, 2021: member

Guix builds

File	commit 35a31d5f7e9cd71a210c1ed10abc9d772ff36049(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...`

DrahtBot removed the label DrahtBot Guix build requested on Oct 3, 2021

laanwj commented at 5:19 pm on October 4, 2021: member

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

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

Code review ACK otherwise.

dongcarl force-pushed on Oct 4, 2021

dongcarl commented at 7:59 pm on October 4, 2021: member

can now be removed from the commit message

Yes, done in latest push!

laanwj commented at 7:43 am on October 5, 2021: member

Thank you Code review ACK 6071282e0af299ebc90ebc8d1b4662a081574bf5

laanwj added the label Build system on Oct 9, 2021

in contrib/devtools/symbol-check.py:68 in 6071282e0a outdated

64+# https://sourceware.org/glibc/wiki/ABIList?action=recall&rev=16
65+#
66+# The value is either a string, or a tuple whereby:
67+#     - the 0th item is the name for big-endian machines, and
68+#     - the 1st item is the name for little-endian machines
69+ELF_INTERPRETER_NAMES = {

laanwj commented at 9:32 am on October 9, 2021:

I’ve been thinking of this a bit and I’d slightly prefer to change this to:

0ELF_INTERPRETER_NAMES = {
1    lief.ELF.ARCH.i386:    (None, "/lib/ld-linux.so.2"),
2    lief.ELF.ARCH.x86_64:  (None, "/lib64/ld-linux-x86-64.so.2"),
3    lief.ELF.ARCH.ARM:     (None, "/lib/ld-linux-armhf.so.3"),
4    lief.ELF.ARCH.AARCH64: (None, "/lib/ld-linux-aarch64.so.1"),
5    lief.ELF.ARCH.PPC64:   ("/lib64/ld64.so.1", "/lib64/ld64.so.2"),
6    lief.ELF.ARCH(243):    (None, "/lib/ld-linux-riscv64-lp64d.so.1"),
7}

Then remove the if not isinstance(expected_interpreter, str): below. But add a check for “None” and fail in that case. This makes sure we do not encode any generalized information about platforms that don’t exist, or at least, we don’t support.

dongcarl commented at 5:36 pm on October 12, 2021:

Done, but changed the tuple into a dict, which seems clearer

laanwj commented at 12:38 pm on October 13, 2021:

Thanks! Yes, even better.

fanquake commented at 3:10 am on October 11, 2021: 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

MarcoFalke added the label DrahtBot Guix build requested on Oct 11, 2021

dongcarl force-pushed on Oct 12, 2021

dongcarl commented at 5:36 pm on October 12, 2021: member

Pushed 6071282e0af299ebc90ebc8d1b4662a081574bf5 -> b74a251e042b8d09c34fd452613736edc339ed30

Rebased on #22392
Addressed: #23148 (review)

guix: Fix powerpc64(le) dynamic linker name

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

b96adcbfae

in contrib/devtools/symbol-check.py:85 in b74a251e04 outdated

80+    },
81+    lief.ELF.ARCH.PPC64:   {
82+        lief.ENDIANNESS.BIG: "/lib64/ld64.so.1",
83+        lief.ENDIANNESS.LITTLE: "/lib64/ld64.so.2",
84+    },
85+    lief.ELF.ARCH(243):    {

laanwj commented at 12:38 pm on October 13, 2021:

Please use the temporary LIEF_ELF_ARCH_RISCV constant here that was introduced in 309eac9019c224dfd79a78e381cfcb70fee190f3.

dongcarl force-pushed on Oct 13, 2021

symbol-check: Check requested ELF interpreter

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.

1527b7e8a1

dongcarl force-pushed on Oct 13, 2021

laanwj commented at 12:41 pm on October 13, 2021: member

Code review ACK 1527b7e8a1339705a48d4d1ed108230fc4953c18

dongcarl commented at 12:41 pm on October 13, 2021: member

Pushed b74a251e042b8d09c34fd452613736edc339ed30 -> 1527b7e8a1339705a48d4d1ed108230fc4953c18

Rebased on master after merge of #22392
Addressed #23148 (review)

laanwj merged this on Oct 13, 2021

laanwj closed this on Oct 13, 2021

DrahtBot commented at 1:07 pm on October 13, 2021: member

Guix builds

File	commit 5b7210c8745d9572fe94620f848d4ee1304c91a7(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...`

DrahtBot removed the label DrahtBot Guix build requested on Oct 13, 2021

sidhujag referenced this in commit 7b0d7a057b on Oct 13, 2021

fanquake referenced this in commit c9371bf09b on Oct 14, 2021

fanquake removed the label Needs backport (22.x) on Oct 14, 2021

fanquake commented at 2:55 am on October 14, 2021: member

Being backported to 22.x in #23276.

fanquake referenced this in commit a75131e849 on Oct 15, 2021

luke-jr referenced this in commit cf549e858c on Oct 17, 2021

fanquake referenced this in commit 98c3294741 on Oct 21, 2021

fanquake referenced this in commit d26d466179 on Feb 14, 2022

fanquake referenced this in commit c1cdeddd90 on Feb 15, 2022

fanquake referenced this in commit 9b5f674abb on Mar 1, 2022

DrahtBot locked this on Oct 30, 2022

build: Fix guix linker-loader path and add check_ELF_interpreter #23148

Conflicts

Guix builds

Guix builds