rtc::MsanUninitialized to mark a trivially copiable object as uninitialized

Setting a default value for a class members prevents memory sanitizer
to behave correctly and may confuse the reader.
Instead, one should use rtc::MsanUninitialized, which creates an object of
a given type and marks its memory as uninitialized.
This prevents issues in production (due to uninitialized memory) and
allows MemorySantizier to catch invalid access patterns.

Bug: webrtc:8762
Change-Id: I74c79caa9c19ea85708e89e24bc5516c4d9d12a1
Reviewed-on: https://webrtc-review.googlesource.com/52342
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22773}

rtc_base/BUILD.gn

@@ -1184,7 +1184,9 @@ if (rtc_include_tests) {
 
   rtc_source_set("rtc_base_approved_unittests") {
     testonly = true
-
+    if (is_msan) {
+      cflags = [ "-fsanitize=memory" ]
+    }
     sources = [
       "atomicops_unittest.cc",
       "base64_unittest.cc",
@@ -1217,6 +1219,7 @@ if (rtc_include_tests) {
       "rate_statistics_unittest.cc",
       "ratetracker_unittest.cc",
       "refcountedobject_unittest.cc",
+      "sanitizer_unittest.cc",
       "string_to_number_unittest.cc",
       "stringencode_unittest.cc",
       "stringize_macros_unittest.cc",
@@ -1240,6 +1243,7 @@ if (rtc_include_tests) {
       ":rtc_task_queue",
       ":safe_compare",
       ":safe_minmax",
+      ":sanitizer",
       ":stringutils",
       "../api:array_view",
       "../system_wrappers:system_wrappers",

rtc_base/sanitizer.h

@@ -11,7 +11,11 @@
 #ifndef RTC_BASE_SANITIZER_H_
 #define RTC_BASE_SANITIZER_H_
 
-#include <stddef.h>  // for size_t
+#include <stddef.h>  // For size_t.
+
+#ifdef __cplusplus
+#include <type_traits>
+#endif
 
 #if defined(__has_feature)
 #if __has_feature(address_sanitizer)
@@ -90,6 +94,17 @@ static inline void rtc_MsanCheckInitialized(const volatile void* ptr,
 #ifdef __cplusplus
 
 namespace rtc {
+namespace sanitizer_impl {
+
+template <typename T>
+constexpr bool IsTriviallyCopyable() {
+  return static_cast<bool>(std::is_trivially_copy_constructible<T>::value &&
+                           (std::is_trivially_copy_assignable<T>::value ||
+                            !std::is_copy_assignable<T>::value) &&
+                           std::is_trivially_destructible<T>::value);
+}
+
+}  // namespace sanitizer_impl
 
 template <typename T>
 inline void AsanPoison(const T& mem) {
@@ -106,6 +121,15 @@ inline void MsanMarkUninitialized(const T& mem) {
   rtc_MsanMarkUninitialized(mem.data(), sizeof(mem.data()[0]), mem.size());
 }
 
+template <typename T>
+inline T MsanUninitialized(T t) {
+  // TODO(bugs.webrtc.org/8762): Switch to std::is_trivially_copyable when it
+  // becomes available in downstream projects.
+  static_assert(sanitizer_impl::IsTriviallyCopyable<T>(), "");
+  rtc_MsanMarkUninitialized(&t, sizeof(T), 1);
+  return t;
+}
+
 template <typename T>
 inline void MsanCheckInitialized(const T& mem) {
   rtc_MsanCheckInitialized(mem.data(), sizeof(mem.data()[0]), mem.size());

rtc_base/sanitizer_unittest.cc

@@ -0,0 +1,158 @@
+/*
+ *  Copyright 2018 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "rtc_base/sanitizer.h"
+
+#include "rtc_base/gunit.h"
+#include "rtc_base/logging.h"
+
+#if RTC_HAS_MSAN
+#include <sanitizer/msan_interface.h>
+#endif
+
+namespace rtc {
+namespace {
+
+// Test sanitizer_impl::IsTriviallyCopyable (at compile time).
+
+// Trivially copyable.
+
+struct TrTrTr {
+  TrTrTr(const TrTrTr&) = default;
+  TrTrTr& operator=(const TrTrTr&) = default;
+  ~TrTrTr() = default;
+};
+static_assert(sanitizer_impl::IsTriviallyCopyable<TrTrTr>(), "");
+
+struct TrDeTr {
+  TrDeTr(const TrDeTr&) = default;
+  TrDeTr& operator=(const TrDeTr&) = delete;
+  ~TrDeTr() = default;
+};
+static_assert(sanitizer_impl::IsTriviallyCopyable<TrDeTr>(), "");
+
+// Non trivially copyable.
+
+struct TrTrNt {
+  TrTrNt(const TrTrNt&) = default;
+  TrTrNt& operator=(const TrTrNt&) = default;
+  ~TrTrNt();
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<TrTrNt>(), "");
+
+struct TrNtTr {
+  TrNtTr(const TrNtTr&) = default;
+  TrNtTr& operator=(const TrNtTr&);
+  ~TrNtTr() = default;
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<TrNtTr>(), "");
+
+struct TrNtNt {
+  TrNtNt(const TrNtNt&) = default;
+  TrNtNt& operator=(const TrNtNt&);
+  ~TrNtNt();
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<TrNtNt>(), "");
+
+struct TrDeNt {
+  TrDeNt(const TrDeNt&) = default;
+  TrDeNt& operator=(const TrDeNt&) = delete;
+  ~TrDeNt();
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<TrDeNt>(), "");
+
+struct NtTrTr {
+  NtTrTr(const NtTrTr&);
+  NtTrTr& operator=(const NtTrTr&) = default;
+  ~NtTrTr() = default;
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<NtTrTr>(), "");
+
+struct NtTrNt {
+  NtTrNt(const NtTrNt&);
+  NtTrNt& operator=(const NtTrNt&) = default;
+  ~NtTrNt();
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<NtTrNt>(), "");
+
+struct NtNtTr {
+  NtNtTr(const NtNtTr&);
+  NtNtTr& operator=(const NtNtTr&);
+  ~NtNtTr() = default;
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<NtNtTr>(), "");
+
+struct NtNtNt {
+  NtNtNt(const NtNtNt&);
+  NtNtNt& operator=(const NtNtNt&);
+  ~NtNtNt();
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<NtNtNt>(), "");
+
+struct NtDeTr {
+  NtDeTr(const NtDeTr&);
+  NtDeTr& operator=(const NtDeTr&) = delete;
+  ~NtDeTr() = default;
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<NtDeTr>(), "");
+
+struct NtDeNt {
+  NtDeNt(const NtDeNt&);
+  NtDeNt& operator=(const NtDeNt&) = delete;
+  ~NtDeNt();
+};
+static_assert(!sanitizer_impl::IsTriviallyCopyable<NtDeNt>(), "");
+
+// Trivially copyable types.
+
+struct Foo {
+  uint32_t field1;
+  uint16_t field2;
+};
+
+struct Bar {
+  uint32_t ID;
+  Foo foo;
+};
+
+// Run the callback, and crash if it *doesn't* make an uninitialized memory
+// read. If MSan isn't on, just run the callback.
+template <typename F>
+void MsanExpectUninitializedRead(F&& f) {
+#if RTC_HAS_MSAN
+  // Allow uninitialized memory reads.
+  RTC_LOG(LS_INFO) << "__msan_set_expect_umr(1)";
+  __msan_set_expect_umr(1);
+#endif
+  f();
+#if RTC_HAS_MSAN
+  // Disallow uninitialized memory reads again, and verify that at least
+  // one uninitialized memory read happened while we weren't looking.
+  RTC_LOG(LS_INFO) << "__msan_set_expect_umr(0)";
+  __msan_set_expect_umr(0);
+#endif
+}
+
+}  // namespace
+
+// TODO(b/9116): Enable the test when the bug is fixed.
+TEST(SanitizerTest, DISABLED_MsanUninitialized) {
+  Bar bar = MsanUninitialized<Bar>({});
+  // Check that a read after initialization is OK.
+  bar.ID = 1;
+  EXPECT_EQ(1u, bar.ID);
+  RTC_LOG(LS_INFO) << "read after init passed";
+  // Check that other fields are uninitialized and equal to zero.
+  MsanExpectUninitializedRead([&] { EXPECT_EQ(0u, bar.foo.field1); });
+  MsanExpectUninitializedRead([&] { EXPECT_EQ(0u, bar.foo.field2); });
+  RTC_LOG(LS_INFO) << "read with no init passed";
+}
+
+}  // namespace rtc