Nikos Mavrogiannopoulos n.mavrogiannopoulos@gmail.com writes:
I tried with few asserts() but couldn't eliminate this error. We may be hitting an analyzer bug, or missing something there.
In theory, it could also happen that rn = 2*mn overflows, potentially making rn == 0.
if (rn < 2 * mn - bn) <---NEW abort(); <--- NEW while (rn >= 2 * mn - bn)
prior to the while above, but the analyzer still thinks that the while path can be skipped.
I'd tend to say that's an analyzer bug. (Regarding analyzer and asserts, it might be useful to have it say which asserts it believes can't happen, but when it thinks an assert might trigger, it should still assume that it doesn't in following code.
The only way I could eliminate the error was through the attached patch which is ugly.
Rewriting using do {} while loop would make some sense, to make it clear both to the analyzer and to humans that loops are intended to run at least once. Would you like to try that out, to see if it gets ugly or not? Both loops should be changed in the same way, for consistency, even if only one matters at the moment.
BTW, I logged the input sizes when running the testsuite (which uses this for more moduli than the non-test code), and the possible cases are
normalized, mn = 4, sn = 2 unnormalized, mn = 3, sn = 1 unnormalized, mn = 4, sn = 1 unnormalized, mn = 4, sn = 3 unnormalized, mn = 6, sn = 3 unnormalized, mn = 9, sn = 4 unnormalized, mn = 9, sn = 8
for a 64-bit build. In a 32-bit bild, I get
normalized, mn = 7, sn = 4 normalized, mn = 8, sn = 1 unnormalized, mn = 12, sn = 6 unnormalized, mn = 12, sn = 7 unnormalized, mn = 17, sn = 16 unnormalized, mn = 17, sn = 8 unnormalized, mn = 6, sn = 3 unnormalized, mn = 7, sn = 3 unnormalized, mn = 8, sn = 7
Maybe we should put this whole function in #ifndef __clang_analyzer__
block to allow a smooth CI run.
If we find no better way, that would work to enable conntinuous static analysis of the rest
Regards, /Niels