simpleperf: Remove stack and register fields for samples without

userspace stack

Otherwise, these fields will be left unrecognized.

Bug: 273341791
Test: run simpleperf_unit_test
(cherry picked from https://android-review.googlesource.com/q/commit:428d4a5fdb7745f73f52e930120dba9d2f215c26)
Merged-In: I481fa7fb21d9155349ab5c90a8ddf834dc364a08
Change-Id: I481fa7fb21d9155349ab5c90a8ddf834dc364a08

3 files changed, 55 insertions, 26 deletions

diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp
index c9ae6591..e168305e 100644
--- a/simpleperf/cmd_record.cpp
+++ b/simpleperf/cmd_record.cpp
@@ -1727,9 +1727,11 @@ void RecordCommand::UpdateRecord(Record* record) {
 }
 
 bool RecordCommand::UnwindRecord(SampleRecord& r) {
-  if ((r.sample_type & PERF_SAMPLE_CALLCHAIN) && (r.sample_type & PERF_SAMPLE_REGS_USER) &&
-      (r.regs_user_data.reg_mask != 0) && (r.sample_type & PERF_SAMPLE_STACK_USER) &&
-      (r.GetValidStackSize() > 0)) {
+  if (!(r.sample_type & PERF_SAMPLE_CALLCHAIN) && (r.sample_type & PERF_SAMPLE_REGS_USER) &&
+      (r.regs_user_data.reg_mask != 0) && (r.sample_type & PERF_SAMPLE_STACK_USER)) {
+    return true;
+  }
+  if (r.GetValidStackSize() > 0) {
     ThreadEntry* thread = thread_tree_.FindThreadOrNew(r.tid_data.pid, r.tid_data.tid);
     RegSet regs(r.regs_user_data.abi, r.regs_user_data.reg_mask, r.regs_user_data.regs);
     std::vector<uint64_t> ips;
@@ -1758,6 +1760,9 @@ bool RecordCommand::UnwindRecord(SampleRecord& r) {
                                          CallChainJoiner::ORIGINAL_OFFLINE, ips, sps)) {
       return false;
     }
+  } else {
+    // For kernel samples, we still need to remove user stack and register fields.
+    r.ReplaceRegAndStackWithCallChain({});
   }
   return true;
 }
diff --git a/simpleperf/record.cpp b/simpleperf/record.cpp
index c364b2fc..29e5d112 100644
--- a/simpleperf/record.cpp
+++ b/simpleperf/record.cpp
@@ -557,6 +557,8 @@ bool SampleRecord::Parse(const perf_event_attr& attr, char* p, char* end) {
     MoveFromBinaryFormat(regs_user_data.abi, p);
     if (regs_user_data.abi == 0) {
       regs_user_data.reg_mask = 0;
+      regs_user_data.reg_nr = 0;
+      regs_user_data.regs = nullptr;
     } else {
       regs_user_data.reg_mask = attr.sample_regs_user;
       size_t bit_nr = __builtin_popcountll(regs_user_data.reg_mask);
@@ -580,7 +582,7 @@ bool SampleRecord::Parse(const perf_event_attr& attr, char* p, char* end) {
   }
   // TODO: Add parsing of other PERF_SAMPLE_*.
   if (UNLIKELY(p < end)) {
-    LOG(DEBUG) << "Record has " << end - p << " bytes left\n";
+    LOG(DEBUG) << "Sample (" << time_data.time << ") has " << end - p << " bytes left";
   }
   return true;
 }
@@ -711,11 +713,15 @@ SampleRecord::SampleRecord(const perf_event_attr& attr, uint64_t id, uint64_t ip
 }
 
 void SampleRecord::ReplaceRegAndStackWithCallChain(const std::vector<uint64_t>& ips) {
-  uint32_t size_added_in_callchain = sizeof(uint64_t) * (ips.size() + 1);
-  uint32_t size_reduced_in_reg_stack =
-      (regs_user_data.reg_nr + 1) * sizeof(uint64_t) + stack_user_data.size + sizeof(uint64_t) * 2;
-
-  uint32_t new_size = size() + size_added_in_callchain - size_reduced_in_reg_stack;
+  uint32_t add_size_in_callchain = ips.empty() ? 0 : sizeof(uint64_t) * (ips.size() + 1);
+  uint32_t reduce_size_in_reg = (regs_user_data.reg_nr + 1) * sizeof(uint64_t);
+  uint32_t reduce_size_in_stack =
+      stack_user_data.size == 0 ? sizeof(uint64_t) : (stack_user_data.size + 2 * sizeof(uint64_t));
+  uint32_t reduce_size = reduce_size_in_reg + reduce_size_in_stack;
+
+  uint32_t new_size = size() + add_size_in_callchain;
+  CHECK_GT(new_size, reduce_size);
+  new_size -= reduce_size;
   sample_type &= ~(PERF_SAMPLE_STACK_USER | PERF_SAMPLE_REGS_USER);
   BuildBinaryWithNewCallChain(new_size, ips);
 }
@@ -818,16 +824,21 @@ void SampleRecord::BuildBinaryWithNewCallChain(uint32_t new_size,
     memcpy(p, &raw_data.size, sizeof(uint32_t));
   }
   uint64_t* p64 = reinterpret_cast<uint64_t*>(p);
-  p64 -= ips.size();
-  memcpy(p64, ips.data(), ips.size() * sizeof(uint64_t));
-  *--p64 = PERF_CONTEXT_USER;
-  if (callchain_data.ip_nr > 0) {
-    p64 -= callchain_data.ip_nr;
+  if (!ips.empty()) {
+    p64 -= ips.size();
+    memcpy(p64, ips.data(), ips.size() * sizeof(uint64_t));
+    *--p64 = PERF_CONTEXT_USER;
+  }
+  p64 -= callchain_data.ip_nr;
+  if (p64 != callchain_data.ips) {
     memcpy(p64, callchain_data.ips, callchain_data.ip_nr * sizeof(uint64_t));
+    callchain_data.ips = p64;
+  }
+  p64--;
+  if (!ips.empty()) {
+    callchain_data.ip_nr += 1 + ips.size();
+    *p64 = callchain_data.ip_nr;
   }
-  callchain_data.ips = p64;
-  callchain_data.ip_nr += 1 + ips.size();
-  *--p64 = callchain_data.ip_nr;
   CHECK_EQ(callchain_pos, static_cast<size_t>(reinterpret_cast<char*>(p64) - new_binary))
       << "record time " << time_data.time;
   if (new_binary != binary_) {
diff --git a/simpleperf/record_test.cpp b/simpleperf/record_test.cpp
index 8d9d151a..20c9c18a 100644
--- a/simpleperf/record_test.cpp
+++ b/simpleperf/record_test.cpp
@@ -105,15 +105,28 @@ TEST_F(RecordTest, SampleRecord_exclude_kernel_callchain) {
 
 TEST_F(RecordTest, SampleRecord_ReplaceRegAndStackWithCallChain) {
   event_attr.sample_type |= PERF_SAMPLE_CALLCHAIN;
-  SampleRecord expected(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, {1, PERF_CONTEXT_USER, 2, 3, 4, 5}, {},
-                        0);
-  for (size_t stack_size : {8, 1024}) {
-    event_attr.sample_type |= PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER;
-    SampleRecord r(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, {1}, std::vector<char>(stack_size), 10);
-    event_attr.sample_type &= ~(PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER);
-    r.ReplaceRegAndStackWithCallChain({2, 3, 4, 5});
-    CheckRecordMatchBinary(r);
-    CheckRecordEqual(r, expected);
+  std::vector<std::vector<uint64_t>> user_ip_tests = {
+      {},                     // no userspace ips, just remove stack and reg fields
+      {2},                    // add one userspace ip, no need to allocate new binary
+      {2, 3, 4, 5, 6, 7, 8},  // add more userspace ips, may need to allocate new binary
+  };
+  std::vector<uint64_t> stack_size_tests = {0, 8, 1024};
+
+  for (const auto& user_ips : user_ip_tests) {
+    std::vector<uint64_t> ips = {1};
+    if (!user_ips.empty()) {
+      ips.push_back(PERF_CONTEXT_USER);
+      ips.insert(ips.end(), user_ips.begin(), user_ips.end());
+    }
+    SampleRecord expected(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, ips, {}, 0);
+    for (size_t stack_size : stack_size_tests) {
+      event_attr.sample_type |= PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER;
+      SampleRecord r(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, {1}, std::vector<char>(stack_size), 10);
+      event_attr.sample_type &= ~(PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER);
+      r.ReplaceRegAndStackWithCallChain(user_ips);
+      CheckRecordMatchBinary(r);
+      CheckRecordEqual(r, expected);
+    }
   }
 }