Migrate TaskAnnotator IPC decoration to use constexpr hashes.

This vastly simplifies the backend pipeline needed to symbolize IPC
messages, requiring a simple lookup table rather than symbolization
followed by source code parsing. It also means the raw IPC message
IDs are constant across Chrome versions/builds/platforms, which
facilitates aggregation.

The impact on binary size is roughly a nop; on some platforms slightly
better, on some slightly worse. The overall impact is still about
the same.

BUG=950668

Change-Id: I13661059db79c1b091afdf68daed14325a18f26a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1615386Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664578}

base/hash/md5_constexpr.h

@@ -5,8 +5,6 @@
 #ifndef BASE_HASH_MD5_CONSTEXPR_H_
 #define BASE_HASH_MD5_CONSTEXPR_H_
 
-#include <array>
-
 #include "base/hash/md5.h"
 #include "base/hash/md5_constexpr_internal.h"
 
@@ -25,11 +23,21 @@ namespace base {
 constexpr MD5Digest MD5SumConstexpr(const char* string);
 constexpr MD5Digest MD5SumConstexpr(const char* data, uint32_t length);
 
-// Calculates the first 64 bits of the MD5 digest of the provided data, returned
-// as a uint64_t. When passing |string| with no explicit length the terminating
-// null will not be processed.
-constexpr uint64_t MD5HashConstexpr(const char* string);
-constexpr uint64_t MD5HashConstexpr(const char* data, uint32_t length);
+// Calculates the first 32/64 bits of the MD5 digest of the provided data,
+// returned as a uint32_t/uint64_t. When passing |string| with no explicit
+// length the terminating null will not be processed. This abstracts away
+// endianness so that the integer will read as the first 4 or 8 bytes of the
+// MD5 sum, ensuring that the following outputs are equivalent for
+// convenience:
+//
+// printf("%08x\n", MD5HashConstexpr32("foo"));
+//
+// MD5Digest d = MD5SumConstexpr("foo");
+// printf("%02x%02x%02x%02x\n", d.a[0], d.a[1], d.a[2], d.a[3]);
+constexpr uint64_t MD5Hash64Constexpr(const char* string);
+constexpr uint64_t MD5Hash64Constexpr(const char* data, uint32_t length);
+constexpr uint32_t MD5Hash32Constexpr(const char* string);
+constexpr uint32_t MD5Hash32Constexpr(const char* data, uint32_t length);
 
 }  // namespace base
 

base/hash/md5_constexpr_internal.h

@@ -6,6 +6,7 @@
 #define BASE_HASH_MD5_CONSTEXPR_INTERNAL_H_
 
 #include <array>
+#include <cstddef>
 #include <cstdint>
 
 #include "base/hash/md5.h"
