deps: Bump lief to 0.16.5

davidgumberg commented at 0:59 am on May 7, 2025: contributor

Partially resolves #30520, updating to lief 0.16.5.

DrahtBot commented at 0:59 am on May 7, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32431.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	laanwj, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25573 ([POC] guix: produce a fully -static-pie bitcoind by fanquake)
#24123 (guix: Pointer Authentication and Branch Target Identification for aarch64 Linux 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.

davidgumberg force-pushed on May 7, 2025

DrahtBot added the label CI failed on May 7, 2025

DrahtBot commented at 1:29 am on May 7, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/41764401882 LLM reason (✨ experimental): (empty)

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

davidgumberg force-pushed on May 7, 2025

laanwj commented at 5:51 am on May 7, 2025: member

Concept ACK.

Fixes an existing bug, where bcrypt.dll was not one of the expected symbols in contrib/devtools/symbol-check.py, even though this symbol gets included by way of secp256k1:

From what i understand, that’s only used for the example, not for the library itself. The bcrypt-using code (fill_random in examples/examples_util.h) is never included in the library.

It’s unlikely that the LIEF version changes anything there.

laanwj added the label Build system on May 7, 2025

hebasto commented at 6:13 am on May 7, 2025: member

~~Also the LIEF version must be bumped here~~

in contrib/guix/manifest.scm:163 in 3d1465f074 outdated

