[cfe-commits] r57240 - /cfe/trunk/lib/Analysis/GRExprEngine.cpp

[Ted Kremenek]

On Oct 7, 2008, at 7:43 PM, Zhongxing Xu wrote:

> New patch:
>
> Index: lib/Analysis/GRExprEngine.cpp
> ===================================================================
> --- lib/Analysis/GRExprEngine.cpp       (revision 57240)
> +++ lib/Analysis/GRExprEngine.cpp       (working copy)
> @@ -1132,10 +1132,12 @@
>            case 5:
>              if (!memcmp(s, "panic", 5)) Builder->BuildSinks = true;
>              else if (!memcmp(s, "error", 5)) {
> -              Expr* Arg = *CE->arg_begin();
> -              if (IntegerLiteral* IL = dyn_cast<IntegerLiteral>(Arg))
> -                if (IL->getValue() != 0)
> +              if (CE->getNumArgs() > 0) {
> +                RVal X = GetRVal(St, *CE->arg_begin());
> +                nonlval::ConcreteInt* CI =  
> dyn_cast<nonlval::ConcreteInt>(&X);
> +                if (CI && CI->getValue() != 0)
>                    Builder->BuildSinks = true;
> +              }
>              }
>              break;

Looks great!  Please apply!

Two possible (optional) amendments.

If you expect that "error" takes a exact number of arguments and has a  
particular signature (e.g., the first parameter must be a typedef of a  
specific name), you may wish to match against those specific  
properties, since "error" may be a function that is implemented  
differently in different code bases.  Incidentally, what code base are  
you analyzing?

A second suggestion is that you could possibly use the "Assume" logic  
to check if the argument could be '1', and when it could be then make  
the node a sink.  This allows the path pruning to also work in the  
case of symbolic values, e.g:

   int foo(int code) {
    error(code);
    // more stuff that executes if code != 1
   }

The idea would be to split the state, just as we do with dereferences  
(i.e., where we assume that the pointer could be null and then also  
assume that the pointer is null).  It makes the code a little more  
complicated, but it does make it handle many more cases, and causes  
the analysis engine to reason about all the constraints that it has  
and not just prune the path when the RVal evaluates to the constant '1'.