test failures on 32-bit architectures
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
Thanks @dkg for the report. @ni4 do you have time for this?
@ronaldtse sure, I'll take a look.
@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.
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 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.
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?
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 right, 197f85e11fc64773d75b52ff9d82a79f3ed58f95 (also in this PR) addresses that failure.
@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!