Enabled -Wthread-safety for Firefox builds
Randell Jesup
In coordination with bholley, nika, dveditz and others, I’ve
landed the main code for bug
1207753, supporting clang thread-safety annotations in
Gecko. This allows the compiler to warn us if static analysis of
lock usage indicates that a value supposed to be guarded by a
Mutex or Monitor is being accessed without an appropriate lock.
Chrome uses this as well.
Like our recent efforts to deploy
TSan, the objective of this work is to reduce the risk of
data races in Firefox. The two approaches — dynamic and static
analysis — are complementary, since each can identify classes of
bugs that the other can’t.
In developing this capability for Gecko, I've annotated about ⅓ of
our existing Mutexes and Monitors (focusing on the most
complicated and high-risk areas), and landed more than 50 patches
to address potential locking issues. I'm also putting up for
review ~100 patches that add those annotations. All
Mutexes/Monitors that haven't been annotated are marked with
MOZ_UNANNOTATED.
Generally speaking, it’s much less work to annotate the code when
it’s being written than it is to do so after the fact. So going
forward, we will require the following for newly-landed code:
- The code should not add any thread-safety warnings. This will be enforced by making clang thread-safety warnings fatal on autoland/m-c.
- Any new Mutexes and Monitors should include thread-safety
annotations. We’re adding a linter to check for this.
We are not currently planning a dedicated effort to exhaustively
annotate the rest of the preexisting Mutexes and Monitors, but
developers are encouraged to add them to cases they come across in
the normal course of their work.
Here is the documentation on how to add annotations, and deal with
warnings:
Thread-safety
docs
--
Randell Jesup