Add SkMSAN.h
authormtklein <mtklein@chromium.org>
Wed, 3 Feb 2016 15:25:02 +0000 (07:25 -0800)
committerCommit bot <commit-bot@chromium.org>
Wed, 3 Feb 2016 15:25:02 +0000 (07:25 -0800)
This lets us tag up pieces of code as requiring initialized inputs.

Almost all code requires initialized inputs, of course.  This is for
code that works correctly with uninitialized data but triggers false
positive warnings in MSAN.  E.g., imagine MSAN's found use of uninitialized
data in this max function:

  static uint8_t max(uint8_t x, uint8_t y) { return x > y ? x : y; }

There's no bug in here... if there's uninitialized data being branched upon
here for the first time, it's sure not max's fault, it's its caller's fault.

So we might do this:
  static uint8_t max(uint8_t x, uint8_t y) {
      // This function uses branching, so if MSAN finds a problem here,
      // we can assert x and y are initialized.  This will remind us the
      // problem somewhere in the caller or above, not here.
      sk_msan_assert_initialized(&x, &x+1);
      sk_masn_assert_initialized(&y, &y+1);
      return x > y ? x : y;
  }

By allowing code to assert its inputs must be initialized,
we can make the blame for use of uninitialized data more clear.

(Sometimes we have another option, to rewrite the code to avoid branching:
  static uint8_t max(uint8_t x, uint8_t y) {
      // This function is branchfree, so MSAN won't complain here.
      // No real need to assert anything as requiring initialization.
      int diff = x - y;
      int negative = diff >> (sizeof(int)*8 - 1);
      return (y & negative) | (x & ~negative);
  }
These approaches to fixing MSAN false positives are orthogonal.)

BUG=chromium:574114
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1658913005
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot

Review URL: https://codereview.chromium.org/1658913005

src/core/SkMSAN.h [new file with mode: 0644]
src/opts/SkBlitRow_opts_SSE2.cpp
src/opts/SkBlitRow_opts_SSE4.cpp

diff --git a/src/core/SkMSAN.h b/src/core/SkMSAN.h
new file mode 100644 (file)
index 0000000..7e54477
--- /dev/null
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkMSAN_DEFINED
+#define SkMSAN_DEFINED
+
+#include <stddef.h>  // size_t
+
+// Typically declared in LLVM's msan_interface.h.  Easier for us to just re-declare.
+extern "C" {
+    void __msan_check_mem_is_initialized(const volatile void*, size_t);
+}
+
+// Code that requires initialized inputs can call this to make it clear that
+// the blame for use of uninitialized data belongs further up the call stack.
+static inline void sk_msan_assert_initialized(const void* begin, const void* end) {
+#if defined(__has_feature)
+    #if __has_feature(memory_sanitizer)
+        __msan_check_mem_is_initialized(begin, (const char*)end - (const char*)begin);
+    #endif
+#endif
+}
+
+#endif//SkMSAN_DEFINED
index c017f7e..3ca9a95 100644 (file)
@@ -11,6 +11,7 @@
 #include "SkColorPriv.h"
 #include "SkColor_opts_SSE2.h"
 #include "SkDither.h"
+#include "SkMSAN.h"
 #include "SkUtils.h"
 
 /* SSE2 version of S32_Blend_BlitRow32()
@@ -69,6 +70,8 @@ void S32_Blend_BlitRow32_SSE2(SkPMColor* SK_RESTRICT dst,
 void S32A_Opaque_BlitRow32_SSE2(SkPMColor* SK_RESTRICT dst,
                                 const SkPMColor* SK_RESTRICT src,
                                 int count, U8CPU alpha) {
+    sk_msan_assert_initialized(src, src+count);
+
     SkASSERT(alpha == 255);
     if (count <= 0) {
         return;
index 02ca54d..e5d8809 100644 (file)
@@ -19,11 +19,14 @@ void S32A_Opaque_BlitRow32_SSE4(SkPMColor* SK_RESTRICT, const SkPMColor* SK_REST
 #include <smmintrin.h>      // SSE4.1 intrinsics
 #include "SkColorPriv.h"
 #include "SkColor_opts_SSE2.h"
+#include "SkMSAN.h"
 
 void S32A_Opaque_BlitRow32_SSE4(SkPMColor* SK_RESTRICT dst,
                                 const SkPMColor* SK_RESTRICT src,
                                 int count,
                                 U8CPU alpha) {
+    sk_msan_assert_initialized(src, src+count);
+
     SkASSERT(alpha == 255);
     // As long as we can, we'll work on 16 pixel pairs at once.
     int count16 = count / 16;