diff options
| author | mtklein <mtklein@chromium.org> | 2014-11-05 08:03:27 -0800 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2014-11-05 08:03:27 -0800 |
| commit | e974c6244c92d32a0e4fe5639a913e78f101a056 (patch) | |
| tree | bcebf20e3c59a1a3400a91acbdb76f50cadce598 | |
| parent | a3d3065eeae5bb177895d3bd840c3d07706f5400 (diff) | |
| download | skia-e974c6244c92d32a0e4fe5639a913e78f101a056.tar.gz | |
Whitelist intentionally racy TRACE_EVENT reads and writes.
Chrome's tracing framework appears to be intentionally racy on its
quick-reject checks, trading some data loss for better performance
when disabled. People will never notice the data loss, but TSAN does.
Let's assuage TSAN with some annotations.
The 'volatile' val in SK_ANNOTATE_UNPROTECTED_WRITE was making this
not compile, but that volatile doesn't really make sense there: the value we're
writing is not what we care about, it's the destination.
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-TSAN-Trybot
No API changes.
TBR=reed
BUG=skia:
Review URL: https://codereview.chromium.org/702883002
| -rw-r--r-- | include/core/SkDynamicAnnotations.h | 2 | ||||
| -rw-r--r-- | src/core/SkTraceEvent.h | 7 |
2 files changed, 6 insertions, 3 deletions
diff --git a/include/core/SkDynamicAnnotations.h b/include/core/SkDynamicAnnotations.h index 422d98db4..872a292e5 100644 --- a/include/core/SkDynamicAnnotations.h +++ b/include/core/SkDynamicAnnotations.h @@ -42,7 +42,7 @@ inline T SK_ANNOTATE_UNPROTECTED_READ(const volatile T& x) { // Like SK_ANNOTATE_UNPROTECTED_READ, but for writes. template <typename T> -inline void SK_ANNOTATE_UNPROTECTED_WRITE(T* ptr, const volatile T& val) { +inline void SK_ANNOTATE_UNPROTECTED_WRITE(T* ptr, const T& val) { AnnotateIgnoreWritesBegin(__FILE__, __LINE__); *ptr = val; AnnotateIgnoreWritesEnd(__FILE__, __LINE__); diff --git a/src/core/SkTraceEvent.h b/src/core/SkTraceEvent.h index 34e3adf0e..2c3c9b418 100644 --- a/src/core/SkTraceEvent.h +++ b/src/core/SkTraceEvent.h @@ -162,6 +162,7 @@ #define SkTraceEvent_DEFINED #include "SkEventTracer.h" +#include "SkDynamicAnnotations.h" // By default, const char* argument values are assumed to have long-lived scope // and will not be copied. Use this macro to force a const char* to be copied. @@ -764,10 +765,12 @@ // write here because it's not strictly needed for correctness. // So says Nat. // FIXME +// +// Skia addition: we mark these as unprotected reads and writes to shut up TSAN. #define TRACE_EVENT_API_ATOMIC_WORD intptr_t -#define TRACE_EVENT_API_ATOMIC_LOAD(var) (*(&var)) -#define TRACE_EVENT_API_ATOMIC_STORE(var, value) (var=value) +#define TRACE_EVENT_API_ATOMIC_LOAD(var) SK_ANNOTATE_UNPROTECTED_READ(var) +#define TRACE_EVENT_API_ATOMIC_STORE(var, value) SK_ANNOTATE_UNPROTECTED_WRITE(&var, value) // Defines visibility for classes in trace_event.h #define TRACE_EVENT_API_CLASS_EXPORT SK_API |