157@@ -158,7 +158,7 @@ chain for " target " development."))
158 (define-public python-lief
159   (package
160     (name "python-lief")
161-    (version "0.13.2")
162+    (version "0.16.5")

hebasto commented at 6:16 am on May 7, 2025:

Does Guix build succeed?

davidgumberg commented at 6:20 am on May 7, 2025:

In the process of testing this, I’ll mark as draft in the meanwhile.

hebasto commented at 6:17 am on May 7, 2025: member

Concept ACK.

davidgumberg force-pushed on May 7, 2025

davidgumberg commented at 6:23 am on May 7, 2025: contributor

From what i understand, that’s only used for the example, not for the library itself. The bcrypt-using code (fill_random in examples/examples_util.h) is never included in the library.

It’s unlikely that the LIEF version changes anything there.

Oops, my bad, I must have ran symbol-check.py against a stale build of the bcrypt branch, rebased to drop adding bcrypt to symbol-check.py, marking draft until I test this as working in Guix.

davidgumberg marked this as a draft on May 7, 2025

DrahtBot removed the label CI failed on May 7, 2025

fanquake commented at 8:05 am on May 7, 2025: member

Add usage notes for contrib/devtools/security-check.py

I think you can actually remove these docs. The symbol / security scripts are not for general developer usage, and there’s no expectation that the scripts should pass, outside of a Guix build. We could move them out of contrib to make this more clear.

Fixes an existing bug, where bcrypt.dll was not one of the expected symbols in contrib/devtools/symbol-check.py, even though this symbol gets included by way of secp256k1:

This seems to be a symptom of the above. Dependencies in libsecp256k1 example code, are not relevant for our release builds. Looking at a Guix build of master, I cannot see any dependency on bcrypt.dll in any binary.

in contrib/devtools/security-check.py:277 in ae6c79c319 outdated

283 
284 if __name__ == '__main__':
285     retval: int = 0
286     for filename in sys.argv[1:]:
287         binary = lief.parse(filename)
288+        if binary is None:

fanquake commented at 8:12 am on May 7, 2025:

Was this added to appease a linter? There should be no need for code like this, as every file passed to these scripts in Guix, is a binary. Leaving the script to fail if .parse fails is fine.

davidgumberg commented at 0:56 am on May 9, 2025:

Yes, but it was to appease the CI linter, I understand now that this script is not meant for general use, so I changed this to just cast to lief.Binary without checking

davidgumberg force-pushed on May 9, 2025

davidgumberg commented at 0:51 am on May 9, 2025: contributor

I think you can actually remove these docs.

Thanks, fixed. I did not understand that these were not meant for use outside of Guix builds,

I definitely opened this PR prematurely, thinking I could get away with just bumping the version and fixing the python scripts 😂.

Working on getting guix builds working right, I referenced prior art by @willcl-ark here (https://github.com/willcl-ark/bitcoin/commit/ede9aa93fe6339fe6285c954dc8fbd9ae8623916).

The python build is convinced that a build option is set in config-settings, even when I override configure-flags, which is where config-settings is supposed to get it’s value from (https://github.com/fanquake/guix/blob/ac2d792aae241f5233ee3fdfa29cd3dbaeb9338c/guix/build/pyproject-build-system.scm#L107-L111) :

 0starting phase `set-pythonpath'
 1phase `set-pythonpath' succeeded after 0.0 seconds
 2starting phase `change-directory'
 3phase `change-directory' succeeded after 0.0 seconds
 4starting phase `build'
 5Using 'setup' to build wheels, auto-detected 'setup', override '#f'.
 6ERROR: Unrecognized options in config-settings:
 7  build -> Did you mean: build-dir?
 8error: in phase 'build': uncaught exception:
 9%exception #<&invoke-error program: "python" arguments: ("-c" "import sys, importlib, json\nconfig_settings = json.loads (sys.argv[3])\nbuilder = importlib.import_module(sys.argv[1])\nbuilder.build_wheel(sys.argv[2], config_settings=config_settings)" "setup" "dist" "[]") exit-status: 7 term-signal: #f stop-signal: #f>
10phase `build' failed after 0.1 seconds
11command "python" "-c" "import sys, importlib, json\nconfig_settings = json.loads (sys.argv[3])\nbuilder = importlib.import_module(sys.argv[1])\nbuilder.build_wheel(sys.argv[2], config_settings

davidgumberg force-pushed on May 9, 2025

laanwj commented at 11:17 am on May 9, 2025: member

The symbol / security scripts are not for general developer usage, and there’s no expectation that the scripts should pass, outside of a Guix build. We could move them out of contrib to make this more clear.

i think this makes sense, especially for the symbol check. It doesn’t have any use outside the guix build. The security check may have some value as a more general “binary security check”. But moving both to a guix-specific path would be fine with me.

fanquake commented at 12:54 pm on May 9, 2025: member

But moving both to a guix-specific path would be fine with me.

See #32458.

DrahtBot added the label Needs rebase on May 12, 2025

willcl-ark commented at 12:43 pm on May 13, 2025: member

Hey @davidgumberg just getting around to the ping here (sorry).

I have a local manifest which builds 0.16.3 lief: https://gist.github.com/willcl-ark/3ac07722025b696910adba256d813cb6

As with the changes to Lief themselves I began, they def need a second look over, but perhaps it will be of some help.

davidgumberg force-pushed on May 20, 2025

davidgumberg renamed this:
~~deps: Bump lief to 0.16.5~~
deps: Bump lief to 0.16.4
on May 20, 2025

davidgumberg commented at 7:48 am on May 20, 2025: contributor

I have a local manifest which builds 0.16.3 lief: https://gist.github.com/willcl-ark/3ac07722025b696910adba256d813cb6

I was using basically this manifest taken from your branch (https://github.com/willcl-ark/bitcoin/tree/bump-lief), but it seems some build changes were made in lief 0.16.5 that prevent this from working, I’ve rebased, bumping to 0.16.4

davidgumberg marked this as ready for review on May 20, 2025

DrahtBot removed the label Needs rebase on May 20, 2025

fanquake commented at 4:14 pm on May 20, 2025: member

I think it’d be worth investigating any issues with 0.16.5, rather than deferring. Seeing some build output with this branch, which is unexpected:

0[100%] Built target bitcoin-qt
1Checking binary security...
2have_gnu_relro:#True and have_bindnow: True
3have_gnu_relro:#True and have_bindnow: True
4have_gnu_relro:#True and have_bindnow: True
5have_gnu_relro:#True and have_bindnow: True
6have_gnu_relro:#True and have_bindnow: True
7have_gnu_relro:#True and have_bindnow: True
8have_gnu_relro:#True and have_bindnow: True
9[100%] Built target check-security

davidgumberg force-pushed on May 20, 2025

deps: bump lief to 0.16.4

Some of the primary changes are:
- lief.EXE_FORMATS became lief.Binary.FORMATS IN 0.14.0
        - https://github.com/lief-project/LIEF/blob/494f116c6b8c9163fc4b5f6497233f18424981ee/doc/sphinx/changelog.rst?plain=1#L702
- lief.ARCHITECTURES became lief.Header.ARCHITECTURES in 0.16.0
        - https://github.com/lief-project/LIEF/blob/494f116c6b8c9163fc4b5f6497233f18424981ee/doc/sphinx/changelog.rst?plain=1#L226C18-L227C18
- lief.ELF.ARCH.x86_64 became lief.ELF.ARCH.X86_64

Co-authored-by: willcl-ark <will8clark@gmail.com>

2d26b4545e

davidgumberg force-pushed on May 20, 2025

davidgumberg commented at 6:39 pm on May 20, 2025: contributor

I think it’d be worth investigating any issues with 0.16.5, rather than deferring.

Sounds good, marking the PR as draft again while I investigate.

I’ve bisected to this commit in lief: https://github.com/lief-project/LIEF/commit/f23ced2f4ffc170d0a6f40ff4a1bee575e3447cf

Seems related to scikit-build 0.10 changing cmake.targets to build.targets, and defining build.targets explains why build is defined in config_settings.

Seeing some build output with this branch, which is unexpected:

[…]

Left some logging in, fixed.

davidgumberg marked this as a draft on May 20, 2025

davidgumberg renamed this:
~~deps: Bump lief to 0.16.4~~
deps: Bump lief to 0.16.5
on May 20, 2025

DrahtBot added the label CI failed on May 20, 2025

DrahtBot removed the label CI failed on May 23, 2025

deps: Bump lief to 0.16.5 #32431

Code Coverage & Benchmarks

Reviews

Conflicts