[cfe-commits] r162246 - in /cfe/trunk: lib/StaticAnalyzer/Core/BasicConstraintManager.cpp lib/StaticAnalyzer/Core/RangeConstraintManager.cpp test/Analysis/reference.cpp

[Jordan Rose]

I agree, but there's no obvious place to put it. We can't add an explicit assumption when the symbol is created because just creating new symbols doesn't update the state. And both constraint managers are lazy, but in different ways:

- RangeConstraintManager treats no range as "all possible values", but generates an actual RangeSet when it needs to do an intersection. This change takes 0 out of the generated set for references.
- BasicConstraintManager collects a set of inequalities unless you actually assume an equality constraint. This change added an implicit "!= 0" for references but doesn't actually change the state.

One way I could see to do this is to change SimpleConstraintManager's assumeSym?? methods to take an opaque type fetched from the state and return an Optional<opaque type> that would then be set back into the state if the assumption was feasible. Then you could also have an "addImplicitAssumptions" for the first time a state is accessed. But that would slow down every check on unconstrained symbols, and would require being non-lazy (and thus take more memory) to make sure we don't do it over and over. (Then again, we don't often make assumptions on completely unconstrained symbols that don't change the state in some way, and most if not all of those are caught before we even look at the ranges / inequalities.)

I'm not sure if that's the best way, though, or if it's really worth it at all. No one's actually ~using~ BasicConstraintManager right now.

Jordan


On Aug 20, 2012, at 21:23 , Ted Kremenek <kremenek at apple.com> wrote:

> Is there any way for us to pull this up higher?  Seems sub-optimal to replicate it in each ConstraintManager.
> 
> On Aug 20, 2012, at 5:27 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> 
>> Author: jrose
>> Date: Mon Aug 20 19:27:33 2012
>> New Revision: 162246
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=162246&view=rev
>> Log:
>> [analyzer] Assume that reference symbols are non-null.
>> 
>> By doing this in the constraint managers, we can ensure that ANY reference
>> whose value we don't know gets the effect, even if it's not a top-level
>> parameter.
>> 
>> Modified:
>>   cfe/trunk/lib/StaticAnalyzer/Core/BasicConstraintManager.cpp
>>   cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
>>   cfe/trunk/test/Analysis/reference.cpp
>> 
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/BasicConstraintManager.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BasicConstraintManager.cpp?rev=162246&r1=162245&r2=162246&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/BasicConstraintManager.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/BasicConstraintManager.cpp Mon Aug 20 19:27:33 2012
>> @@ -363,6 +363,10 @@
>> bool BasicConstraintManager::isNotEqual(ProgramStateRef state,
>>                                        SymbolRef sym,
>>                                        const llvm::APSInt& V) const {
>> +  // Special case: references are known to be non-zero.
>> +  if (sym->getType(getBasicVals().getContext())->isReferenceType())
>> +    if (V == 0)
>> +      return true;
>> 
>>  // Retrieve the NE-set associated with the given symbol.
>>  const ConstNotEqTy::data_type* T = state->get<ConstNotEq>(sym);
>> 
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp?rev=162246&r1=162245&r2=162246&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp Mon Aug 20 19:27:33 2012
>> @@ -380,7 +380,17 @@
>>  // given symbol type.
>>  BasicValueFactory &BV = getBasicVals();
>>  QualType T = sym->getType(BV.getContext());
>> -  return RangeSet(F, BV.getMinValue(T), BV.getMaxValue(T));
>> +
>> +  RangeSet Result(F, BV.getMinValue(T), BV.getMaxValue(T));
>> +
>> +  // Special case: references are known to be non-zero.
>> +  if (T->isReferenceType()) {
>> +    APSIntType IntType = BV.getAPSIntType(T);
>> +    Result = Result.Intersect(BV, F, ++IntType.getZeroValue(),
>> +                                     --IntType.getZeroValue());
>> +  }
>> +
>> +  return Result;
>> }
>> 
>> //===------------------------------------------------------------------------===
>> 
>> Modified: cfe/trunk/test/Analysis/reference.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=162246&r1=162245&r2=162246&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/reference.cpp (original)
>> +++ cfe/trunk/test/Analysis/reference.cpp Mon Aug 20 19:27:33 2012
>> @@ -1,3 +1,4 @@
>> +// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -analyzer-store=region -analyzer-constraints=basic -verify -Wno-null-dereference %s
>> // RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -analyzer-store=region -analyzer-constraints=range -verify -Wno-null-dereference %s
>> 
>> void clang_analyzer_eval(bool);
>> @@ -110,6 +111,16 @@
>>  y = 5; // expected-warning{{Dereference of null pointer}}
>> }
>> 
>> +void testReferenceAddress(int &x) {
>> +  clang_analyzer_eval(&x != 0); // expected-warning{{TRUE}}
>> +  clang_analyzer_eval(&ref() != 0); // expected-warning{{TRUE}}
>> +
>> +  struct S { int &x; };
>> +
>> +  extern S *getS();
>> +  clang_analyzer_eval(&getS()->x != 0); // expected-warning{{TRUE}}
>> +}
>> +
>> 
>> // ------------------------------------
>> // False negatives
>> @@ -127,5 +138,11 @@
>>    B *x = 0;
>>    return *x; // should warn here!
>>  }
>> +}
>> +
>> +void testReferenceFieldAddress() {
>> +  struct S { int &x; };
>> 
>> +  extern S getS();
>> +  clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
>> }
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>