On Wed, Sep 13, 2017 at 8:09 AM, Niels Möller nisse@lysator.liu.se wrote:
nisse@lysator.liu.se (Niels Möller) writes:
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).
It turns out analysis isn't that subtle. The thing is, the static analyzer thinks that the loop condition (rn >= 2*mn - bn) can be initially false.
Now, rn is initialized to 2*mn, so this could be false only if the subtraction ounderflows. Which is doesn't do, since valid range for bn is 0 < bn < mn.
Question is what's the best way to make thhat clear to compilers and analyzers.
Loops could be rewritten as do {} while (0) loops. Or we could add more asserts, maybe it's sufficient to replace the somewhat weak
assert (sn > 0);
with
assert (bn < mn);
Since sn is size_t (unsigned), the former only checks that mn != bn.
I tried with few asserts() but couldn't eliminate this error. We may be hitting an analyzer bug, or missing something there.
I've even added:
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.
The only way I could eliminate the error was through the attached patch which is ugly.
Maybe we should put this whole function in #ifndef __clang_analyzer__
block to allow a smooth CI run.
regards, Nikos