Nikos Mavrogiannopoulos nmav@redhat.com writes:
There are still two open issues with the static analyzer at [0].
A few weeks ago (prior to fixing the "dead increment" issue), I installed the clang static analyzer locally, but I didn't get the same issues reported. I got a few issues inside mini-gmp which looked like false positives (iirc, complaints were about some division function doing something bad in case qp was a NULL pointer but q wasn't, clang not realizing that the function logic makes qp == NULL if and only if q == NULL).
Both are at ecc-mod(), where the hi value can be used uninitialized on two occasions; ecc-mod.c:104 and ecc-mod.c:110. The issue can be addressed by initializing to zero, but although it looks reasonable, I'm not sure if that's the expectation of the algorithm.
I try to avoid initializing things just to avoid warnings. I'm reasonably confident there are no uninitialized values used at run time, since (1) all conditions involve sizes which are pretty static, not depending on numerical values of the bignums, and (2) I've seen no complaints about uninitialized values when running the testsuite under valgrind.
If we changed the code to initialize the variable was initialized to some dummy value, valgrind would no longer be able to spot such problems in this code.
Actually, I've also seen warnings about this code from gcc, for the use at line 104, but only when compiling for sparc (and on a machine that I no longer use).
Addressing that issue would allow for successful CI runs.
I agree that's desirable.
This code needs some careful analysis, to see under what conditions hi might be used uninitialized, and in case there's some valid inputs for which this could happen fix that, and if not, back up our assumptions with asserts (which I hope the static analyzer will understand).
Regards, /Niels