[cfe-dev] NonNull attribute, bug report. (Or misunderstanding)

[Ted Kremenek]

There indeed was a serious bug in parsing attribute(nonnull), now fixed:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20081201/009943.html

Thanks so much for brining this to everyone's attention!

In your code I think if you use zero-based indexing for the arguments  
your test case should work as expected.  Please let us know if it  
doesn't.

On Dec 4, 2008, at 11:15 AM, Ted Kremenek wrote:

> Incidentally, I do think there is a clang bug here in how we process  
> the nonnull attribute.  I'm writing a few more test cases for the  
> analyzer to identify the issue.
>
> On Dec 4, 2008, at 10:39 AM, Ted Kremenek wrote:
>
>>
>> On Dec 4, 2008, at 4:27 AM, Paolo Bolzoni wrote:
>>
>>> Simple declaration:
>>> extern void* my_memcpy (void *dest, const void *src, unsigned long  
>>> len)
>>>    __attribute__((nonnull (1, 2)));
>>>
>>>
>>>
>>> The relative AST vertex do have the clang::Attr pointer. But this  
>>> simple loop
>>> outputs nothing:
>>>
>>> my_decl is a clang::FunctionDecl* relative to the function  
>>> declaration.
>>> non_null_attribute is a clang::NonNullAttr* relative to the non  
>>> null attribute
>>> of my_decl.
>>>
>>> for (unsigned int e = my_decl-> getNumParams(), b = 1; b <= e; + 
>>> +b) {
>>> if (non_null_attribute-> isNonNull(b)) {
>>>   std::cout << "Yes, parameter " << b << "is non-null!" <<  
>>> std::endl;
>>> }
>>> }
>>>
>>> What am I doing wrong? Or there indeed is a clang bug somewhere?
>>
>> Hi Paolo,
>>
>> You should use a zero-indexed numbering of the attributes.  '1' and  
>> '2' getting converted internally to '0' and '1' to match with the  
>> indexing of parameters in the ASTs.
>>
>> It may also be instructive to look at how the static analyzer  
>> consults this attribute (this is in GRExprEngineInternalChecks.cpp):
>>
>>   FunctionDecl* FD =  
>> dyn_cast<FunctionDecl>(cast<loc::FuncVal>(X).getDecl());
>>   const NonNullAttr* Att = FD->getAttr<NonNullAttr>();
>>
>>   if (!Att)
>>     return false;
>>
>>   // Iterate through the arguments of CE and check them for null.
>>
>>   unsigned idx = 0;
>>   bool hasError = false;
>>
>>   for (CallExpr::arg_iterator I=CE->arg_begin(), E=CE->arg_end(); I! 
>> =E;
>>        ++I, ++idx) {
>>
>>     if (!VMgr.isEqual(state, *I, 0) || !Att->isNonNull(idx))
>>       continue;
>>
>>     RangedBugReport R(BT, N);
>>     R.addRange((*I)->getSourceRange());
>>     Reports.push_back(R);
>>     hasError = true;
>>   }
>>
>> Since this code is so similar to yours, I'm a little confused why  
>> you don't get even one "Yes, parameter X is non-null" anywhere.   
>> One thing worth verifying is that 'e' is non-zero (i.e., is the  
>> look ever entered).  Inserting breakpoints in gdb would be my next  
>> step.
>>
>> There is also a test case for the static analyzer in test/Analysis  
>> that might be useful to compare against:
>>
>> $ cd test/Analysis
>> $ clang -checker-simple null-deref-ps.c
>> ...
>> ANALYZE: null-deref-ps.c f6
>> null-deref-ps.c:64:15: warning: Null pointer passed as an argument  
>> to a 'nonnull' parameter
>> return !p ? bar(p, 1) // expected-warning {{Null pointer passed as  
>> an argument to a 'nonnull' parameter}}
>>             ^   ~
>> ANALYZE: null-deref-ps.c f6b
>> null-deref-ps.c:71:15: warning: Null pointer passed as an argument  
>> to a 'nonnull' parameter
>> return !p ? bar2(p, 1) // expected-warning {{Null pointer passed as  
>> an argument to a 'nonnull' parameter}}
>>             ^    ~
>> ...
>