Merge "probes: invalidate seen pids on task_rename event"

8 files changed, 129 insertions, 0 deletions

diff --git a/src/traced/probes/ftrace/cpu_reader.cc b/src/traced/probes/ftrace/cpu_reader.cc
index d77fb780e..436edfbe4 100644
--- a/src/traced/probes/ftrace/cpu_reader.cc
+++ b/src/traced/probes/ftrace/cpu_reader.cc
@@ -582,6 +582,15 @@ bool CpuReader::ParseEvent(uint16_t ftrace_event_id,
     }
   }
 
+  if (PERFETTO_UNLIKELY(info.proto_field_id ==
+                        protos::pbzero::FtraceEvent::kTaskRenameFieldNumber)) {
+    // For task renames, we want to store that the pid was renamed. We use the
+    // common pid to reduce code complexity as in all the cases we care about,
+    // the common pid is the same as the renamed pid (the pid inside the event).
+    PERFETTO_DCHECK(metadata->last_seen_common_pid);
+    metadata->AddRenamePid(metadata->last_seen_common_pid);
+  }
+
   // This finalizes |nested| and |proto_field| automatically.
   message->Finalize();
   metadata->FinishEvent();
diff --git a/src/traced/probes/ftrace/cpu_reader_unittest.cc b/src/traced/probes/ftrace/cpu_reader_unittest.cc
index fe2ca7635..0631ecb8f 100644
--- a/src/traced/probes/ftrace/cpu_reader_unittest.cc
+++ b/src/traced/probes/ftrace/cpu_reader_unittest.cc
@@ -902,6 +902,33 @@ TEST_F(CpuReaderTableTest, ParseAllFields) {
               Contains(Pair(99u, k64BitUserspaceBlockDeviceId)));
 }
 
