[cfe-commits] r110317 - /cfe/trunk/lib/Checker/GRExprEngine.cpp

[Ted Kremenek]

Looks great Zhongxing.

We should also update the corresponding logic in ProcessAssume to also do the right checking to make sure that all checkers were evaluated.  Down the line we should also consider if we can merge this logic, instead of having separate copies.

On Aug 5, 2010, at 5:16 PM, Zhongxing Xu wrote:

> I see. How about this patch to make the check functional.
> 
> Index: lib/Checker/GRExprEngine.cpp
> ===================================================================
> --- lib/Checker/GRExprEngine.cpp	(版本 110392)
> +++ lib/Checker/GRExprEngine.cpp	(工作副本)
> @@ -206,6 +206,8 @@
>   unsigned checkersEvaluated = 0;
> 
>   for (CheckersOrdered::iterator I=CO->begin(), E=CO->end(); I!=E; ++I){
> +    if (PrevSet->empty())
> +      break;
>     ExplodedNodeSet *CurrSet = 0;
>     if (I+1 == E)
>       CurrSet = &Dst;
> 
> 
> On Thu, Aug 5, 2010 at 11:08 PM, Ted Kremenek <kremenek at apple.com> wrote:
>> I have reverted this.  This is definitely should not be an assertion.  Not all of the checkers will be evaluated if one of them generates a sink node.   The fact that this assertion doesn't trigger on our test cases means that there is a bug in the caching logic.  We probably need to bail out of the above loop when one of the ExplodedNodeSets becomes empty.  We don't want to creates a new checker set unless all of the checkers have evaluated; otherwise we won't know if we are leaving some checkers out just because one generated (and only generated) a sink node.
>> 
>> On Aug 5, 2010, at 12:38 AM, Zhongxing Xu wrote:
>> 
>>> Author: zhongxingxu
>>> Date: Thu Aug  5 02:38:23 2010
>>> New Revision: 110317
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=110317&view=rev
>>> Log:
>>> Turn the predicate into an assertion. When could the unequal case happen?
>>> 
>>> Modified:
>>>    cfe/trunk/lib/Checker/GRExprEngine.cpp
>>> 
>>> Modified: cfe/trunk/lib/Checker/GRExprEngine.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/GRExprEngine.cpp?rev=110317&r1=110316&r2=110317&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Checker/GRExprEngine.cpp (original)
>>> +++ cfe/trunk/lib/Checker/GRExprEngine.cpp Thu Aug  5 02:38:23 2010
>>> @@ -237,8 +237,10 @@
>>>   // If we built NewCO, check if we called all the checkers.  This is important
>>>   // so that we know that we accurately determined the entire set of checkers
>>>   // that responds to this callback.
>>> -  if (NewCO.get() && checkersEvaluated == Checkers.size())
>>> +  if (NewCO.get()) {
>>> +    assert(checkersEvaluated == Checkers.size());
>>>     CO_Ref = NewCO.take();
>>> +  }
>>> 
>>>   // Don't autotransition.  The CheckerContext objects should do this
>>>   // automatically.
>>> 
>>> 
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> 
>>