[PATCH] Division by zero
Anders.Ronnholm at evidente.se
Wed Jul 9 04:41:48 PDT 2014
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>();
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.
Yes it seems like we can do that, don't know why it was a problem previously (before we began looping the exploded node) because this was what we did.
I have made a new patch now.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 18285 bytes
More information about the cfe-commits