rnp icon indicating copy to clipboard operation
rnp copied to clipboard

test failures on 32-bit architectures

Open dkg opened this issue 3 months ago • 9 comments

Over in https://bugs.debian.org/1116189 we're seeing test failures on 32-bit architectures:

103: [ RUN      ] rnp_tests.test_ffi_decrypt_v6_pkesk_test_vector
103: [armored_src_read() ./src/librepgp/stream-armor.cpp:378] Warning: missing or malformed CRC line
103: rnp_tests: ./src/librepgp/stream-sig.cpp:85: void signature_hash_key(const pgp_key_pkt_t&, rnp::Hash&, pgp_version_t): Assertion `key.pub_data.size() < ((size_t) 1 << 32)' failed.
 97/288 Test #103: rnp_tests.test_ffi_decrypt_v6_pkesk_test_vector ................................Subprocess aborted***Exception:   0.03 sec
...
97% tests passed, 9 tests failed out of 288

Total Test time (real) = 255.47 sec

The following tests FAILED:
	103 - rnp_tests.test_ffi_decrypt_v6_pkesk_test_vector (Subprocess aborted)
	104 - rnp_tests.test_ffi_encrypt_pk_with_v6_key (Subprocess aborted)
	111 - rnp_tests.test_ffi_v5_signatures (Subprocess aborted)
	165 - rnp_tests.test_ffi_v6_sig_subpackets (Subprocess aborted)
	166 - rnp_tests.test_ffi_v6_cert_import (Subprocess aborted)
	167 - rnp_tests.test_ffi_v6_seckey_import (Subprocess aborted)
	177 - rnp_tests.test_v5_keys (Subprocess aborted)
	179 - rnp_tests.test_v5_sec_keys (Subprocess aborted)
	287 - cli_tests-Misc (Failed)
Errors while running CTest
make[1]: *** [Makefile:94: test] Error 8

dkg avatar Sep 24 '25 16:09 dkg

Thanks @dkg for the report. @ni4 do you have time for this?

ronaldtse avatar Sep 25 '25 09:09 ronaldtse

@ronaldtse sure, I'll take a look.

ni4 avatar Sep 26 '25 10:09 ni4

@dkg thanks, was able to reproduce - while we had Debian 32bit CI runners, they made builds in Release mode for better performance, effectively disabling asserts. This exact issue is caused by checking against 1 << 32 size_t which goes wrong on 32-bit system.

ni4 avatar Sep 28 '25 16:09 ni4

yep, i think (size_t) 1 << 32 is undefined behavior on a 32-bit system (and at the very least it is an overflow). It's being used to compare against the size_type returned from std::vector<uint8_t>::size(). On a 32-bit system, the output of size() simply can't exceed 2³² as the test proposes here for "v5" (LibrePGP) and v6 keys. So what is this test actually doing on such a platform?

Do you have a proposed fix? I'd like to get this fixed up so that rnp can migrate in Debian.

dkg avatar Sep 30 '25 22:09 dkg

@dkg yeah, 1 << 32 would be zero, failing the assert. I'll commit fix for this soon, as well as for the v6 stuff which is not included into actively running tests yet.

ni4 avatar Oct 03 '25 17:10 ni4

just a ping on this, as this is preventing the good work in 0.18.0 from migrating to debian's testing distro. Shall i just patch out those asserts on 32-bit platforms? Shall i test with uint64_t instead of size_t?

dkg avatar Oct 07 '25 23:10 dkg

This unveiled another test failure on 32-bit architectures with 64-bit time_t: https://buildd.debian.org/status/fetch.php?pkg=rnp&arch=armhf&ver=0.18.0-2&stamp=1760052939&raw=0

281: ======================================================================
281: FAIL: test_set_current_time (__main__.Misc.test_set_current_time)
281: ----------------------------------------------------------------------
281: Traceback (most recent call last):
281:   File "/build/reproducible-path/rnp-0.18.0/src/tests/cli_tests.py", line 3847, in test_set_current_time
281:     self.assertEqual(ret, 0)
281:     ~~~~~~~~~~~~~~~~^^^^^^^^
281: AssertionError: 1 != 0
281: 
281: ----------------------------------------------------------------------
281: Ran 66 tests in 133.963s

https://github.com/rnpgp/rnp/blob/main/src/tests/cli_tests.py#L3847

AdrianBunk avatar Oct 10 '25 10:10 AdrianBunk

@AdrianBunk right, 197f85e11fc64773d75b52ff9d82a79f3ed58f95 (also in this PR) addresses that failure.

dkg avatar Oct 21 '25 04:10 dkg

@dkg thanks for pinging, I'll fix CI (as usual, something got broken with Github runners or other updates to the packages which are used in CI), and then add a fix for these issues. Sorry for a delay!

ni4 avatar Oct 21 '25 11:10 ni4