From 52d766400da164e2f6f2debc6bd94a568ec64cad Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Wed, 25 Oct 2017 14:26:29 -0400
Subject: [PATCH] linux: ProcessReader can own ProcessMemoryLinux without
 unique_ptr
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There’s no reason for ProcessReader to own its ProcessMemoryLinux via
std::unique_ptr<>.

This was discovered in a trunk Clang build, during which a
-Wdelete-non-virtual-dtor warning was produced (since Clang r312167).
The warning is not produced by earlier Clang versions or by GCC because
the “delete” happens in a system header, <memory>, when performed by
std::unique_ptr<>. Although ownership via std::unique_ptr<> is no longer
used, ProcessMemoryLinux is marked “final” because it ought to be.

In file included from ../../snapshot/linux/process_reader.cc:15:
In file included from ../../snapshot/linux/process_reader.h:21:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/7.2.0/../../include/c++/7.2.0/memory:80:
/usr/bin/../lib/gcc/x86_64-linux-gnu/7.2.0/../../include/c++/7.2.0/bits/unique_ptr.h:78:2: error: delete called on non-final 'crashpad::ProcessMemoryLinux' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non-virtual-dtor]
        delete __ptr;
        ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/7.2.0/../../include/c++/7.2.0/bits/unique_ptr.h:268:4: note: in instantiation of member function 'std::default_delete<crashpad::ProcessMemoryLinux>::operator()' requested here
          get_deleter()(__ptr);
          ^
../../snapshot/linux/process_reader.cc:169:16: note: in instantiation of member function 'std::unique_ptr<crashpad::ProcessMemoryLinux, std::default_delete<crashpad::ProcessMemoryLinux> >::~unique_ptr' requested here
ProcessReader::ProcessReader()
               ^
1 error generated.
Change-Id: Ibe9671db429262aca12bbfdf457c8f72cad2f358
Reviewed-on: https://chromium-review.googlesource.com/738530
Reviewed-by: Dave Bort <dbort@google.com>
Commit-Queue: Mark Mentovai <mark@chromium.org>
---
 snapshot/linux/process_reader.cc      | 3 +--
 snapshot/linux/process_reader.h       | 7 +++----
 snapshot/linux/process_reader_test.cc | 1 +
 util/process/process_memory_linux.h   | 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/snapshot/linux/process_reader.cc b/snapshot/linux/process_reader.cc
index 904fdced..c9a5a382 100644
--- a/snapshot/linux/process_reader.cc
+++ b/snapshot/linux/process_reader.cc
@@ -192,8 +192,7 @@ bool ProcessReader::Initialize(PtraceConnection* connection) {
     return false;
   }
 
-  process_memory_.reset(new ProcessMemoryLinux());
-  if (!process_memory_->Initialize(pid)) {
+  if (!process_memory_.Initialize(pid)) {
     return false;
   }
 
diff --git a/snapshot/linux/process_reader.h b/snapshot/linux/process_reader.h
index 85708383..26d777e4 100644
--- a/snapshot/linux/process_reader.h
+++ b/snapshot/linux/process_reader.h
@@ -18,7 +18,6 @@
 #include <sys/time.h>
 #include <sys/types.h>
 
-#include <memory>
 #include <vector>
 
 #include "base/macros.h"
@@ -79,7 +78,7 @@ class ProcessReader {
   pid_t ParentProcessID() const { return process_info_.ParentProcessID(); }
 
   //! \brief Return a memory reader for the target process.
-  ProcessMemory* Memory() { return process_memory_.get(); }
+  ProcessMemory* Memory() { return &process_memory_; }
 
   //! \brief Return a memory map of the target process.
   MemoryMap* GetMemoryMap() { return &memory_map_; }
@@ -113,9 +112,9 @@ class ProcessReader {
 
   PtraceConnection* connection_;  // weak
   ProcessInfo process_info_;
-  class MemoryMap memory_map_;
+  MemoryMap memory_map_;
   std::vector<Thread> threads_;
-  std::unique_ptr<ProcessMemoryLinux> process_memory_;
+  ProcessMemoryLinux process_memory_;
   bool is_64_bit_;
   bool initialized_threads_;
   InitializationStateDcheck initialized_;
diff --git a/snapshot/linux/process_reader_test.cc b/snapshot/linux/process_reader_test.cc
index 5c8b8cba..08edf02a 100644
--- a/snapshot/linux/process_reader_test.cc
+++ b/snapshot/linux/process_reader_test.cc
@@ -24,6 +24,7 @@
 #include <unistd.h>
 
 #include <map>
+#include <memory>
 #include <string>
 #include <utility>
 
diff --git a/util/process/process_memory_linux.h b/util/process/process_memory_linux.h
index 95e93f5a..e03e251f 100644
--- a/util/process/process_memory_linux.h
+++ b/util/process/process_memory_linux.h
@@ -27,7 +27,7 @@
 namespace crashpad {
 
 //! \brief Accesses the memory of another Linux process.
-class ProcessMemoryLinux : public ProcessMemory {
+class ProcessMemoryLinux final : public ProcessMemory {
  public:
   ProcessMemoryLinux();
   ~ProcessMemoryLinux();