Yubico/yubico-piv-tool

2.3.0 build failure

travisghansen opened this issue · 8 comments

travisghansen commented

I package for arch linux in the aur and trying to bump to 2.3.0 produces:

/home/thansen/Projects/aur/yubico-piv-tool/src/yubico-piv-tool-2.3.0/ykcs11/tests/ykcs11_tests_util.c: In function ‘test_rsa_sign_thorough’:
/home/thansen/Projects/aur/yubico-piv-tool/src/yubico-piv-tool-2.3.0/ykcs11/tests/ykcs11_tests_util.c:1047:14: error: ‘hdata_len’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 1047 |         asrt(EVP_PKEY_verify(ctx, sig, sig_len, hdata, hdata_len), 1, "EVP_PKEY_verify");
      |              ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [ykcs11/tests/CMakeFiles/test_ykcs11.dir/build.make:90: ykcs11/tests/CMakeFiles/test_ykcs11.dir/ykcs11_tests_util.c.o] Error 1
make[2]: Leaving directory '/home/thansen/Projects/aur/yubico-piv-tool/src/build'
make[1]: *** [CMakeFiles/Makefile2:380: ykcs11/tests/CMakeFiles/test_ykcs11.dir/all] Error 2
make[1]: Leaving directory '/home/thansen/Projects/aur/yubico-piv-tool/src/build'
make: *** [Makefile:146: all] Error 2
make: Leaving directory '/home/thansen/Projects/aur/yubico-piv-tool/src/build'
fuhry commented

This looks like gcc pickiness, hdata_len is initialized at the top of the function and passed by reference a few lines up. I don't see how it can be uninitialized. But I made a small patch for it anyway and got yubico-piv-tool 2.3.0 built on latest arch gcc.

Link to my patch against the AUR package: fuhry/yubico-piv-tool.aur@6219f37

travisghansen commented

Thanks @fuhry! I've updated the aur to include the patch and bumped the version now.

qpernil commented

@fuhry if you'd like to test this fix it would be appreciated. Thx in advance.

qpernil commented

I think the issue is that in some cases variables are declared & initialized at the top, but then they are used in a loop, so from one iteration to the next they are 'uninitialized' in the sense that they contain whatever value was left from the previous iteration. In #364 I have gotten rid of most if not all old K&R style declarations at the top of functions and instead almost exclusively using combined declarations and initialization. I also moved offending initialization inside the loops I mentioned. But I haven't tried compiling on arch, so I'm not 100% certain this will help, or maybe even introduce som new issue. If someone is able to help by compiling this branch on arch I'd appreciate it.

travisghansen commented

Can the full diff be applied against 2.3.0? If so I’ll just try that in a couple days (not available to test atm).

qpernil commented

There is one more commit on master now, which #364 is based on, so if you mean just taking the diff in that branch and applying to the yubico-piv-tool-2.3.0 tag then no, that won't work. But it can be applied to the current master, and maybe that's what you meant.

travisghansen commented

I meant 2.3.0

qpernil commented

Well #364 is two commits ahead of the yubico-piv-tool-2.3.0 tag, if you make a diff of those then you can obviously apply it without issue. Im still unsure if that's what you meant, i.e. what you mean by 'the full diff'. My initial answer was if you could just apply the last of those two, which gives some conflicts. I might have over-interpreted the question there.