Enable memcmp checking in tsan and msan builds.

And fix or suppress a few issues that the msan bot found.

The components/download change is a real bug fix: If we compared N
bytes but the file is only M < N bytes long, we used to read M
bytes and then compare N - M bytes of uninit memory, and if we were
unlucky we could've had a false positive. (Unlikely, but possible.)

Bug: 523428
Change-Id: I01b4c9bf312160eceb81980c731ab301a927341b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062290
Auto-Submit: Nico Weber <thakis@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742654}

base/profiler/stack_copier_signal_unittest.cc

@@ -85,8 +85,11 @@ class TargetThread : public SimpleThread {
 }  // namespace
 
 // ASAN moves local variables outside of the stack extents, which breaks the
-// sentinels. TSAN hangs on the AsyncSafeWaitableEvent FUTEX_WAIT call.
-#if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
+// sentinels.
+// MSan complains that the memcmp() reads uninitialized memory.
+// TSAN hangs on the AsyncSafeWaitableEvent FUTEX_WAIT call.
+#if defined(ADDRESS_SANITIZER) || defined(MEMORY_SANITIZER) || \
+    defined(THREAD_SANITIZER)
 #define MAYBE_CopyStack DISABLED_CopyStack
 #elif defined(OS_CHROMEOS)
 // https://crbug.com/1042974

build/sanitizers/sanitizer_options.cc

@@ -95,7 +95,7 @@ SANITIZER_HOOK_ATTRIBUTE const char *__asan_default_suppressions() {
 const char kTsanDefaultOptions[] =
     "detect_deadlocks=1 second_deadlock_stack=1 report_signal_unsafe=0 "
     "report_thread_leaks=0 print_suppressions=1 history_size=7 "
-    "strict_memcmp=0 strip_path_prefix=/../../ ";
+    "strip_path_prefix=/../../ ";
 
 SANITIZER_HOOK_ATTRIBUTE const char *__tsan_default_options() {
   return kTsanDefaultOptions;
@@ -111,12 +111,9 @@ SANITIZER_HOOK_ATTRIBUTE const char *__tsan_default_suppressions() {
 
 #if defined(MEMORY_SANITIZER)
 // Default options for MemorySanitizer:
-//   intercept_memcmp=0 - do not detect uninitialized memory in memcmp() calls.
-//     Pending cleanup, see http://crbug.com/523428
 //   strip_path_prefix=/../../ - prefixes up to and including this
 //     substring will be stripped from source file paths in symbolized reports.
-const char kMsanDefaultOptions[] =
-    "intercept_memcmp=0 strip_path_prefix=/../../ ";
+const char kMsanDefaultOptions[] = "strip_path_prefix=/../../ ";
 
 SANITIZER_HOOK_ATTRIBUTE const char *__msan_default_options() {
   return kMsanDefaultOptions;

components/download/internal/common/base_file.cc

@@ -235,7 +235,8 @@ bool BaseFile::ValidateDataInFile(int64_t offset,
     return true;
 
   std::unique_ptr<char[]> buffer(new char[data_len]);
-  if (file_.Read(offset, buffer.get(), data_len) <= 0)
+  int bytes_read = file_.Read(offset, buffer.get(), data_len);
+  if (bytes_read < 0 || static_cast<size_t>(bytes_read) < data_len)
     return false;
 
   return memcmp(data, buffer.get(), data_len) == 0;

sandbox/linux/seccomp-bpf/syscall_unittest.cc

@@ -226,6 +226,8 @@ TEST(Syscall, ComplexSyscallSixArgs) {
                                                           kPageSize
 #endif
                                                           )));
+#if !defined(MEMORY_SANITIZER)
+  // MSan considers the memory backing addr2 uninitialized.
   EXPECT_EQ(0, memcmp(addr2 + kPageSize, addr3, kPageSize));
 
   // Just to be absolutely on the safe side, also verify that the file
@@ -237,6 +239,7 @@ TEST(Syscall, ComplexSyscallSixArgs) {
                                                              2 * kPageSize
                                                              )));
   EXPECT_EQ(0, memcmp(addr2, buf, 2 * kPageSize));
+#endif
 
   // Clean up
   EXPECT_EQ(0, Syscall::Call(__NR_munmap, addr2, 2 * kPageSize));