[cfe-dev] Suggested starter bug on clang analyzer codebase

[Malhar Thakkar via cfe-dev]

Dear Dr. Devin,

Thank you for your response.
Just to be clear, I think I'll have to query what is known about the symbol
in PthreadLockChecker::ReleaseLock() instead of
PthreadLockChecker::AcquireLock() as pthread_mutex_destroy() returns a
non-zero value when a mutex is locked by another thread.
So, in order to destroy that mutex, one would first need to unlock it and
then destroy it.

The code snippet which produces a false positive in the current checker:

if(pthread_mutex_destroy(&(o->lock_mutex))!=0) {
   pthread_mutex_unlock(&(o->lock_mutex)); // It raises a warning here.
   pthread_mutex_destroy(&(o->lock_mutex));
}


Thank you.


Regards,
Malhar
ᐧ

On Wed, Apr 12, 2017 at 11:13 PM, Devin Coughlin <dcoughlin at apple.com>
wrote:

>
> On Apr 12, 2017, at 3:32 AM, Malhar Thakkar <cs13b1031 at iith.ac.in> wrote:
>
> Hello Dr. Devin,
>
> I have the following approach in mind.
> Just like the Unix API stream checker (where the return value of fopen is
> checked), we can use the constraint manager to check if the return value
> of any function (for example: pthread_mutex_destroy) is zero or not.
> If it is non-zero (which indicates failure), then we don't update the set
> & map which are used to keep track of the states all mutex variables.
>
> Let me know your thoughts on this.
>
>
> Malhar,
>
> These sounds like the right approach to me, although I would focus only on
> pthread_mutex_destroy()’s return value for now.
>
> One thing that makes this slightly tricky is that at the point of the
> return from the call to pthread_mutex_destroy() we don’t know anything
> about the return value — it is only later, when the programmer checks that
> value (in a if statement, for example) that the analyzer can discover
> whether the value returned was zero or not.
>
> Here is an example:
>     pthread_mutex_t m;
>     pthread_mutex_init(&m, NULL);
>
>     // …
>
>     int result = pthread_mutex_destroy(&m);
>     // (1) At this point the analyzer doesn’t know value of result.
>     // It tracks it as a symbol with no constraints
>
>     if (result != 0) { // (2) At this point the analyzer constrains the
> symbol from (1) to be non-zero
>
> // (3) At this point the checker should look at the at the symbol.
> // If it is unknown or is definitely zero (meaning the destroy succeeded)
> // it should emit a diagnostic. But if it is definitely non-zero then
> // the analyzer should not report the diagnostic.
>        pthread_mutex_lock(&m);
>     }
>
> What this means is that you’ll need to modify PthreadLockChecker::DestroyLock()
> to keep track of the returned symbol from pthread_mutex_destroy() in the
> LockState and then query what is known about that symbol in
> PthreadLockChecker::AcquireLock().
>
> Please don’t hesitate to ask if you more questions!
>
> Devin
>
> P.S. In the future you should CC the cfe-dev mailing on these kinds of
> questions so that the community can offer their own input and get a sense
> of what you are working on!
>
>
>
>
> Thank you.
>
>
> Regards,
> Malhar
> ᐧ
>
> On Fri, Apr 7, 2017 at 2:26 AM, Devin Coughlin <dcoughlin at apple.com>
> wrote:
>
>> Hi Malhar (+Michael, cc’d),
>>
>> Here is a suggested starter bug on the clang static analyzer code base:
>>
>> “False Positive PthreadLockChecker Use destroyed lock” <
>> https://bugs.llvm.org/show_bug.cgi?id=32455>
>>
>> The analyzer has a checker (PthreadLockChecker.cpp) that emits a
>> diagnostic when the programmer tries to acquire a mutex twice or lock a
>> mutex that has already been destroyed. Unfortunately, it has a false
>> positive: if the programmer calls pthread_destroy_lock() and that call
>> fails (returns a non-zero value) and then later tries to lock then the
>> analyzer incorrectly warns that the mutex has been destroyed. There is an
>> example of the false positive in the linked bugzilla URL above.
>>
>> Fixing this false positive is a good starter bug to get you familiar with
>> the checker side of the analyzer. If you haven’t seen it already, a good
>> place to start would be the Checker Development Manual <
>> https://clang-analyzer.llvm.org/checker_dev_manual.html>.
>>
>> And please don’t hesitate to email if you have any questions or want to
>> discuss approaches to fixing the bug!
>>
>> Devin
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170413/f2cf10ea/attachment.html>