+TEST(CpuReaderTest, TaskRenameEvent) {
+  BundleProvider bundle_provider(base::kPageSize);
+
+  BinaryWriter writer;
+  ProtoTranslationTable* table = GetTable("android_seed_N2F62_3.10.49");
+
+  constexpr uint32_t kTaskRenameId = 19;
+
+  writer.Write<int32_t>(1001);             // Common field.
+  writer.Write<int32_t>(9999);             // Common pid
+  writer.Write<int32_t>(9999);             // Pid
+  writer.WriteFixedString(16, "Hello");    // Old Comm
+  writer.WriteFixedString(16, "Goodbye");  // New Comm
+  writer.Write<uint64_t>(10);              // flags
+  writer.Write<int16_t>(10);               // oom_score_adj
+
+  auto input = writer.GetCopy();
+  auto length = writer.written();
+  FtraceMetadata metadata{};
+
+  ASSERT_TRUE(CpuReader::ParseEvent(kTaskRenameId, input.get(),
+                                    input.get() + length, table,
+                                    bundle_provider.writer(), &metadata));
+  EXPECT_THAT(metadata.rename_pids, Contains(9999));
+  EXPECT_THAT(metadata.pids, Contains(9999));
+}
+
 TEST(CpuReaderTest, TranslateBlockDeviceIDToUserspace) {
   const uint32_t kKernelBlockDeviceId = 271581216;
   const BlockDeviceID kUserspaceBlockDeviceId = 66336;
diff --git a/src/traced/probes/ftrace/ftrace_metadata.cc b/src/traced/probes/ftrace/ftrace_metadata.cc
index 8660a852d..3ae8aaa96 100644
--- a/src/traced/probes/ftrace/ftrace_metadata.cc
+++ b/src/traced/probes/ftrace/ftrace_metadata.cc
@@ -24,6 +24,8 @@ FtraceMetadata::FtraceMetadata() {
   // A sched_switch is 64 bytes, a page is 4096 bytes and we expect
   // 2 pid's per sched_switch. 4096/64*2=128
   pids.reserve(128);
+  // We expect to see only a small number of task rename events.
+  rename_pids.reserve(16);
 }
 
 void FtraceMetadata::AddDevice(BlockDeviceID device_id) {
@@ -63,6 +65,10 @@ void FtraceMetadata::AddPid(int32_t pid) {
   pids.push_back(pid);
 }
 
+void FtraceMetadata::AddRenamePid(int32_t pid) {
+  rename_pids.push_back(pid);
+}
+
 void FtraceMetadata::FinishEvent() {
   last_seen_device_id = 0;
 #if PERFETTO_DCHECK_IS_ON()
@@ -74,6 +80,7 @@ void FtraceMetadata::FinishEvent() {
 void FtraceMetadata::Clear() {
   inode_and_device.clear();
   pids.clear();
+  rename_pids.clear();
   overwrite_count = 0;
   FinishEvent();
 }
diff --git a/src/traced/probes/ftrace/ftrace_metadata.h b/src/traced/probes/ftrace/ftrace_metadata.h
index 4e2e04a8c..9d8f98d91 100644
--- a/src/traced/probes/ftrace/ftrace_metadata.h
+++ b/src/traced/probes/ftrace/ftrace_metadata.h
@@ -44,11 +44,13 @@ struct FtraceMetadata {
   // A vector not a set to keep the writer_fast.
   std::vector<std::pair<Inode, BlockDeviceID>> inode_and_device;
   std::vector<int32_t> pids;
+  std::vector<int32_t> rename_pids;
 
   void AddDevice(BlockDeviceID);
   void AddInode(Inode);
   void AddPid(int32_t);
   void AddCommonPid(int32_t);
+  void AddRenamePid(int32_t);
   void Clear();
   void FinishEvent();
 };
diff --git a/src/traced/probes/probes_producer.cc b/src/traced/probes/probes_producer.cc
index 202fadc20..ebe68675a 100644
--- a/src/traced/probes/probes_producer.cc
+++ b/src/traced/probes/probes_producer.cc
@@ -428,8 +428,13 @@ void ProbesProducer::OnFtraceDataWrittenIntoDataSourceBuffers() {
     if (it == session_data_sources_.end() || it->first != last_session_id) {
       bool has_inodes = metadata && !metadata->inode_and_device.empty();
       bool has_pids = metadata && !metadata->pids.empty();
+      bool has_rename_pids = metadata && !metadata->rename_pids.empty();
       if (has_inodes && inode_data_source)
         inode_data_source->OnInodes(metadata->inode_and_device);
+      // Ordering the rename pids before the seen pids is important so that any
+      // renamed processes get scraped in the OnPids call.
+      if (has_rename_pids && ps_data_source)
+        ps_data_source->OnRenamePids(metadata->rename_pids);
       if (has_pids && ps_data_source)
         ps_data_source->OnPids(metadata->pids);
       if (metadata)
diff --git a/src/traced/probes/ps/process_stats_data_source.cc b/src/traced/probes/ps/process_stats_data_source.cc
index d0283b996..524a8b468 100644
--- a/src/traced/probes/ps/process_stats_data_source.cc
+++ b/src/traced/probes/ps/process_stats_data_source.cc
@@ -176,6 +176,19 @@ void ProcessStatsDataSource::OnPids(const std::vector<int32_t>& pids) {
   FinalizeCurPacket();
 }
 
+void ProcessStatsDataSource::OnRenamePids(const std::vector<int32_t>& pids) {
+  PERFETTO_METATRACE("OnRenamePids", 0);
+  if (!enable_on_demand_dumps_)
+    return;
+  PERFETTO_DCHECK(!cur_ps_tree_);
+  for (int32_t pid : pids) {
+    auto pid_it = seen_pids_.find(pid);
+    if (pid_it == seen_pids_.end())
+      continue;
+    seen_pids_.erase(pid_it);
+  }
+}
+
 void ProcessStatsDataSource::Flush(FlushRequestID,
                                    std::function<void()> callback) {
   // We shouldn't get this in the middle of WriteAllProcesses() or OnPids().
diff --git a/src/traced/probes/ps/process_stats_data_source.h b/src/traced/probes/ps/process_stats_data_source.h
index 4320f5344..efea49487 100644
--- a/src/traced/probes/ps/process_stats_data_source.h
+++ b/src/traced/probes/ps/process_stats_data_source.h
@@ -57,6 +57,7 @@ class ProcessStatsDataSource : public ProbesDataSource {
   base::WeakPtr<ProcessStatsDataSource> GetWeakPtr() const;
   void WriteAllProcesses();
   void OnPids(const std::vector<int32_t>& pids);
+  void OnRenamePids(const std::vector<int32_t>& pids);
 
   // ProbesDataSource implementation.
   void Start() override;
diff --git a/src/traced/probes/ps/process_stats_data_source_unittest.cc b/src/traced/probes/ps/process_stats_data_source_unittest.cc
index ffbb22745..0ccd7036a 100644
--- a/src/traced/probes/ps/process_stats_data_source_unittest.cc
+++ b/src/traced/probes/ps/process_stats_data_source_unittest.cc
@@ -215,6 +215,71 @@ TEST_F(ProcessStatsDataSourceTest, IncrementalStateClear) {
   }
 }
 
+TEST_F(ProcessStatsDataSourceTest, RenamePids) {
+  // assertion helpers
+  auto expected_old_process = [](int pid) {
+    return [pid](protos::ProcessTree::Process process) {
+      return process.pid() == pid && process.cmdline_size() > 0 &&
+             process.cmdline(0) == "proc_" + std::to_string(pid);
+    };
+  };
+  auto expected_new_process = [](int pid) {
+    return [pid](protos::ProcessTree::Process process) {
+      return process.pid() == pid && process.cmdline_size() > 0 &&
+             process.cmdline(0) == "new_" + std::to_string(pid);
+    };
+  };
+
+  DataSourceConfig config;
+  auto data_source = GetProcessStatsDataSource(config);
+  for (int p : {10, 20}) {
+    EXPECT_CALL(*data_source, ReadProcPidFile(p, "status"))
+        .WillRepeatedly(Invoke([](int32_t pid, const std::string&) {
+          return "Name: \tthread_" + std::to_string(pid) +
+                 "\nTgid:  " + std::to_string(pid) +
+                 "\nPid:   " + std::to_string(pid) + "\nPPid:  1\n";
+        }));
+
+    std::string old_proc_name = "proc_" + std::to_string(p);
+    old_proc_name.resize(old_proc_name.size() + 1);  // Add a trailing \0.
+
+    std::string new_proc_name = "new_" + std::to_string(p);
+    new_proc_name.resize(new_proc_name.size() + 1);  // Add a trailing \0.
+    EXPECT_CALL(*data_source, ReadProcPidFile(p, "cmdline"))
+        .WillOnce(Return(old_proc_name))
+        .WillOnce(Return(new_proc_name));
+  }
+
+  data_source->OnPids({10, 20});
+  data_source->OnRenamePids({10});
+  data_source->OnPids({10, 20});
+  data_source->OnRenamePids({20});
+  data_source->OnPids({10, 20});
+
+  // check written contents
+  std::vector<protos::TracePacket> trace = writer_raw_->GetAllTracePackets();
+  EXPECT_EQ(trace.size(), 3);
+
+  // first packet - two unique processes
+  auto ps_tree = trace[0].process_tree();
+  EXPECT_THAT(ps_tree.processes(),
+              UnorderedElementsAre(Truly(expected_old_process(10)),
+                                   Truly(expected_old_process(20))));
+  EXPECT_EQ(ps_tree.threads_size(), 0);
+
+  // second packet - one new process
+  ps_tree = trace[1].process_tree();
+  EXPECT_THAT(ps_tree.processes(),
+              UnorderedElementsAre(Truly(expected_new_process(10))));
+  EXPECT_EQ(ps_tree.threads_size(), 0);
+
+  // third packet - two threads that haven't been seen before
+  ps_tree = trace[2].process_tree();
+  EXPECT_THAT(ps_tree.processes(),
+              UnorderedElementsAre(Truly(expected_new_process(20))));
+  EXPECT_EQ(ps_tree.threads_size(), 0);
+}
+
 TEST_F(ProcessStatsDataSourceTest, ProcessStats) {
   DataSourceConfig cfg;
   cfg.mutable_process_stats_config()->set_proc_stats_poll_ms(1);