Add instrumentation to investigate a possible UAF.

Bug: 901501
Change-Id: Ic778c96c1bb10a9f4bb2f8f3efd883e452bc9742
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1606770
Auto-Submit: Zhongyi Shi <zhongyi@chromium.org>
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658861}

net/spdy/spdy_http_stream.cc

@@ -10,6 +10,9 @@
 #include <utility>
 
 #include "base/bind.h"
+#ifdef TEMP_INSTRUMENTATION_901501
+#include "base/debug/alias.h"
+#endif
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ -128,6 +131,14 @@ SpdyHttpStream::~SpdyHttpStream() {
     stream_->DetachDelegate();
     DCHECK(!stream_);
   }
+#ifdef TEMP_INSTRUMENTATION_901501
+  liveness_ = DEAD;
+  stack_trace_ = base::debug::StackTrace();
+  // Probably not necessary, but just in case compiler tries to optimize out the
+  // writes to liveness_ and stack_trace_.
+  base::debug::Alias(&liveness_);
+  base::debug::Alias(&stack_trace_);
+#endif
 }
 
 int SpdyHttpStream::InitializeStream(const HttpRequestInfo* request_info,
@@ -443,7 +454,8 @@ void SpdyHttpStream::OnDataSent() {
 void SpdyHttpStream::OnTrailers(const spdy::SpdyHeaderBlock& trailers) {}
 
 void SpdyHttpStream::OnClose(int status) {
-  DCHECK(stream_);
+  CHECK(stream_);
+  CrashIfInvalid();
 
   // Cancel any pending reads from the upload data stream.
   if (request_info_ && request_info_->upload_data_stream)
@@ -467,6 +479,8 @@ void SpdyHttpStream::OnClose(int status) {
       return;
   }
 
+  CrashIfInvalid();
+
   if (status == OK) {
     // We need to complete any pending buffered read now.
     DoBufferedReadCallback();
@@ -474,6 +488,8 @@ void SpdyHttpStream::OnClose(int status) {
       return;
   }
 
+  CrashIfInvalid();
+
   if (!response_callback_.is_null()) {
     DoResponseCallback(status);
   }
@@ -590,6 +606,8 @@ bool SpdyHttpStream::ShouldWaitForMoreBufferedData() const {
 }
 
 void SpdyHttpStream::DoBufferedReadCallback() {
+  CrashIfInvalid();
+
   buffered_read_callback_pending_ = false;
 
   // If the transaction is cancelled or errored out, we don't need to complete
@@ -611,6 +629,8 @@ void SpdyHttpStream::DoBufferedReadCallback() {
   if (!user_buffer_.get())
     return;
 
+  CrashIfInvalid();
+
   if (!response_body_queue_.IsEmpty()) {
     int rv =
         response_body_queue_.Dequeue(user_buffer_->data(), user_buffer_len_);
@@ -673,4 +693,22 @@ void SpdyHttpStream::SetPriority(RequestPriority priority) {
   }
 }
 
+void SpdyHttpStream::CrashIfInvalid() const {
+#ifdef TEMP_INSTRUMENTATION_901501
+  Liveness liveness = liveness_;
+
+  if (liveness == ALIVE)
+    return;
+
+  // Copy relevant variables onto the stack to guarantee they will be available
+  // in minidumps, and then crash.
+  base::debug::StackTrace stack_trace = stack_trace_;
+
+  base::debug::Alias(&liveness);
+  base::debug::Alias(&stack_trace);
+
+  CHECK_EQ(ALIVE, liveness);
+#endif
+}
+
 }  // namespace net

net/spdy/spdy_http_stream.h

@@ -10,6 +10,13 @@
 #include <list>
 #include <memory>
 
+#include "build/build_config.h"
+
+// TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
+#ifndef OS_NACL
+#define TEMP_INSTRUMENTATION_901501
+#include "base/debug/stack_trace.h"
+#endif
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ -96,6 +103,14 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
   NetLogSource source_dependency() const override;
 
  private:
+#ifdef TEMP_INSTRUMENTATION_901501
+  // TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
+  enum Liveness {
+    ALIVE = 0xCA11AB13,
+    DEAD = 0xDEADBEEF,
+  };
+#endif
+
   // Helper function used to initialize private members and to set delegate on
   // stream when stream is created.
   void InitializeStreamHelper();
@@ -136,6 +151,9 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
   void DoBufferedReadCallback();
   bool ShouldWaitForMoreBufferedData() const;
 
+  // TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
+  void CrashIfInvalid() const;
+
   const base::WeakPtr<SpdySession> spdy_session_;
 
   // The ID of the pushed stream if one is claimed by this request.
@@ -209,6 +227,12 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
 
   bool was_alpn_negotiated_;
 
+#ifdef TEMP_INSTRUMENTATION_901501
+  // TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
+  Liveness liveness_ = ALIVE;
+  base::debug::StackTrace stack_trace_;
+#endif
+
   base::WeakPtrFactory<SpdyHttpStream> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SpdyHttpStream);