Fix MSan failures

Bug: 932205
Change-Id: Ic31986d270634e42bf8c2620f37c434a4cb79b33
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1474271
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>

client/crash_report_database_generic.cc

@@ -26,6 +26,7 @@
 #include "util/file/directory_reader.h"
 #include "util/file/filesystem.h"
 #include "util/misc/initialization_state_dcheck.h"
+#include "util/misc/memory_sanitizer.h"
 
 namespace crashpad {
 
@@ -1003,6 +1004,11 @@ bool CrashReportDatabaseGeneric::WriteNewMetadata(const base::FilePath& path) {
   }
 
   ReportMetadata metadata;
+#if defined(MEMORY_SANITIZER)
+  // memset() + re-initialization is required to zero padding bytes for MSan.
+  memset(&metadata, 0, sizeof(metadata));
+#endif  // defined(MEMORY_SANITIZER)
+  metadata = {};
   metadata.creation_time = time(nullptr);
 
   return LoggingWriteFile(handle.get(), &metadata, sizeof(metadata));
@@ -1023,6 +1029,11 @@ bool CrashReportDatabaseGeneric::WriteMetadata(const base::FilePath& path,
   }
 
   ReportMetadata metadata;
+#if defined(MEMORY_SANITIZER)
+  // memset() + re-initialization is required to zero padding bytes for MSan.
+  memset(&metadata, 0, sizeof(metadata));
+#endif  // defined(MEMORY_SANITIZER)
+  metadata = {};
   metadata.creation_time = report.creation_time;
   metadata.last_upload_attempt_time = report.last_upload_attempt_time;
   metadata.upload_attempts = report.upload_attempts;

client/crashpad_client_linux_test.cc

@@ -348,7 +348,7 @@ class StartHandlerForClientTest {
     static void HandleCrash(int signo, siginfo_t* siginfo, void* context) {
       auto state = Get();
 
-      char c;
+      char c = 0;
       CHECK(LoggingWriteFile(state->client_sock_, &c, sizeof(c)));
 
       ExceptionInformation exception_information;

snapshot/linux/process_reader_linux_test.cc

@@ -50,6 +50,7 @@
 #include "util/linux/direct_ptrace_connection.h"
 #include "util/misc/address_sanitizer.h"
 #include "util/misc/from_pointer_cast.h"
+#include "util/misc/memory_sanitizer.h"
 #include "util/synchronization/semaphore.h"
 
 #if defined(OS_ANDROID)
@@ -337,6 +338,11 @@ class ChildThreadTest : public Multiprocess {
     thread_pool.StartThreads(kThreadCount, stack_size_);
 
     TestThreadPool::ThreadExpectation expectation;
+#if defined(MEMORY_SANITIZER)
+    // memset() + re-initialization is required to zero padding bytes for MSan.
+    memset(&expectation, 0, sizeof(expectation));
+#endif  // defined(MEMORY_SANITIZER)
+    expectation = {};
     expectation.tls = GetTLS();
     expectation.stack_address = reinterpret_cast<LinuxVMAddress>(&thread_pool);
 
@@ -771,7 +777,7 @@ class ChildModuleTest : public Multiprocess {
     ScopedModuleHandle empty_test_module(LoadTestModule(module_name_));
     ASSERT_TRUE(empty_test_module.valid());
 
-    char c;
+    char c = 0;
     ASSERT_TRUE(LoggingWriteFile(WritePipeHandle(), &c, sizeof(c)));
 
     CheckedReadFileAtEOF(ReadPipeHandle());

util/BUILD.gn

@@ -108,6 +108,7 @@ static_library("util") {
     "misc/initialization_state_dcheck.h",
     "misc/lexing.cc",
     "misc/lexing.h",
+    "misc/memory_sanitizer.h",
     "misc/metrics.cc",
     "misc/metrics.h",
     "misc/paths.h",

util/linux/exception_handler_protocol.cc

@@ -20,6 +20,9 @@ ExceptionHandlerProtocol::ClientInformation::ClientInformation()
     : exception_information_address(0), sanitization_information_address(0) {}
 
 ExceptionHandlerProtocol::ClientToServerMessage::ClientToServerMessage()
-    : version(kVersion), type(kTypeCrashDumpRequest), client_info() {}
+    : version(kVersion),
+      type(kTypeCrashDumpRequest),
+      requesting_thread_stack_address(0),
+      client_info() {}
 
 }  // namespace crashpad

util/linux/ptrace_broker.cc

@@ -24,6 +24,7 @@
 
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
+#include "util/misc/memory_sanitizer.h"
 
 namespace crashpad {
 
@@ -355,6 +356,10 @@ int PtraceBroker::SendMemory(pid_t pid, VMAddress address, VMSize size) {
   return 0;
 }
 
+#if defined(MEMORY_SANITIZER)
+// MSan doesn't intercept syscall() and doesn't see that buffer is initialized.
+__attribute__((no_sanitize("memory")))
+#endif  // defined(MEMORY_SANITIZER)
 int PtraceBroker::SendDirectory(FileHandle handle) {
   char buffer[4096];
   int rv;

util/linux/ptrace_client.cc

@@ -62,7 +62,7 @@ bool ReceiveAndLogReadError(int sock, const std::string& operation) {
 }
 
 bool AttachImpl(int sock, pid_t tid) {
-  PtraceBroker::Request request;
+  PtraceBroker::Request request = {};
   request.type = PtraceBroker::Request::kTypeAttach;
   request.tid = tid;
   if (!LoggingWriteFile(sock, &request, sizeof(request))) {
@@ -136,7 +136,7 @@ PtraceClient::PtraceClient()
 
 PtraceClient::~PtraceClient() {
   if (sock_ != kInvalidFileHandle) {
-    PtraceBroker::Request request;
+    PtraceBroker::Request request = {};
     request.type = PtraceBroker::Request::kTypeExit;
     LoggingWriteFile(sock_, &request, sizeof(request));
   }
@@ -151,7 +151,7 @@ bool PtraceClient::Initialize(int sock, pid_t pid, bool try_direct_memory) {
     return false;
   }
 
-  PtraceBroker::Request request;
+  PtraceBroker::Request request = {};
   request.type = PtraceBroker::Request::kTypeIs64Bit;
   request.tid = pid_;
 
@@ -197,7 +197,7 @@ bool PtraceClient::Is64Bit() {
 bool PtraceClient::GetThreadInfo(pid_t tid, ThreadInfo* info) {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 
-  PtraceBroker::Request request;
+  PtraceBroker::Request request = {};
   request.type = PtraceBroker::Request::kTypeGetThreadInfo;
   request.tid = tid;
   if (!LoggingWriteFile(sock_, &request, sizeof(request))) {
@@ -222,7 +222,7 @@ bool PtraceClient::ReadFileContents(const base::FilePath& path,
                                     std::string* contents) {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 
-  PtraceBroker::Request request;
+  PtraceBroker::Request request = {};
   request.type = PtraceBroker::Request::kTypeReadFile;
   request.path.path_length = path.value().size();
 
@@ -273,7 +273,7 @@ bool PtraceClient::Threads(std::vector<pid_t>* threads) {
   char path[32];
   snprintf(path, base::size(path), "/proc/%d/task", pid_);
 
-  PtraceBroker::Request request;
+  PtraceBroker::Request request = {};
   request.type = PtraceBroker::Request::kTypeListDirectory;
   request.path.path_length = strlen(path);
 
@@ -324,7 +324,7 @@ ssize_t PtraceClient::ReadUpTo(VMAddress address,
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   char* buffer_c = reinterpret_cast<char*>(buffer);
 
-  PtraceBroker::Request request;
+  PtraceBroker::Request request = {};
   request.type = PtraceBroker::Request::kTypeReadMemory;
   request.tid = pid_;
   request.iov.base = address;

util/misc/capture_context_test.cc

@@ -21,6 +21,7 @@
 #include "gtest/gtest.h"
 #include "util/misc/address_sanitizer.h"
 #include "util/misc/capture_context_test_util.h"
+#include "util/misc/memory_sanitizer.h"
 
 namespace crashpad {
 namespace test {
@@ -33,7 +34,12 @@ namespace {
 // find an approximately valid stack pointer by comparing locals to the
 // captured one, disable safe-stack for this function.
 __attribute__((no_sanitize("safe-stack")))
-#endif
+#endif  // defined(OS_FUCHSIA)
+
+#if defined(MEMORY_SANITIZER)
+// CaptureContext() calls inline assembly and is incompatible with MSan.
+__attribute__((no_sanitize("memory")))
+#endif  // defined(MEMORY_SANITIZER)
 
 void TestCaptureContext() {
   NativeCPUContext context_1;
@@ -49,8 +55,9 @@ void TestCaptureContext() {
   // reference program counter.
   uintptr_t pc = ProgramCounterFromContext(context_1);
 
-#if !defined(ADDRESS_SANITIZER) && !defined(ARCH_CPU_MIPS_FAMILY)
-  // AddressSanitizer can cause enough code bloat that the “nearby” check would
+#if !defined(ADDRESS_SANITIZER) && !defined(ARCH_CPU_MIPS_FAMILY) && \
+    !defined(MEMORY_SANITIZER)
+  // Sanitizers can cause enough code bloat that the “nearby” check would
   // likely fail.
   const uintptr_t kReferencePC =
       reinterpret_cast<uintptr_t>(TestCaptureContext);
@@ -58,7 +65,7 @@ void TestCaptureContext() {
                   uintptr_t reference) { return actual - reference < 128u; },
                pc,
                kReferencePC);
-#endif  // !defined(ADDRESS_SANITIZER)
+#endif
 
   const uintptr_t sp = StackPointerFromContext(context_1);
 
@@ -82,7 +89,7 @@ void TestCaptureContext() {
                   uintptr_t reference) { return reference - actual < 768u; },
                sp,
                kReferenceSP);
-#endif  // !ADDRESS_SANITIZER
+#endif  // !defined(ADDRESS_SANITIZER)
 
   // Capture the context again, expecting that the stack pointer stays the same
   // and the program counter increases. Strictly speaking, there’s no guarantee

util/misc/memory_sanitizer.h

@@ -0,0 +1,27 @@
+// Copyright 2019 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_UTIL_MISC_MEMORY_SANITIZER_H_
+#define CRASHPAD_UTIL_MISC_MEMORY_SANITIZER_H_
+
+#include "base/compiler_specific.h"
+#include "build/build_config.h"
+
+#if !defined(MEMORY_SANITIZER)
+#if HAS_FEATURE(memory_sanitizer)
+#define MEMORY_SANITIZER 1
+#endif  // HAS_FEATURE(memory_sanitizer)
+#endif  // !defined(MEMORY_SANITIZER)
+
+#endif  // CRASHPAD_UTIL_MISC_MEMORY_SANITIZER_H_