devtools: Correctly extract symbol versions in symbol-check#22244
devtools: Correctly extract symbol versions in symbol-check#22244laanwj merged 3 commits intobitcoin:masterfrom
Conversation
I misunderstood the ELF specification for version symbols (verneed): The `vn_aux` pointer is relative to the main verneed record, not the start of the section. This caused many symbols to not be versioned properly in the return value of `elf.dyn_symbols`. This was discovered in bitcoin#21454. Fix it by correcting the offset computation.
xkb versions symbols (using the prefix `V`), as this library is used by bitcoin-qt, add it to the valid versions in `symbol-check.py`.
|
|
Is there a reference document that explains what the offset is? I found a doc that mentioned it, but my reading of that also came to the original conclusion that you made. |
I couldn't find anything. But this matches the All the other offsets ( |
|
Found it:
Source: Oracle, of all places… |
hebasto
left a comment
There was a problem hiding this comment.
Approach ACK c9b19edf9990d5ebf34945ce8e9a3220459d70f3, tested including comparison with master + reverted 634f6ec from #20434 (non-pixie).
* xkb versions symbols (using the prefix `V`), as this library is used by bitcoin-qt...
The libxkbcommon package was added in #21376.
This PR works flawlessly. Need more time to reason about the last commit to final ACK.
FWW, the last commit is a continuation of #17538 from 2019, which bumped the minimum glibc to 2.17. See also your comment here you were rightly confused: #17538 (comment) Maybe it would be better to rewrite the code so that the minimum GLIBC version isn't split over two places? |
…bol-check.py The (ancient) versions specified here were deceptive. Entries older than MAX_VERSIONS['GLIBC'], which is 2.17, are ignored here. So reorganize the code to avoid confusion for other people reading this code.
Okay, pushed a new last commit that does this. Instead of hardcoding a separate table for GLIBC, it optionally allows entries in |
hebasto
left a comment
There was a problem hiding this comment.
ACK e8cd370
Okay, pushed a new last commit that does this. Instead of hardcoding a separate table for GLIBC, it optionally allows entries in
MAX_VERSIONSto be a per-architecture dictionary. I hope this is clearer.
Many thanks for that!
On master, with applied #22281, #22287 and this PR: For |
…ymbol-check e8cd370 devtools: Integrate ARCH_MIN_GLIBC_VER table into MAX_VERSIONS in symbol-check.py (W. J. van der Laan) a33381a devtools: Add xkb version to symbol-check (W. J. van der Laan) 19e598b devtools: Fix verneed section parsing in pixie (W. J. van der Laan) Pull request description: I misunderstood the ELF specification for version symbols (verneed): The `vn_aux` pointer is relative to the main verneed record, not the start of the section. This caused many symbols to not be versioned properly in the return value of `elf.dyn_symbols`. This was discovered in bitcoin#21454. Fix it by correcting the offset computation. - xkb versions symbols (using the prefix `V`), as this library is used by bitcoin-qt, add it to the valid versions in `symbol-check.py` This unfortunately brings to light some symbols that have been introduced since and weren't caught (from a gitian compile of master): ``` bitcoin-cli: symbol getrandom from unsupported version GLIBC_2.25 bitcoin-cli: failed IMPORTED_SYMBOLS bitcoind: symbol getrandom from unsupported version GLIBC_2.25 bitcoind: symbol log from unsupported version GLIBC_2.29 bitcoind: symbol fcntl64 from unsupported version GLIBC_2.28 bitcoind: symbol pow from unsupported version GLIBC_2.29 bitcoind: symbol exp from unsupported version GLIBC_2.29 bitcoind: failed IMPORTED_SYMBOLS bitcoin-qt: symbol exp from unsupported version GLIBC_2.29 bitcoin-qt: symbol fcntl64 from unsupported version GLIBC_2.28 bitcoin-qt: symbol log from unsupported version GLIBC_2.29 bitcoin-qt: symbol pow from unsupported version GLIBC_2.29 bitcoin-qt: symbol statx from unsupported version GLIBC_2.28 bitcoin-qt: symbol getrandom from unsupported version GLIBC_2.25 bitcoin-qt: symbol renameat2 from unsupported version GLIBC_2.28 bitcoin-qt: symbol getentropy from unsupported version GLIBC_2.25 bitcoin-qt: failed IMPORTED_SYMBOLS bitcoin-wallet: symbol exp from unsupported version GLIBC_2.29 bitcoin-wallet: symbol log from unsupported version GLIBC_2.29 bitcoin-wallet: symbol fcntl64 from unsupported version GLIBC_2.28 bitcoin-wallet: failed IMPORTED_SYMBOLS test_bitcoin: symbol getrandom from unsupported version GLIBC_2.25 test_bitcoin: symbol log from unsupported version GLIBC_2.29 test_bitcoin: symbol fcntl64 from unsupported version GLIBC_2.28 test_bitcoin: symbol pow from unsupported version GLIBC_2.29 test_bitcoin: symbol exp from unsupported version GLIBC_2.29 test_bitcoin: failed IMPORTED_SYMBOLS ``` ACKs for top commit: hebasto: ACK e8cd370 Tree-SHA512: 8c15e3478eb642f01a1ddaadef03f80583f088f9fa8e3bf171ce16b0ec05ffb4675ec147d7ffc6a4360637ed47fca517c6ca2bac7bb30d794c03783cfb964b79
I misunderstood the ELF specification for version symbols (verneed): The
vn_auxpointer is relative to the main verneed record, not the start of the section.This caused many symbols to not be versioned properly in the return value of
elf.dyn_symbols. This was discovered in #21454.Fix it by correcting the offset computation.
V), as this library is used by bitcoin-qt, add it to the valid versions insymbol-check.pyThis unfortunately brings to light some symbols that have been introduced since and weren't caught (from a gitian compile of master):