[PATCH] Division by zero
jordan_rose at apple.com
Tue Jul 8 09:37:02 PDT 2014
On Jul 7, 2014, at 2:28 , Anders Rönnholm <Anders.Ronnholm at evidente.se> wrote:
>>> We don't get into the if-case because we never get a Loc from Sval therefore it always returns false and our tests are passed.
>>> Optional<Loc> L = S.getAs<Loc>();
>>> if (!L)
>>> return false;
>> Um. Right, we shouldn't get a Loc at that point. But we don't need a Loc. We just need whatever NonLoc symbol is there, and we can check to see if that is 0. Why did we need a Loc again?
>> Anyway, we shouldn't have features that don't show up in the tests. I think this code was either trying to avoid emitting duplicate messages if the denominator is known to be 0 already (which won't happen because that's a fatal error), or trying to avoid adding the symbol to the map if it's known not to be 0 (which is not what it's doing now, or at least not what it says it's doing). The latter is kind of useful but it's just optimization.
> Are you suggesting that we use NonLoc instead of Loc? Can we get an Sval from NonLoc? We needed Loc to get sval from state instead of looping the exploded node to find the range before the division.
Let's stop for a second. We want to check if the denominator's value is equal to 0. At this point in execution, the denominator expression is an rvalue, i.e. the integer value has already been loaded from the variable. That means the value of the denominator expression is going to be a NonLoc (because it represents an integer value rather than a location). Additionally, we don't need to load anything, because we're already past the point in analysis where the variable is loaded.
So if I'm right, you should be able to just take the denominator SVal as is, and feed it into the constraint manager if it's defined.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits