[cfe-dev] [libc++] thread safety annotations for unique_lock

Richard Trieu via cfe-dev cfe-dev at lists.llvm.org
Wed Nov 9 11:31:02 PST 2016


It looks like you are running into the limits of what has been implemented
for thread safety analysis.  It doesn't look like SCOPED_CAPABILITY
supports unlocking and relocking the mutex.  In these cases, the usual way
to use a locking class is to do something like:

int main(int argc, char const *argv[]) {
  {
    unique_lock lock(mtx);
    any_number = 42;
  }
  int i = increment();

  unique_lock lock(mtx);
  return i;
}

The limitations section also says that it doesn't track pointer aliases:
http://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-alias-analysis

On Wed, Nov 9, 2016 at 8:03 AM, christoph at ruediger.engineering <
christoph at ruediger.engineering> wrote:

> Am 09.11.16, 01:32 schrieb "Richard Trieu" <rtrieu at google.com>:
>
> >
> >
> >On Tue, Nov 8, 2016 at 10:54 AM, christoph at ruediger.engineering via
> cfe-dev
> ><cfe-dev at lists.llvm.org> wrote:
> >
> >Sorry for the incomplete mail; hit the wrong shortcut. The mail continues
> down below.
> >
> >Am 08.11.16, 19:38 schrieb "cfe-dev im Auftrag von
> >cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>" <
> cfe-dev-bounces at lists.llvm.org im Auftrag von
> >cfe-dev at lists.llvm.org>:
> >
> >>Hi everyone,
> >>
> >>this is my first post to this list, so please remind me if I’m violating
> your netiquette in some way.
> >>
> >>I’m trying to introduce the thread safety analysis to one of our
> projects and stumble upon the missing thread safety annotations for
> unique_lock. I thought this might be easy, so let’s provide a patch. But
> now I’m stuck with correctly annotating the move constructor
> > and the lock() and unlock() methods of unique_lock.
> >>
> >>Here is a very basic implementation of my annotated unique_lock:
> >>
> >>class SCOPED_CAPABILITY unique_lock {
> >>  ::bluebox::mutex *mu;
> >>  bool lock_held;
> >>public:
> >>  unique_lock() = delete;
> >>  unique_lock(unique_lock &&) = delete;   // We cannot annotate move
> semantics
> >>
> >>  explicit unique_lock(::bluebox::mutex &m) ACQUIRE(m) : mu(&m),
> lock_held(true) { m.lock(); }
> >>  explicit unique_lock(::bluebox::mutex *m) ACQUIRE(m) : mu(m),
> lock_held(true) { m->lock(); }
> >
> >
> > Change this to ACQUIRE(mu) instead of ACQUIRE(m).  The ACQUIRE and
> RELEASE annotations need
> > to refer to the same variable for the thread safety analysis to work.
>
> This doesn’t work either. When putting ACQUIRE(mu) in the constructor
> annotation, I get this error:
>
> $ clang++-3.8 -Wthread-safety -O0 -std=c++11 mutex.cpp
> mutex.cpp:65:12: warning: writing variable 'any_number' requires holding
> mutex 'mtx' exclusively [-Wthread-safety-analysis]
>   return ++any_number;
>            ^
> mutex.cpp:70:3: warning: writing variable 'any_number' requires holding
> mutex 'mtx' exclusively [-Wthread-safety-analysis]
>   any_number = 42;
>   ^
> 2 warnings generated.
>
> I’ve put together a very minimalistic example which can be found at
> https://dl.dropboxusercontent.com/u/13595729/mutex.cpp
>
> The basic code fragment from the example is (with the mutex and unique
> locker as described in my previous mail):
> mutex mtx;
> int any_number GUARDED_BY(mtx) = 0;
>
>
> int increment() EXCLUDES(mtx) {
>   unique_lock lock(mtx);
>   return ++any_number;   // <- First warning here
> }
>
> int main(int argc, char const *argv[]) {
>   unique_lock lock(mtx);
>   any_number = 42;  // <- Second warning here
>   lock.unlock();
>   int i = increment();
>   lock.lock();
>   return i;
> }
>
>
> To me it looks like the analyzer does not get it that unique_lock::mu is
> essentially the same mutex as mtx.
>
> Just for reference, the warnings are exactly the same when using pointers
> instead of references in unique_lock.
>
>
>
>
> >
> >
> >>
> >>  void unlock() RELEASE(mu) NO_THREAD_SAFETY_ANALYSIS {
> >>    if (lock_held) {
> >>      lock_held = false;
> >>      mu->unlock();
> >>    }
> >>  }
> >>
> >>  void lock() ACQUIRE(mu) NO_THREAD_SAFETY_ANALYSIS {
> >>    if (!lock_held) {
> >>      mu->lock();
> >>      lock_held = true;
> >>    }
> >>  }
> >>
> >>  ~unique_lock() RELEASE() NO_THREAD_SAFETY_ANALYSIS {
> >>    if (lock_held)
> >>      mu->unlock();
> >>  }
> >>};
> >>
> >>
> >>And this is my test case:
> >>
> >>static std::mutex mtx;
> >>void someFunction() {
> >>  my::unique_lock lock(mtx);
> >>  // … Here you would normally do something meaningful
> >>  lock.unlock();
> >>  // … Call a method that might modify your protected data
> >>  lock.lock();
> >>  // … Continue with super important work
> >>}
> >>
> >>And here is my list of issues I can’t get around with:
> >>1. How can I correctly annotate the unlock and lock methods? When using
> unlock() in the above example, clang throws this error: fatal error:
> releasing mutex
> >>      'lock.mu <http://lock.mu>' that was not held
> [-Wthread-safety-analysis]
> >>2. How can I correctly annotate the move constructor of unique_lock?
> >
> >
> >
> >3. I couldn’t find really that much valuable information about the
> annotations except for the one page in the clang documentation and a talk
> from DeLesly Hutchins. Plus the information, that the guys at Google are
> making heavy use of the thread safety analyzer.
> > But are you really limiting yourself to std::mutex and std::lock_guard?
> You can’t even use std::condition_variable. Maybe one of the Google
> engineers on this list can shade some light without revealing confidential
> information.
> >
> >With these first tests in mind, I’m having troubles to use this really
> helpful feature in my projects. Anybody who wants to share some experience
> is appreciated. I will try to update the documentation accordingly or
> provide any other help.
> >
> >Thanks and regards,
> >Christoph
> >
> >--
> >rüdiger.engineering
> >Christoph Rüdiger
> >Düsseldorfer Str. 12
> >45145 Essen
> >Germany
> >
> >phone: +49 201 458 478 58 <tel:%2B49%20201%20458%20478%2058>
> >VAT ID/Ust. ID: DE304572498
> >
> >
> >_______________________________________________
> >cfe-dev mailing list
> >cfe-dev at lists.llvm.org
> >http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >
> >_______________________________________________
> >cfe-dev mailing list
> >cfe-dev at lists.llvm.org
> >http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >
> >
> >
> >
> >
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20161109/2b4007f4/attachment.html>


More information about the cfe-dev mailing list