On Oct 11, 2013, at 1:32 , Richard <[email protected]> wrote:

> 
> On 11 Oct 2013, at 03:00, Jordan Rose <[email protected]> wrote:
> 
>> 
>> On Oct 10, 2013, at 8:06 , Richard <[email protected]> wrote:
>> 
>>> I'm not sure if this is exactly what is happening. It is not at the 
>>> beginning of addTransition where we are failing to generate a sink. If 
>>> MarkAsSink is true, this optimisation will always be skipped. The problem 
>>> is in NodeBuilder::generateNodeImpl where IsNew is set to false for the 
>>> null object call in CallAndMessageChecker::checkPreCall if there is a 
>>> previous sink generated. In this case, 0 is always returned, so there is no 
>>> node to report the bug on. 
>>> 
>>> I am not sure why generating a sink on the not null path returns a cached 
>>> sink from the null path though. Is it because the true state and the not 
>>> true state both have the same predecessor with no other changes? If so, 
>>> what is the solution here?
>> 
>> Now I'm confused. Why would we have already generated a sink? None of the 
>> relevant checks here actually warn unless we're already fully-constrained, 
>> which means we should only be generating sinks in cases where there's no 
>> alternative. And that means we should never try to generate the second sink, 
>> because the first sink would have finished that path.
>> 
>> Can you attach your latest patch(es), so that we're on the same page?
>> 
>> Jordan
> 
> Current state is attached. The first sink generated is for the implicit null 
> dereference event, which occurs only in the not constrained case. But it 
> should be on a different path to the sink generated in checkPreCall, 
> shouldn't it?

Ah...yes, you were right the first time. It's not that the true and false state 
are the same, per se...it's that the two sinks have the same state that causes 
the analyzer to think they're the same. There are a few ways around this:

1. Ignore cases where the state doesn't change. DereferenceChecker doesn't do 
this explicitly, but it does skip non-Locs, which will mostly have the same 
effect.

2. Add an explicit tag. A SimpleProgramPointTag can be declared as a static 
variable to provide a tag that's different from the default checker tag.

Either one of these would work. #1 saves a bit of work when we have no 
information, but it's not impossible that we'll eventually end up having a case 
where an unconstrained UnknownVal should be considered a problem.

Jordan

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

Reply via email to