From owner-freebsd-arch@FreeBSD.ORG Thu Aug 28 17:02:58 2014 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D65C2E00; Thu, 28 Aug 2014 17:02:58 +0000 (UTC) Received: from mail-lb0-x234.google.com (mail-lb0-x234.google.com [IPv6:2a00:1450:4010:c04::234]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 09E6F1266; Thu, 28 Aug 2014 17:02:57 +0000 (UTC) Received: by mail-lb0-f180.google.com with SMTP id w7so1219295lbi.25 for ; Thu, 28 Aug 2014 10:02:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:date:message-id:subject:from:to:cc:content-type; bh=AIN+SpCXJTmKi8Qbs/CAA3Cr02G3MFKlt2jwi734a6c=; b=hXZv5lEzDqWVr/1gpXS/bucurwsu4EEBAokScuXwRBnCT4uRnQVPFSnk8PCnoRhor5 oTTP7Z++NrX4v3qdhNnADtNAM3TDx2rC0AukLmm9ZAD3oeRF/vqH2t9OIH6Z9Fgoe1pi wTptn+4D20mKfBmU88IYaCG8XOQn1QNfM8wV/ofy74xmj3XsWptBvFf5CL3IKV5l0UoN FGgazA0lFjkXpToPyaODMsLGUNg6SPU+Zo8Tne5FRQHxYwCkEB4qyVkvolu8mIba/HRa Qp0Qux6+RQoRE2yGN44tQW+F83te3AAz36EkW9OrtMA2Ym3PACR8EXpEhycTbPOCqPnb kEHg== MIME-Version: 1.0 X-Received: by 10.112.61.68 with SMTP id n4mr5354153lbr.91.1409245375509; Thu, 28 Aug 2014 10:02:55 -0700 (PDT) Sender: edschouten@gmail.com Received: by 10.152.144.2 with HTTP; Thu, 28 Aug 2014 10:02:55 -0700 (PDT) Date: Thu, 28 Aug 2014 19:02:55 +0200 X-Google-Sender-Auth: zZp9dGYxyBGLbliO0zYZO_0ZfgE Message-ID: Subject: Lock annotations: enable them for libpthread, libstdthreads From: Ed Schouten To: freebsd-arch@freebsd.org, freebsd-toolchain@freebsd.org Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Aug 2014 17:02:59 -0000 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 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 , 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