@@ -239,7 +240,7 @@ struct MD5CE {
   // Converts an IntermediateData to a final digest.
   static constexpr MD5Digest IntermediateDataToMD5Digest(
       const IntermediateData& intermediate) {
-    return MD5Digest{static_cast<uint8_t>((intermediate.a >> 0) & 0xff),
+    return MD5Digest{{static_cast<uint8_t>((intermediate.a >> 0) & 0xff),
                       static_cast<uint8_t>((intermediate.a >> 8) & 0xff),
                       static_cast<uint8_t>((intermediate.a >> 16) & 0xff),
                       static_cast<uint8_t>((intermediate.a >> 24) & 0xff),
@@ -254,14 +255,23 @@ struct MD5CE {
                       static_cast<uint8_t>((intermediate.d >> 0) & 0xff),
                       static_cast<uint8_t>((intermediate.d >> 8) & 0xff),
                       static_cast<uint8_t>((intermediate.d >> 16) & 0xff),
-                     static_cast<uint8_t>((intermediate.d >> 24) & 0xff)};
+                      static_cast<uint8_t>((intermediate.d >> 24) & 0xff)}};
   }
 
   static constexpr uint32_t StringLength(const char* string) {
     const char* end = string;
     while (*end != 0)
       ++end;
-    return end - string;
+    // Double check that the precision losing conversion is safe.
+    DCHECK(end >= string);
+    DCHECK(static_cast<std::ptrdiff_t>(static_cast<uint32_t>(end - string)) ==
+           (end - string));
+    return static_cast<uint32_t>(end - string);
+  }
+
+  static constexpr uint32_t SwapEndian(uint32_t a) {
+    return ((a & 0xff) << 24) | (((a >> 8) & 0xff) << 16) |
+           (((a >> 16) & 0xff) << 8) | ((a >> 24) & 0xff);
   }
 
   //////////////////////////////////////////////////////////////////////////////
@@ -271,10 +281,15 @@ struct MD5CE {
     return IntermediateDataToMD5Digest(ProcessMessage(data, n));
   }
 
-  static constexpr uint64_t Hash(const char* data, uint32_t n) {
+  static constexpr uint64_t Hash64(const char* data, uint32_t n) {
+    IntermediateData intermediate = ProcessMessage(data, n);
+    return (static_cast<uint64_t>(SwapEndian(intermediate.a)) << 32) |
+           static_cast<uint64_t>(SwapEndian(intermediate.b));
+  }
+
+  static constexpr uint32_t Hash32(const char* data, uint32_t n) {
     IntermediateData intermediate = ProcessMessage(data, n);
-    return (static_cast<uint64_t>(intermediate.b) << 32) |
-           static_cast<uint64_t>(intermediate.a);
+    return SwapEndian(intermediate.a);
   }
 };
 
@@ -290,12 +305,20 @@ constexpr MD5Digest MD5SumConstexpr(const char* string, uint32_t length) {
   return internal::MD5CE::Sum(string, length);
 }
 
-constexpr uint64_t MD5HashConstexpr(const char* string) {
-  return internal::MD5CE::Hash(string, internal::MD5CE::StringLength(string));
+constexpr uint64_t MD5Hash64Constexpr(const char* string) {
+  return internal::MD5CE::Hash64(string, internal::MD5CE::StringLength(string));
+}
+
+constexpr uint64_t MD5Hash64Constexpr(const char* string, uint32_t length) {
+  return internal::MD5CE::Hash64(string, length);
+}
+
+constexpr uint32_t MD5Hash32Constexpr(const char* string) {
+  return internal::MD5CE::Hash32(string, internal::MD5CE::StringLength(string));
 }
 
-constexpr uint64_t MD5HashConstexpr(const char* string, uint32_t length) {
-  return internal::MD5CE::Hash(string, length);
+constexpr uint32_t MD5Hash32Constexpr(const char* string, uint32_t length) {
+  return internal::MD5CE::Hash32(string, length);
 }
 
 }  // namespace base

base/hash/md5_constexpr_unittest.cc

@@ -36,8 +36,11 @@ static_assert(Equal(MD5SumConstexpr(kMessage0),
                               0x52, 0x5A, 0x2F, 0x31, 0xAA, 0xF1, 0x61, 0xD0}),
               "incorrect MD5Sum implementation");
 
-static_assert(MD5HashConstexpr(kMessage0) == 0x8D93B77C7D696BF9ull,
-              "incorrect MD5Hash implementation");
+static_assert(MD5Hash64Constexpr(kMessage0) == 0xF96B697D7CB7938Dull,
+              "incorrect MD5Hash64 implementation");
+
+static_assert(MD5Hash32Constexpr(kMessage0) == 0xF96B697Dul,
+              "incorrect MD5Hash32 implementation");
 
 constexpr char kMessage1[] = "The quick brown fox jumps over the lazy dog";
 static_assert(Equal(MD5SumConstexpr(kMessage1, base::size(kMessage1) - 1),
@@ -45,9 +48,13 @@ static_assert(Equal(MD5SumConstexpr(kMessage1, base::size(kMessage1) - 1),
                               0x6b, 0xd8, 0x1d, 0x35, 0x42, 0xa4, 0x19, 0xd6}),
               "incorrect MD5Sum implementation");
 
-static_assert(MD5HashConstexpr(kMessage1, base::size(kMessage1) - 1) ==
-                  0x82b62b379d7d109eull,
-              "incorrect MD5Hash implementation");
+static_assert(MD5Hash64Constexpr(kMessage1, base::size(kMessage1) - 1) ==
+                  0x9E107D9D372BB682ull,
+              "incorrect MD5Hash64 implementation");
+
+static_assert(MD5Hash32Constexpr(kMessage1, base::size(kMessage1) - 1) ==
+                  0x9E107D9Dul,
+              "incorrect MD5Hash32 implementation");
 
 // Comparison operator for checking that the constexpr MD5 implementation
 // matches the default implementation.

base/logging.cc

@@ -617,13 +617,12 @@ LogMessage::~LogMessage() {
     if (!task_trace.empty())
       task_trace.OutputToStream(&stream_);
 
-    // Include the IPC context, if any. This is output as a stack trace with a
-    // single frame.
+    // Include the IPC context, if any.
+    // TODO(chrisha): Integrate with symbolization once those tools exist!
     const auto* task = base::TaskAnnotator::CurrentTaskForThread();
-    if (task && task->ipc_program_counter) {
-      stream_ << "IPC message handler context:" << std::endl;
-      base::debug::StackTrace ipc_trace(&task->ipc_program_counter, 1);
-      ipc_trace.OutputToStream(&stream_);
+    if (task && task->ipc_hash) {
+      stream_ << "IPC message handler context: "
+              << base::StringPrintf("0x%08X", task->ipc_hash) << std::endl;
     }
   }
 #endif

base/pending_task.h

@@ -59,14 +59,14 @@ struct BASE_EXPORT PendingTask {
   std::array<const void*, kTaskBacktraceLength> task_backtrace = {};
 
   // The context of the IPC message that was being handled when this task was
-  // posted. This is a program counter that is set within the scope of an IPC
-  // handler and when symbolized uniquely identifies the message being
+  // posted. This is a hash of the IPC message name that is set within the scope
+  // of an IPC handler and when symbolized uniquely identifies the message being
   // processed. This property is also propagated from one PendingTask to the
   // next. For example, if pending task A was posted while handling an IPC,
   // and pending task B was posted from within pending task A, then pending task
-  // B will inherit the |ipc_program_counter| of pending task A. In some sense
-  // this can be interpreted as a "root" task backtrace frame.
-  const void* ipc_program_counter = nullptr;
+  // B will inherit the |ipc_hash| of pending task A. In some sense this can be
+  // interpreted as a "root" task backtrace frame.
+  uint32_t ipc_hash = 0;
 
   // Secondary sort key for run time.
   int sequence_num = 0;

base/task/common/task_annotator.cc

@@ -31,7 +31,7 @@ ThreadLocalPointer<PendingTask>* GetTLSForCurrentPendingTask() {
 }
 
 // Determines whether or not the given |task| is a dummy pending task that has
-// been injected by ScopedSetIpcProgramCounter solely for the purposes of
+// been injected by ScopedSetIpcHash solely for the purposes of
 // tracking IPC context.
 bool IsDummyPendingTask(const PendingTask* task) {
   if (task->sequence_num == kSentinelSequenceNum &&
@@ -78,7 +78,7 @@ void TaskAnnotator::WillQueueTask(const char* trace_event_name,
   if (!parent_task)
     return;
 
-  pending_task->ipc_program_counter = parent_task->ipc_program_counter;
+  pending_task->ipc_hash = parent_task->ipc_hash;
   pending_task->task_backtrace[0] = parent_task->posted_from.program_counter();
   std::copy(parent_task->task_backtrace.begin(),
             parent_task->task_backtrace.end() - 1,
@@ -96,8 +96,7 @@ void TaskAnnotator::RunTask(const char* trace_event_name,
   debug::ScopedTaskRunActivity task_activity(*pending_task);
 
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("toplevel.ipc"),
-               "TaskAnnotator::RunTask", "ipc_program_counter",
-               pending_task->ipc_program_counter);
+               "TaskAnnotator::RunTask", "ipc_hash", pending_task->ipc_hash);
 
   TRACE_EVENT_WITH_FLOW0(
       TRACE_DISABLED_BY_DEFAULT("toplevel.flow"), trace_event_name,
@@ -115,9 +114,9 @@ void TaskAnnotator::RunTask(const char* trace_event_name,
   // Store a marker to locate |task_backtrace| content easily on a memory
   // dump. The layout is as follows:
   //
-  // +------------ +----+---------+-----+-----------+--------+-------------+
-  // | Head Marker | PC | frame 0 | ... | frame N-1 | IPC PC | Tail Marker |
-  // +------------ +----+---------+-----+-----------+--------+-------------+
+  // +------------ +----+---------+-----+-----------+----------+-------------+
+  // | Head Marker | PC | frame 0 | ... | frame N-1 | IPC hash | Tail Marker |
+  // +------------ +----+---------+-----+-----------+----------+-------------+
   //
   // Markers glossary (compliments of wez):
   //      cool code,do it dude!
@@ -131,7 +130,7 @@ void TaskAnnotator::RunTask(const char* trace_event_name,
   std::copy(pending_task->task_backtrace.begin(),
             pending_task->task_backtrace.end(), task_backtrace.begin() + 2);
   task_backtrace[kStackTaskTraceSnapshotSize - 2] =
-      pending_task->ipc_program_counter;
+      reinterpret_cast<void*>(pending_task->ipc_hash);
   debug::Alias(&task_backtrace);
 
   auto* tls = GetTLSForCurrentPendingTask();
@@ -162,8 +161,7 @@ void TaskAnnotator::ClearObserverForTesting() {
   g_task_annotator_observer = nullptr;
 }
 
-TaskAnnotator::ScopedSetIpcProgramCounter::ScopedSetIpcProgramCounter(
-    const void* program_counter) {
+TaskAnnotator::ScopedSetIpcHash::ScopedSetIpcHash(uint32_t ipc_hash) {
   // We store the IPC context in the currently running task. If there is none
   // then introduce a dummy task.
   auto* tls = GetTLSForCurrentPendingTask();
@@ -175,18 +173,18 @@ TaskAnnotator::ScopedSetIpcProgramCounter::ScopedSetIpcProgramCounter(
     tls->Set(current_task);
   }
 
-  old_ipc_program_counter_ = current_task->ipc_program_counter;
-  current_task->ipc_program_counter = program_counter;
+  old_ipc_hash_ = current_task->ipc_hash;
+  current_task->ipc_hash = ipc_hash;
 }
 
-TaskAnnotator::ScopedSetIpcProgramCounter::~ScopedSetIpcProgramCounter() {
+TaskAnnotator::ScopedSetIpcHash::~ScopedSetIpcHash() {
   auto* tls = GetTLSForCurrentPendingTask();
   auto* current_task = tls->Get();
   DCHECK(current_task);
   if (current_task == dummy_pending_task_.get()) {
     tls->Set(nullptr);
   } else {
-    current_task->ipc_program_counter = old_ipc_program_counter_;
+    current_task->ipc_hash = old_ipc_hash_;
   }
 }
 

base/task/common/task_annotator.h

@@ -27,9 +27,9 @@ class BASE_EXPORT TaskAnnotator {
     virtual void BeforeRunTask(const PendingTask* pending_task) = 0;
   };
 
-  // This is used to set the |ipc_program_counter| field for PendingTasks. It is
-  // intended to be used only from within generated IPC handler dispatch code.
-  class ScopedSetIpcProgramCounter;
+  // This is used to set the |ipc_hash| field for PendingTasks. It is intended
+  // to be used only from within generated IPC handler dispatch code.
+  class ScopedSetIpcHash;
 
   static const PendingTask* CurrentTaskForThread();
 
@@ -66,16 +66,16 @@ class BASE_EXPORT TaskAnnotator {
   DISALLOW_COPY_AND_ASSIGN(TaskAnnotator);
 };
 
-class BASE_EXPORT TaskAnnotator::ScopedSetIpcProgramCounter {
+class BASE_EXPORT TaskAnnotator::ScopedSetIpcHash {
  public:
-  explicit ScopedSetIpcProgramCounter(const void* program_counter);
-  ~ScopedSetIpcProgramCounter();
+  explicit ScopedSetIpcHash(uint32_t ipc_hash);
+  ~ScopedSetIpcHash();
 
  private:
   std::unique_ptr<PendingTask> dummy_pending_task_;
-  const void* old_ipc_program_counter_ = nullptr;
+  uint32_t old_ipc_hash_ = 0;
 
-  DISALLOW_COPY_AND_ASSIGN(ScopedSetIpcProgramCounter);
+  DISALLOW_COPY_AND_ASSIGN(ScopedSetIpcHash);
 };
 
 }  // namespace base

ipc/ipc_message_macros.h

@@ -199,6 +199,7 @@
 #include <tuple>
 
 #include "base/export_template.h"
+#include "base/hash/md5_constexpr.h"
 #include "base/location.h"
 #include "base/task/common/task_annotator.h"
 #include "ipc/ipc_message_templates.h"
@@ -325,12 +326,15 @@
 //     return handled;
 //   }
 
+#define IPC_TASK_ANNOTATOR_STRINGIFY(s) #s
+
 // A macro to be used from within the IPC_MESSAGE_FORWARD macros, for providing
 // the IPC message context to the TaskAnnotator. This allows posted tasks to be
 // associated with the incoming IPC message that caused them to be posted.
 #define IPC_TASK_ANNOTATOR_CONTEXT(msg_class)                            \
-  base::TaskAnnotator::ScopedSetIpcProgramCounter scoped_ipc_pc( \
-      base::GetProgramCounter());
+  static constexpr uint32_t kMessageHash =                               \
+      base::MD5Hash32Constexpr(IPC_TASK_ANNOTATOR_STRINGIFY(msg_class)); \
+  base::TaskAnnotator::ScopedSetIpcHash scoped_ipc_hash(kMessageHash);
 
 #define IPC_BEGIN_MESSAGE_MAP(class_name, msg) \
   { \

mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl

@@ -407,8 +407,9 @@ bool {{class_name}}StubDispatch::Accept(
           "(Impl){{namespace_as_string}}::{{class_name}}::{{method.name}}",
           "message", message->name());
 #endif
-      base::TaskAnnotator::ScopedSetIpcProgramCounter scoped_ipc_pc(
-          base::GetProgramCounter());
+      static constexpr uint32_t kMessageHash = base::MD5Hash32Constexpr(
+              "(Impl){{namespace_as_string}}::{{class_name}}::{{method.name}}");
+      base::TaskAnnotator::ScopedSetIpcHash scoped_ipc_hash(kMessageHash);
       mojo::internal::MessageDispatchContext context(message);
 {%-       if method|method_supports_lazy_serialization %}
       if (!message->is_serialized()) {
@@ -464,8 +465,9 @@ bool {{class_name}}StubDispatch::AcceptWithResponder(
           "(Impl){{namespace_as_string}}::{{class_name}}::{{method.name}}",
           "message", message->name());
 #endif
-      base::TaskAnnotator::ScopedSetIpcProgramCounter scoped_ipc_pc(
-          base::GetProgramCounter());
+      static constexpr uint32_t kMessageHash = base::MD5Hash32Constexpr(
+              "(Impl){{namespace_as_string}}::{{class_name}}::{{method.name}}");
+      base::TaskAnnotator::ScopedSetIpcHash scoped_ipc_hash(kMessageHash);
       mojo::internal::MessageDispatchContext context(message);
 {%-       if method|method_supports_lazy_serialization %}
       if (!message->is_serialized()) {

mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl

@@ -28,7 +28,7 @@
 #include <stdint.h>
 #include <utility>
 
-#include "base/location.h"
+#include "base/hash/md5_constexpr.h"
 #include "base/logging.h"
 #include "base/run_loop.h"
 #include "base/task/common/task_annotator.h"

services/tracing/public/cpp/trace_event_args_whitelist.cc

@@ -36,8 +36,7 @@ const char* const kRendererHostAllowedArgs[] = {
     "bytes_allocated", nullptr};
 const char* const kV8GCAllowedArgs[] = {"num_items", "num_tasks", nullptr};
 const char* const kTopLevelFlowAllowedArgs[] = {"task_queue_name", nullptr};
-const char* const kTopLevelIpcRunTaskAllowedArgs[] = {"ipc_program_counter",
-                                                      nullptr};
+const char* const kTopLevelIpcRunTaskAllowedArgs[] = {"ipc_hash", nullptr};
 
 const WhitelistEntry kEventArgsWhitelist[] = {
     {"__metadata", "thread_name", nullptr},