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

[Zhongxing Xu]

Is this OK?

Index: lib/Analysis/GRExprEngine.cpp
===================================================================
--- lib/Analysis/GRExprEngine.cpp       (revision 57240)
+++ lib/Analysis/GRExprEngine.cpp       (working copy)
@@ -1132,9 +1132,9 @@
           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)
+              RVal X = GetRVal(St, *CE->arg_begin());
+              if (nonlval::ConcreteInt* CI =
dyn_cast<nonlval::ConcreteInt>(&X))
+                if (CI->getValue() != 0)
                   Builder->BuildSinks = true;
             }
             break;


On Wed, Oct 8, 2008 at 12:49 AM, Ted Kremenek <kremenek at apple.com> wrote:

>
> On Oct 7, 2008, at 3:06 AM, Zhongxing Xu wrote:
>
>             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)
>> +                  Builder->BuildSinks = true;
>> +            }
>>            break;
>>
>
>
> Hi Zhongxing,
>
> I think this code will crash if error() isn't passed any arguments.  It
> should check to see if error() has the number of arguments that you expect.
>
> Also, is there a reason you check the AST node for an IntegerLiteral
> instead of just consulting the value of 'CE' that is in the state?  You'll
> get far more accurate information by looking at the semantics of the
> argument instead of its syntax.
>
> For example, checking for an integer literal will miss the following cases:
>
>  error(1 - 1);
>
>  error((1));  // note the '()'
>
>  x = 1;
>  error(x);
>
> I think just doing GetRVal(St, CE) should do the trick.
>
> Ted
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081008/9e3e9167/attachment.html>