On Jul 7, 2014, at 2:28 , Anders Rönnholm <[email protected]> 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.
>> 
>> Jordan
>> 
> 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.

Jordan
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to