Date: Thu, 28 Aug 2014 19:02:55 +0200 From: Ed Schouten <ed@80386.nl> To: freebsd-arch@freebsd.org, freebsd-toolchain@freebsd.org Subject: Lock annotations: enable them for libpthread, libstdthreads Message-ID: <CAJOYFBBbzu9ouwfUk1%2B1MLW0B2y%2B5JTD9-b0%2BkiymOT=xKU0uQ@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
Hello all, As some of you may know, Clang supports some form of thread safety analysis by means of annotations: http://clang.llvm.org/docs/ThreadSafetyAnalysis.html To summarize, it works as follows: - You annotate which functions in your code (in)directly acquire/release/depend on which locks. - You annotate global variables and structure members to specify which locks should be held when accessing them (guards). You can then invoke Clang with -Wthread-safety and it will throw a compiler error in case these invariants don't hold. For example, in case you acquire a lock in an unannotated function and don't unlock it before the function returns, it will generate a warning. Couple of random observations on this feature from my side: - It seems that the thread safety stuff was mainly designed with C++ in mind. So far I haven't been able to get guarding annotations working for mutexes residing in structures; only global mutexes. Still, lock/unlock flow analysis on these mutexes works fine, which should already be a game-changer. - We can even use this feature to mark functions like pthread_cond_wait() as requiring the lock to be held. That's pretty sweet. - In userspace, almost all of the code compiled out of the box with -Wthread-safety turned on. I found one actual bug with this enabled (see r270749) and had to patch up a small number of files in auditdistd and hastd to annotate functions that acquire/release locks (see synch.h). I've written a patch that adds #defines for the attributes that are relevant for us (read: things that are useful for C programs) to <sys/cdefs.h> and patched up all functions in libpthread and libstdthreads to use these attributes where possible: http://80386.nl/pub/20140828-lock_annotate.txt One of my observations is that if we want to use this in C code seriously (which I hope), we would not use it in the same way as is done in C++ canonically. In kernel space it seems that our locking/unlocking strategy is a lot more asymmetric and non-trivial compared to high-level C++ programs. It is more common to have functions that pick up a lock and don't drop it or vice versa. Especially free()-like functions that expect the object to be locked. This is why I decided to go for a different naming scheme for the macros that is a bit shorter than what's used on the Clang page mentioned above. It is also a bit more accurate for our use case. Instead of it being "THE exclusive lock member function of a mutex", it is a function that just happens to "lock a mutex exclusively". My intent is to slowly push bits and pieces of this patch into HEAD within the next week. As the existing userspace only needs a very small number of patches to build with this flag, I am also going to add -Wthread-safety to WARNS=6, to make sure it won't regress. I'm also planning on MFC'ing at least the changes to <sys/cdefs.h>, so it is at least possible to write portable code. Be sure to take a look at the diff, play around with it and let me know what you think. Thanks, -- Ed Schouten <ed@80386.nl>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJOYFBBbzu9ouwfUk1%2B1MLW0B2y%2B5JTD9-b0%2BkiymOT=xKU0uQ>