Re: Thread safety annotations and analysis in GCC

["Le-Chun Wu" <lcwu@google.com>]

Ken,

Thanks a lot for your feedback. Somehow your email fell through the
cracks and I didn't notice it until a colleague of mine reminded me.
Sorry for my delay in replying.

Here are my answers to your questions.

>
> All the examples seem to be C++ oriented; is it, in fact, a goal for the
> annotations and analysis to be just as useful in C?

It's true that we started out with a design more C++ oriented, but it
is also our goal to make the annotations and analysis useful in C. For
example, the original design of the annotations for lock/unlock
primitives didn't take any argument (as we assumed the primitives
would be member functions of a lockable class), but later we changed
the design to allow the annotations to take lock arguments for C-style
lock/unlock primitives.


>
> What are the scoping rules used for finding the mutex referenced in the
> GUARDED_BY macro within a C++ class definition?  Are they the same as for
> looking up identifiers in other contexts?  How is the lookup done for C
> structures?

The scoping rules for annotations on class members are the same as for
looking up identifiers in, say member functions. We modified GCC
front-end to process the thread-safety attributes after the class
definition is finished parsing so that we can reference in the
annotations data members declared later in the class. For example,
referencing "mu" in the GUARDED_BY annotation in the following example
is allowed:

class Foo {
  int a GUARDED_BY(mu);
  Mutex mu;
};

Unfortunately in our current implementation (on the thread-annotations
branch) we haven't modified the C front-end to support this. We will
do that soon.

>
> Will the compiler get built-in knowledge of the OS library routines (e.g.,
> pthread_mutex_lock) on various platforms?

Currently no. Users will need to provide declarations with the
annotations for these library routines in their code. e.g. they will
need to provide a declaration for pthread_mutex_lock like the
following:

int pthread_mutex_lock(pthread_mutex_t *mutex) EXCLUSIVE_LOCK_FUNCTION(1);

>
> You list separate annotations for "trylock" functions.  It appears that the
> difference is that "trylock" functions can fail.  However,
> pthread_mutex_lock can fail, if the mutex isn't properly initialized, if
> recursive locking of a non-recursive mutex is detected, or other reasons;
> the difference between pthread_mutex_lock and pthread_mutex_trylock is
> whether it will wait or immediately return EBUSY for a mutex locked by
> another thread.  So I think pthread_mutex_lock should be described as a
> "trylock" function too, under your semantics.  Conservatively written code
> will check for errors, and will have a path in which the lock is assumed
> *not* to have been acquired; if the analysis assumes pthread_mutex_lock
> always succeeds, that path may be analyzed incorrectly.  (I ran into a tool
> once before that complained about my locking code until I added an unlock
> call to the error handling path.  Since it's actively encouraging writing
> incorrect code, I'm not using it any more.)
>

Good point. pthread_mutex_lock should be annotated as a trylock
routine so that the analyzer won't emit (bogus) warning messages in
the code shown below (which is similar to what you referred to).

if (! pthread_mutex_lock(mu))
  {
    /* Do something */
    pthread_mutex_unlock(mu);
  }
else
  {
    /* Error handling */
    /* No need to unlock */
  }

(It also means that the declaration example I gave above for
pthread_mutex_lock should be changed. :-))

Thanks again for your feedback. Please let me know if you have any
other comments/questions (or bugs if you start to play with the
analysis pass).

Le-chun