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

[Zhongxing Xu]

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
>
>