gpu: Add message filter destructor stack traces to crash keys.

base::debug::Alias values don't show up in minidumps. Using crash keys
is recommended for use-after-free bugs in the following doc:
http://dev.chromium.org/developers/debugging-with-crash-keys

R=jbauman,rsesek,sandersd
BUG=729483

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I8fd129daf66dc9235307a28fe5cd7a6c23b85a94
Reviewed-on: https://chromium-review.googlesource.com/538056Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Bauman <jbauman@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480879}

chrome/common/crash_keys.cc

@@ -112,6 +112,8 @@ size_t RegisterChromeCrashKeys() {
     {kApValue, kSmallSize},
     {kCohortName, kSmallSize},
 #endif  // defined(OS_WIN)
+
+// gpu
 #if !defined(OS_ANDROID)
     {gpu::crash_keys::kGPUVendorID, kSmallSize},
     {gpu::crash_keys::kGPUDeviceID, kSmallSize},
@@ -125,6 +127,10 @@ size_t RegisterChromeCrashKeys() {
     {gpu::crash_keys::kGPUVendor, kSmallSize},
     {gpu::crash_keys::kGPURenderer, kSmallSize},
 #endif
+    // Temporary for https://crbug.com/729483.
+    // TODO(sunnyps): Remove after https://crbug.com/729483 is fixed.
+    {gpu::crash_keys::kGpuChannelFilterTrace, crash_keys::kMediumSize},
+    {gpu::crash_keys::kMediaGpuChannelFilterTrace, crash_keys::kMediumSize},
 
     // content/:
     {"bad_message_reason", kSmallSize},
@@ -169,7 +175,7 @@ size_t RegisterChromeCrashKeys() {
     {"channel_error_bt", kMediumSize},
     {"remove_route_bt", kMediumSize},
     {"rwhvm_window", kMediumSize},
-    // media/:
+// media/:
 #endif
     {kBug464926CrashKey, kSmallSize},
     {kViewCount, kSmallSize},

gpu/config/gpu_crash_keys.cc

@@ -19,5 +19,10 @@ const char kGPUGLVersion[] = "gpu-glver";
 const char kGPUVendor[] = "gpu-gl-vendor";
 const char kGPURenderer[] = "gpu-gl-renderer";
 #endif
+
+// TODO(sunnyps): Remove after https://crbug.com/729483 is fixed.
+const char kGpuChannelFilterTrace[] = "gpu-channel-filter-trace";
+const char kMediaGpuChannelFilterTrace[] = "media-gpu-channel-filter-trace";
+
 }  // namespace crash_keys
 }  // namespace gpu

gpu/config/gpu_crash_keys.h

@@ -24,6 +24,12 @@ extern const char kGPUGLVersion[];
 extern const char kGPUVendor[];
 extern const char kGPURenderer[];
 #endif
+
+// TEMPORARY: Backtraces for gpu message filters to fix use-after-free.
+// TODO(sunnyps): Remove after https://crbug.com/729483 is fixed.
+extern const char kGpuChannelFilterTrace[];
+extern const char kMediaGpuChannelFilterTrace[];
+
 }  // namespace crash_keys
 }  // namespace gpu
 

gpu/ipc/service/BUILD.gn

@@ -74,6 +74,7 @@ target(link_target_type, "ipc_service_sources") {
     "//gpu/command_buffer/common:common_sources",
     "//gpu/command_buffer/service:service_sources",
     "//gpu/config:config_sources",
+    "//gpu/config:crash_keys",
     "//gpu/ipc/common:ipc_common_sources",
   ]
   libs = []

gpu/ipc/service/gpu_channel.cc

@@ -18,7 +18,8 @@
 #include "base/atomicops.h"
 #include "base/bind.h"
 #include "base/command_line.h"
-#include "base/debug/alias.h"
+#include "base/debug/crash_logging.h"
+#include "base/debug/stack_trace.h"
 #include "base/location.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/single_thread_task_runner.h"
@@ -37,6 +38,7 @@
 #include "gpu/command_buffer/service/mailbox_manager.h"
 #include "gpu/command_buffer/service/preemption_flag.h"
 #include "gpu/command_buffer/service/scheduler.h"
+#include "gpu/config/gpu_crash_keys.h"
 #include "gpu/ipc/common/gpu_messages.h"
 #include "gpu/ipc/service/gpu_channel_manager.h"
 #include "gpu/ipc/service/gpu_channel_manager_delegate.h"
