diff options
author | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-02-04 18:00:23 +0000 |
---|---|---|
committer | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-02-04 18:00:23 +0000 |
commit | ea6e14a3825d6f805527ddfbce4fd6b2bf73a7df (patch) | |
tree | f241129175e1162bc90d343dc7cba2d74e6300a6 /include | |
parent | 8df32b978f12e541af29d301e5d744cbd8f63676 (diff) |
TSAN: use somewhat pithier SK_ANNOTATE_UNPROTECTED_READ.
This is a little bit better practice to be i than the existing SK_ANNOTATE_BENIGN_RACE, as UNPROTECTED_READ will only ignore reads, not writes.
Tag SkRefCnt::unique() as a safe unprotected read like SkOnce's double-checked locking.
BUG=skia:
R=reed@google.com, bungeman@google.com
Author: mtklein@google.com
Review URL: https://codereview.chromium.org/144953005
git-svn-id: http://skia.googlecode.com/svn/trunk@13309 2bbb7eff-a529-9590-31e7-b0007b416f81
Diffstat (limited to 'include')
-rw-r--r-- | include/core/SkDynamicAnnotations.h | 46 | ||||
-rw-r--r-- | include/core/SkOnce.h | 19 | ||||
-rw-r--r-- | include/core/SkRefCnt.h | 7 |
3 files changed, 53 insertions, 19 deletions
diff --git a/include/core/SkDynamicAnnotations.h b/include/core/SkDynamicAnnotations.h new file mode 100644 index 0000000000..6d21cddb94 --- /dev/null +++ b/include/core/SkDynamicAnnotations.h @@ -0,0 +1,46 @@ +/* + * Copyright 2014 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkDynamicAnnotations_DEFINED +#define SkDynamicAnnotations_DEFINED + +// This file contains macros used to send out-of-band signals to dynamic instrumentation systems, +// namely thread sanitizer. This is a cut-down version of the full dynamic_annotations library with +// only the features used by Skia. + +// We check the same define to know to enable the annotations, but prefix all our macros with SK_. +#if DYNAMIC_ANNOTATIONS_ENABLED + +extern "C" { +// TSAN provides these hooks. +void AnnotateIgnoreReadsBegin(const char* file, int line); +void AnnotateIgnoreReadsEnd(const char* file, int line); +} // extern "C" + +// SK_ANNOTATE_UNPROTECTED_READ can wrap any variable read to tell TSAN to ignore that it appears to +// be a racy read. This should be used only when we can make an external guarantee that though this +// particular read is racy, it is being used as part of a mechanism which is thread safe. Examples: +// - the first check in double-checked locking; +// - checking if a ref count is equal to 1. +// Note that in both these cases, we must still add terrifyingly subtle memory barriers to provide +// that overall thread safety guarantee. Using this macro to shut TSAN up without providing such an +// external guarantee is pretty much never correct. +template <typename T> +inline T SK_ANNOTATE_UNPROTECTED_READ(const volatile T& x) { + AnnotateIgnoreReadsBegin(__FILE__, __LINE__); + T read = x; + AnnotateIgnoreReadsEnd(__FILE__, __LINE__); + return read; +} + +#else // !DYNAMIC_ANNOTATIONS_ENABLED + +#define SK_ANNOTATE_UNPROTECTED_READ(x) (x) + +#endif + +#endif//SkDynamicAnnotations_DEFINED diff --git a/include/core/SkOnce.h b/include/core/SkOnce.h index 9663ef13bf..59eaf598bc 100644 --- a/include/core/SkOnce.h +++ b/include/core/SkOnce.h @@ -28,6 +28,7 @@ // // You may optionally pass SkOnce a second function to be called at exit for cleanup. +#include "SkDynamicAnnotations.h" #include "SkThread.h" #include "SkTypes.h" @@ -115,26 +116,10 @@ static void sk_once_slow(SkOnceFlag* once, Func f, Arg arg, void (*atExit)()) { } } -// We nabbed this code from the dynamic_annotations library, and in their honor -// we check the same define. If you find yourself wanting more than just -// SK_ANNOTATE_BENIGN_RACE, it might make sense to pull that in as a dependency -// rather than continue to reproduce it here. - -#if DYNAMIC_ANNOTATIONS_ENABLED -// TSAN provides this hook to supress a known-safe apparent race. -extern "C" { -void AnnotateBenignRace(const char* file, int line, const volatile void* mem, const char* desc); -} -#define SK_ANNOTATE_BENIGN_RACE(mem, desc) AnnotateBenignRace(__FILE__, __LINE__, mem, desc) -#else -#define SK_ANNOTATE_BENIGN_RACE(mem, desc) -#endif - // This is our fast path, called all the time. We do really want it to be inlined. template <typename Func, typename Arg> inline void SkOnce(SkOnceFlag* once, Func f, Arg arg, void(*atExit)()) { - SK_ANNOTATE_BENIGN_RACE(&(once->done), "Don't worry TSAN, we're sure this is safe."); - if (!once->done) { + if (!SK_ANNOTATE_UNPROTECTED_READ(once->done)) { sk_once_slow(once, f, arg, atExit); } // Also known as a load-load/load-store barrier, this acquire barrier makes diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h index 28591920a6..e4524beff7 100644 --- a/include/core/SkRefCnt.h +++ b/include/core/SkRefCnt.h @@ -10,6 +10,7 @@ #ifndef SkRefCnt_DEFINED #define SkRefCnt_DEFINED +#include "SkDynamicAnnotations.h" #include "SkThread.h" #include "SkInstCnt.h" #include "SkTemplates.h" @@ -44,11 +45,13 @@ public: /** Return the reference count. Use only for debugging. */ int32_t getRefCnt() const { return fRefCnt; } - /** Returns true if the caller is the only owner. + /** May return true if the caller is the only owner. * Ensures that all previous owner's actions are complete. */ bool unique() const { - bool const unique = (1 == fRefCnt); + // We believe we're reading fRefCnt in a safe way here, so we stifle the TSAN warning about + // an unproctected read. Generally, don't read fRefCnt, and don't stifle this warning. + bool const unique = (1 == SK_ANNOTATE_UNPROTECTED_READ(fRefCnt)); if (unique) { // Acquire barrier (L/SL), if not provided by load of fRefCnt. // Prevents user's 'unique' code from happening before decrements. |