From b438c746721acc45e8a87988e89417ab38d868f8 Mon Sep 17 00:00:00 2001
From: Googler <noreply@google.com>
Date: Wed, 24 Jul 2024 16:16:55 -0700
Subject: [PATCH] Minor refactoring.

PiperOrigin-RevId: 655735390
---
 src/perf_data_handler.cc   | 30 +++++++++++++++++-------------
 src/quipper/compat/proto.h |  2 +-
 src/quipper/perf_parser.cc |  4 ++--
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/perf_data_handler.cc b/src/perf_data_handler.cc
index 350b7a12..55f72729 100644
--- a/src/perf_data_handler.cc
+++ b/src/perf_data_handler.cc
@@ -533,15 +533,23 @@ void Normalizer::HandleSample(PerfDataHandler::SampleContext* context) {
   context->callchain.resize(sample.callchain_size());
   for (int i = 0; i < sample.callchain_size(); ++i) {
     ++stat_.callchain_ips;
-    if (sample.callchain(i) == quipper::PERF_CONTEXT_USER) {
+    uint64_t ip = sample.callchain(i);
+    if (ip == quipper::PERF_CONTEXT_USER) {
       ip_in_user_context = true;
-    } else if (sample.callchain(i) >= quipper::PERF_CONTEXT_MAX) {
+    } else if (ip >= quipper::PERF_CONTEXT_MAX) {
       ip_in_user_context = false;
     }
-    context->callchain[i].ip = sample.callchain(i);
-    context->callchain[i].mapping =
-        GetMappingFromPidAndIP(pid, sample.callchain(i), ip_in_user_context);
-    stat_.missing_callchain_mmap += context->callchain[i].mapping == nullptr;
+    const PerfDataHandler::Mapping* mapping;
+    if (ip >= quipper::PERF_CONTEXT_MAX) {
+      // This callchain frame is actually a context marker.
+      // Don't give it a mapping.
+      mapping = nullptr;
+      ++stat_.missing_callchain_mmap;
+    } else {
+      mapping = GetMappingFromPidAndIP(pid, ip, ip_in_user_context);
+    }
+    context->callchain[i].ip = ip;
+    context->callchain[i].mapping = mapping;
   }
 
   // Normalize the branch_stack.
@@ -863,13 +871,9 @@ const PerfDataHandler::Mapping* Normalizer::TryLookupInPid(uint32_t pid,
 // in our process.
 const PerfDataHandler::Mapping* Normalizer::GetMappingFromPidAndIP(
     uint32_t pid, uint64_t ip, bool ip_in_user_context) const {
-  if (ip >= quipper::PERF_CONTEXT_MAX || ip >> 60 == 0x8) {
-    // In case the ip is context hint or the highest 4 bits of ip is 1000,
-    // it has null mapping. For the latter case, we set the highest bit to mark
-    // the unmapped address. Kernel addresses in x86 and ARM have the high 16
-    // bits set, and for PowerPC the high 4 bits in range 0001 to 1011 is
-    // reserved space. So we would identify the unmapped address by checking
-    // its high four bits as 1000.
+  if (ip >> 60 == 0x8) {
+    // In case the highest 4 bits of ip is 1000, it has a null mapping. See
+    // the comment mentioning "highest 4 bits" in perf_parser.cc for details.
     return nullptr;
   }
   // First look up the mapping for the ip in the address space of the given pid.
diff --git a/src/quipper/compat/proto.h b/src/quipper/compat/proto.h
index c2227048..118bf913 100644
--- a/src/quipper/compat/proto.h
+++ b/src/quipper/compat/proto.h
@@ -5,7 +5,7 @@
 #ifndef CHROMIUMOS_WIDE_PROFILING_COMPAT_PROTO_H_
 #define CHROMIUMOS_WIDE_PROFILING_COMPAT_PROTO_H_
 
-#include "perf_data.pb.h"
+#include "src/quipper/perf_data.pb.h"
 #include "perf_parser_options.pb.h"
 #include "perf_stat.pb.h"
 #include "google/protobuf//arena.h"
diff --git a/src/quipper/perf_parser.cc b/src/quipper/perf_parser.cc
index f7ff4327..bcd3a834 100644
--- a/src/quipper/perf_parser.cc
+++ b/src/quipper/perf_parser.cc
@@ -654,8 +654,8 @@ bool PerfParser::MapIPAndPidAndGetNameAndOffset(
     // address is guaranteed to be larger than the mapped quipper space. When
     // options_.do_remap is false, the kernel addresses of x86 and ARM have
     // the high 16 bit set and PowerPC has a reserved space from
-    // 0x1000000000000000 to 0xBFFFFFFFFFFFFFFF. Thus, setting highest byte of
-    // the unmapped address, which starts with 0x8, should not collide with
+    // 0x1000000000000000 to 0xBFFFFFFFFFFFFFFF. Thus, setting highest 4 bits
+    // of the unmapped address, which starts with 0x8, should not collide with
     // any existing addresses or mapped quipper addresses.
     mapped_addr = (ip & ~(0xfULL << 60)) | 0x8ULL << 60;
   }