@@ -588,7 +590,10 @@ GpuChannelMessageFilter::GpuChannelMessageFilter(
 }
 
 GpuChannelMessageFilter::~GpuChannelMessageFilter() {
-  DCHECK(!gpu_channel_);
+  // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
+  base::debug::SetCrashKeyToStackTrace(gpu::crash_keys::kGpuChannelFilterTrace,
+                                       base::debug::StackTrace());
+  CHECK(!gpu_channel_);
 }
 
 void GpuChannelMessageFilter::Destroy() {
@@ -614,8 +619,6 @@ void GpuChannelMessageFilter::RemoveRoute(int32_t route_id) {
 void GpuChannelMessageFilter::OnFilterAdded(IPC::Channel* channel) {
   // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
   CHECK(io_thread_checker_.CalledOnValidThread());
-  GpuChannelMessageFilter* alias_this = this;
-  base::debug::Alias(&alias_this);
 
   DCHECK(!ipc_channel_);
   ipc_channel_ = channel;
@@ -626,8 +629,6 @@ void GpuChannelMessageFilter::OnFilterAdded(IPC::Channel* channel) {
 void GpuChannelMessageFilter::OnFilterRemoved() {
   // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
   CHECK(io_thread_checker_.CalledOnValidThread());
-  GpuChannelMessageFilter* alias_this = this;
-  base::debug::Alias(&alias_this);
 
   for (scoped_refptr<IPC::MessageFilter>& filter : channel_filters_)
     filter->OnFilterRemoved();
@@ -638,8 +639,6 @@ void GpuChannelMessageFilter::OnFilterRemoved() {
 void GpuChannelMessageFilter::OnChannelConnected(int32_t peer_pid) {
   // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
   CHECK(io_thread_checker_.CalledOnValidThread());
-  GpuChannelMessageFilter* alias_this = this;
-  base::debug::Alias(&alias_this);
 
   DCHECK(peer_pid_ == base::kNullProcessId);
   peer_pid_ = peer_pid;
@@ -650,8 +649,6 @@ void GpuChannelMessageFilter::OnChannelConnected(int32_t peer_pid) {
 void GpuChannelMessageFilter::OnChannelError() {
   // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
   CHECK(io_thread_checker_.CalledOnValidThread());
-  GpuChannelMessageFilter* alias_this = this;
-  base::debug::Alias(&alias_this);
 
   for (scoped_refptr<IPC::MessageFilter>& filter : channel_filters_)
     filter->OnChannelError();
@@ -660,8 +657,6 @@ void GpuChannelMessageFilter::OnChannelError() {
 void GpuChannelMessageFilter::OnChannelClosing() {
   // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
   CHECK(io_thread_checker_.CalledOnValidThread());
-  GpuChannelMessageFilter* alias_this = this;
-  base::debug::Alias(&alias_this);
 
   for (scoped_refptr<IPC::MessageFilter>& filter : channel_filters_)
     filter->OnChannelClosing();
@@ -671,8 +666,6 @@ void GpuChannelMessageFilter::AddChannelFilter(
     scoped_refptr<IPC::MessageFilter> filter) {
   // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
   CHECK(io_thread_checker_.CalledOnValidThread());
-  GpuChannelMessageFilter* alias_this = this;
-  base::debug::Alias(&alias_this);
 
   channel_filters_.push_back(filter);
   if (ipc_channel_)
@@ -685,8 +678,6 @@ void GpuChannelMessageFilter::RemoveChannelFilter(
     scoped_refptr<IPC::MessageFilter> filter) {
   // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
   CHECK(io_thread_checker_.CalledOnValidThread());
-  GpuChannelMessageFilter* alias_this = this;
-  base::debug::Alias(&alias_this);
 
   if (ipc_channel_)
     filter->OnFilterRemoved();

media/gpu/ipc/service/BUILD.gn

@@ -38,6 +38,7 @@ target(link_target_type, "service") {
     "//media/gpu",
   ]
   deps = [
+    "//gpu/config:crash_keys",
     "//gpu/ipc/service",
     "//media:media_features",
     "//media/gpu/ipc/common",

media/gpu/ipc/service/media_gpu_channel.cc

@@ -4,8 +4,11 @@
 
 #include "media/gpu/ipc/service/media_gpu_channel.h"
 
+#include "base/debug/crash_logging.h"
+#include "base/debug/stack_trace.h"
 #include "base/single_thread_task_runner.h"
 #include "base/unguessable_token.h"
+#include "gpu/config/gpu_crash_keys.h"
 #include "gpu/ipc/service/gpu_channel.h"
 #include "ipc/message_filter.h"
 #include "media/gpu/ipc/common/media_messages.h"
@@ -72,7 +75,12 @@ class MediaGpuChannelFilter : public IPC::MessageFilter {
   }
 
  private:
-  ~MediaGpuChannelFilter() override {}
+  ~MediaGpuChannelFilter() override {
+    // TODO(sunnyps): Remove once crbug.com/729483 has been resolved.
+    base::debug::SetCrashKeyToStackTrace(
+        gpu::crash_keys::kMediaGpuChannelFilterTrace,
+        base::debug::StackTrace());
+  }
 
   IPC::Channel* channel_;
   base::UnguessableToken channel_token_;