Unavailable method checker

[Richard]

On 14 Oct 2013, at 18:29, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Oct 11, 2013, at 1:32 , Richard <tarka.t.otter at gmail.com> wrote:
> 
>> 
>> On 11 Oct 2013, at 03:00, Jordan Rose <jordan_rose at apple.com> wrote:
>> 
>>> 
>>> On Oct 10, 2013, at 8:06 , Richard <tarka.t.otter at gmail.com> 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
> 

I went with the Tag solution, nice and simple. So, attached is the current patch with passing test, any further suggestions?

Ta,
Richard


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131018/126ef7f9/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unavailable-1810.patch
Type: application/octet-stream
Size: 17788 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131018/126ef7f9/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131018/126ef7f9/attachment-0001.html>