Tag all tracked objects, including Tasks, with the program counter at the site of FROM_HERE.

This is to make it easier to determine the site Tasks are posted from in release builds, especially when only a minidump is available. It should help diagnose http://crbug.com/81499.

I added a debug function to alias variables so that the optimizer will not strip them out if they are not live.

The semantics of the MessageLoop::PostTask functions is changed and it is wrong but I am not sure what semantics are intended. It seems location information was no longer being tracked for Tasks wrapped as Closures and I don't know if this was intended. PTAL. Update: this has since been fixed.

TEST=Set breakpoint in TaskClosureAdapter::Run and very that the post site can be located in an optimized build.
BUG=81499
Review URL: http://codereview.chromium.org/7039020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85991 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 117 insertions, 17 deletions

diff --git a/base/base.gypi b/base/base.gypi
index 25060bb..def5b0cf6 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -49,6 +49,8 @@
           'compiler_specific.h',
           'cpu.cc',
           'cpu.h',
+          'debug/alias.cc',
+          'debug/alias.h',
           'debug/debug_on_start_win.cc',
           'debug/debug_on_start_win.h',
           'debug/debugger.cc',
diff --git a/base/debug/alias.cc b/base/debug/alias.cc
new file mode 100644
index 0000000..04122fb
--- /dev/null
+++ b/base/debug/alias.cc
@@ -0,0 +1,20 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/debug/alias.h"
+#include "build/build_config.h"
+
+namespace base {
+namespace debug {
+
+namespace {
+const void* g_global;
+}
+
+void Alias(const void* var) {
+  g_global = var;
+}
+
+}  // namespace debug
+}  // namespace base
diff --git a/base/debug/alias.h b/base/debug/alias.h
new file mode 100644
index 0000000..957afdb
--- /dev/null
+++ b/base/debug/alias.h
@@ -0,0 +1,20 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_DEBUG_ALIAS_H_
+#define BASE_DEBUG_ALIAS_H_
+#pragma once
+
+namespace base {
+namespace debug {
+
+// Make the optimizer think that var is aliased. This is to prevent it from
+// optimizing out variables that that would not otherwise be live at the point
+// of a potential crash.
+void Alias(const void* var);
+
+}  // namespace debug
+}  // namespace base
+
+#endif  // BASE_DEBUG_ALIAS_H_
diff --git a/base/message_loop.cc b/base/message_loop.cc
index 059cdc25..e6b49ac8 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/debug/alias.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/message_pump_default.h"
@@ -467,6 +468,14 @@ void MessageLoop::RunTask(const PendingTask& pending_task) {
   // Execute the task and assume the worst: It is probably not reentrant.
   nestable_tasks_allowed_ = false;
 
+  // Before running the task, store the program counter where it was posted
+  // and deliberately alias it to ensure it is on the stack if the task
+  // crashes. Be careful not to assume that the variable itself will have the
+  // expected value when displayed by the optimizer in an optimized build.
+  // Look at a memory dump of the stack.
+  const void* program_counter = pending_task.birth_program_counter;
+  base::debug::Alias(&program_counter);
+
   HistogramEvent(kTaskRunEvent);
   FOR_EACH_OBSERVER(TaskObserver, task_observers_,
                     WillProcessTask(pending_task.time_posted));
@@ -766,7 +775,8 @@ MessageLoop::PendingTask::PendingTask(
       time_posted(TimeTicks::Now()),
       delayed_run_time(delayed_run_time),
       sequence_num(0),
-      nestable(nestable) {
+      nestable(nestable),
+      birth_program_counter(posted_from.program_counter()) {
 #if defined(TRACK_ALL_TASK_OBJECTS)
   if (tracked_objects::ThreadData::IsActive()) {
     tracked_objects::ThreadData* current_thread_data =
diff --git a/base/message_loop.h b/base/message_loop.h
index c747947..c660c9e 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -415,6 +415,9 @@ class BASE_API MessageLoop : public base::MessagePump::Delegate {
 
     // OK to dispatch from a nested loop.
     bool nestable;
+
+    // The site this PendingTask was posted from.
+    const void* birth_program_counter;
   };
 
   class TaskQueue : public std::queue<PendingTask> {
diff --git a/base/tracked.cc b/base/tracked.cc
index 767f072..9a135f5 100644
--- a/base/tracked.cc
+++ b/base/tracked.cc
@@ -1,7 +1,13 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "build/build_config.h"
+
+#if defined(COMPILER_MSVC)
+#include <intrin.h>
+#endif
+
 #include "base/tracked.h"
 
 #include "base/stringprintf.h"
@@ -13,17 +19,21 @@ namespace tracked_objects {
 
 //------------------------------------------------------------------------------
 
-Location::Location(const char* function_name, const char* file_name,
-                   int line_number)
+Location::Location(const char* function_name,
+                   const char* file_name,
+                   int line_number,
+                   const void* program_counter)
     : function_name_(function_name),
       file_name_(file_name),
-      line_number_(line_number) {
+      line_number_(line_number),
+      program_counter_(program_counter) {
 }
 
 Location::Location()
     : function_name_("Unknown"),
       file_name_("Unknown"),
-      line_number_(-1) {
+      line_number_(-1),
+      program_counter_(NULL) {
 }
 
 void Location::Write(bool display_filename, bool display_function_name,
@@ -58,15 +68,29 @@ void Location::WriteFunctionName(std::string* output) const {
   }
 }
 
+BASE_API const void* GetProgramCounter() {
+#if defined(COMPILER_MSVC)
+  return _ReturnAddress();
+#elif defined(COMPILER_GCC)
+  return __builtin_extract_return_addr(__builtin_return_address(0));
+#endif  // COMPILER_GCC
+
+  return NULL;
+}
+
 //------------------------------------------------------------------------------
 
 #ifndef TRACK_ALL_TASK_OBJECTS
 
-Tracked::Tracked() {}
+Tracked::Tracked() : birth_program_counter_(NULL) {}
 Tracked::~Tracked() {}
-void Tracked::SetBirthPlace(const Location& from_here) {}
+
+void Tracked::SetBirthPlace(const Location& from_here) {
+  birth_program_counter_ = from_here.program_counter();
+}
+
 const Location Tracked::GetBirthPlace() const {
-  static Location kNone("NoFunctionName", "NeedToSetBirthPlace", -1);
+  static Location kNone("NoFunctionName", "NeedToSetBirthPlace", -1, NULL);
   return kNone;
 }
 bool Tracked::MissingBirthplace() const { return false; }
@@ -79,7 +103,7 @@ Tracked::Tracked()
       tracked_birth_time_(TimeTicks::Now()) {
   if (!ThreadData::IsActive())
     return;
-  SetBirthPlace(Location("NoFunctionName", "NeedToSetBirthPlace", -1));
+  SetBirthPlace(Location("NoFunctionName", "NeedToSetBirthPlace", -1, NULL));
 }
 
 Tracked::~Tracked() {
@@ -98,6 +122,8 @@ void Tracked::SetBirthPlace(const Location& from_here) {
   if (!current_thread_data)
     return;  // Shutdown started, and this thread wasn't registered.
   tracked_births_ = current_thread_data->TallyABirth(from_here);
+
+  birth_program_counter_ = from_here.program_counter();
 }
 
 const Location Tracked::GetBirthPlace() const {
diff --git a/base/tracked.h b/base/tracked.h
index 45776d1..777e604 100644
--- a/base/tracked.h
+++ b/base/tracked.h
@@ -41,7 +41,10 @@ class BASE_API Location {
   // Constructor should be called with a long-lived char*, such as __FILE__.
   // It assumes the provided value will persist as a global constant, and it
   // will not make a copy of it.
-  Location(const char* function_name, const char* file_name, int line_number);
+  Location(const char* function_name,
+           const char* file_name,
+           int line_number,
+           const void* program_counter);
 
   // Provide a default constructor for easy of debugging.
   Location();
@@ -60,9 +63,10 @@ class BASE_API Location {
     return function_name_ < other.function_name_;
   }
 
-  const char* function_name() const { return function_name_; }
-  const char* file_name()     const { return file_name_; }
-  int line_number()           const { return line_number_; }
+  const char* function_name()   const { return function_name_; }
+  const char* file_name()       const { return file_name_; }
+  int line_number()             const { return line_number_; }
+  const void* program_counter() const { return program_counter_; }
 
   void Write(bool display_filename, bool display_function_name,
              std::string* output) const;
@@ -74,13 +78,19 @@ class BASE_API Location {
   const char* const function_name_;
   const char* const file_name_;
   const int line_number_;
+  const void* const program_counter_;
 };
 
+BASE_API const void* GetProgramCounter();
 
 //------------------------------------------------------------------------------
 // Define a macro to record the current source location.
 
-#define FROM_HERE tracked_objects::Location(__FUNCTION__, __FILE__, __LINE__)
+#define FROM_HERE tracked_objects::Location(                                   \
+    __FUNCTION__,                                                              \
+    __FILE__,                                                                  \
+    __LINE__,                                                                  \
+    tracked_objects::GetProgramCounter())                                      \
 
 
 //------------------------------------------------------------------------------
@@ -110,6 +120,11 @@ class BASE_API Tracked {
   base::TimeTicks tracked_birth_time() const { return base::TimeTicks::Now(); }
 #endif  // defined(TRACK_ALL_TASK_OBJECTS)
 
+  // Returns null if SetBirthPlace has not been called.
+  const void* get_birth_program_counter() const {
+    return birth_program_counter_;
+  }
+
  private:
 #if defined(TRACK_ALL_TASK_OBJECTS)
 
@@ -123,6 +138,8 @@ class BASE_API Tracked {
 
 #endif  // defined(TRACK_ALL_TASK_OBJECTS)
 
+  const void* birth_program_counter_;
+
   DISALLOW_COPY_AND_ASSIGN(Tracked);
 };
 
diff --git a/chrome/browser/sync/glue/data_type_manager.h b/chrome/browser/sync/glue/data_type_manager.h
index c621b49..7d789d4 100644
--- a/chrome/browser/sync/glue/data_type_manager.h
+++ b/chrome/browser/sync/glue/data_type_manager.h
@@ -66,7 +66,8 @@ class DataTypeManager {
       this->location.reset(new tracked_objects::Location(
           location.function_name(),
           location.file_name(),
-          location.line_number()));
+          location.line_number(),
+          location.program_counter()));
     }
 
       ~ConfigureResultWithErrorLocation();
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 4c7fbf2..30980a7 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -505,7 +505,8 @@ void ProfileSyncService::OnUnrecoverableError(
   unrecoverable_error_location_.reset(
       new tracked_objects::Location(from_here.function_name(),
                                     from_here.file_name(),
-                                    from_here.line_number()));
+                                    from_here.line_number(),
+                                    from_here.program_counter()));
 
   // Tell the wizard so it can inform the user only if it is already open.
   wizard_.Step(SyncSetupWizard::FATAL_ERROR);