simpleperf: loosen unwinding arch check for system wide collection.
When doing system wide collection, it is possible that there are
32-bit compat processes running on 64-bit devices. It is not proper
to abort in this situation. So loosen the check to allow it. Also
add corresponding test.
Bug: 27927427
Change-Id: I5c9253eb6e474497e4f37e234e0e523e141fab20
diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp
index f4bd64c..1cea173 100644
--- a/simpleperf/cmd_record.cpp
+++ b/simpleperf/cmd_record.cpp
@@ -689,7 +689,11 @@
RegSet regs = CreateRegSet(r.regs_user_data.reg_mask, r.regs_user_data.regs);
std::vector<char>& stack = r.stack_user_data.data;
ArchType arch = GetArchForAbi(GetBuildArch(), r.regs_user_data.abi);
- std::vector<uint64_t> unwind_ips = UnwindCallChain(arch, *thread, regs, stack);
+ // Normally do strict arch check when unwinding stack. But allow unwinding 32-bit processes
+ // on 64-bit devices for system wide profiling.
+ bool strict_arch_check = !system_wide_collection_;
+ std::vector<uint64_t> unwind_ips = UnwindCallChain(arch, *thread, regs, stack,
+ strict_arch_check);
r.callchain_data.ips.push_back(PERF_CONTEXT_USER);
r.callchain_data.ips.insert(r.callchain_data.ips.end(), unwind_ips.begin(), unwind_ips.end());
r.regs_user_data.abi = 0;
diff --git a/simpleperf/cmd_record_test.cpp b/simpleperf/cmd_record_test.cpp
index a89febf..41a675b 100644
--- a/simpleperf/cmd_record_test.cpp
+++ b/simpleperf/cmd_record_test.cpp
@@ -53,9 +53,7 @@
}
TEST(record_cmd, system_wide_option) {
- if (IsRoot()) {
- ASSERT_TRUE(RunRecordCmd({"-a"}));
- }
+ TEST_IN_ROOT(ASSERT_TRUE(RunRecordCmd({"-a"})));
}
TEST(record_cmd, sample_period_option) {
@@ -107,9 +105,7 @@
}
TEST(record_cmd, tracepoint_event) {
- if (IsRoot()) {
- ASSERT_TRUE(RunRecordCmd({"-a", "-e", "sched:sched_switch"}));
- }
+ TEST_IN_ROOT(ASSERT_TRUE(RunRecordCmd({"-a", "-e", "sched:sched_switch"})));
}
TEST(record_cmd, branch_sampling) {
@@ -133,6 +129,10 @@
ASSERT_TRUE(RunRecordCmd({"--call-graph", "fp"}));
}
+TEST(record_cmd, system_wide_fp_callchain_sampling) {
+ TEST_IN_ROOT(ASSERT_TRUE(RunRecordCmd({"-a", "--call-graph", "fp"})));
+}
+
TEST(record_cmd, dwarf_callchain_sampling) {
if (IsDwarfCallChainSamplingSupported()) {
ASSERT_TRUE(RunRecordCmd({"--call-graph", "dwarf"}));
@@ -144,6 +144,15 @@
}
}
+TEST(record_cmd, system_wide_dwarf_callchain_sampling) {
+ if (IsDwarfCallChainSamplingSupported()) {
+ TEST_IN_ROOT(RunRecordCmd({"-a", "--call-graph", "dwarf"}));
+ } else {
+ GTEST_LOG_(INFO)
+ << "This test does nothing as dwarf callchain sampling is not supported on this device.";
+ }
+}
+
TEST(record_cmd, no_unwind_option) {
if (IsDwarfCallChainSamplingSupported()) {
ASSERT_TRUE(RunRecordCmd({"--call-graph", "dwarf", "--no-unwind"}));
@@ -196,9 +205,7 @@
TEST(record_cmd, cpu_option) {
ASSERT_TRUE(RunRecordCmd({"--cpu", "0"}));
- if (IsRoot()) {
- ASSERT_TRUE(RunRecordCmd({"--cpu", "0", "-a"}));
- }
+ TEST_IN_ROOT(ASSERT_TRUE(RunRecordCmd({"--cpu", "0", "-a"})));
}
TEST(record_cmd, mmap_page_option) {
diff --git a/simpleperf/cmd_report.cpp b/simpleperf/cmd_report.cpp
index d42bf69..b9c3d6f 100644
--- a/simpleperf/cmd_report.cpp
+++ b/simpleperf/cmd_report.cpp
@@ -272,6 +272,7 @@
record_filename_("perf.data"),
record_file_arch_(GetBuildArch()),
use_branch_address_(false),
+ system_wide_collection_(false),
accumulate_callchain_(false),
print_callgraph_(false),
callgraph_show_callee_(true),
@@ -312,6 +313,7 @@
std::unique_ptr<SampleTree> sample_tree_;
bool use_branch_address_;
std::string record_cmdline_;
+ bool system_wide_collection_;
bool accumulate_callchain_;
bool print_callgraph_;
bool callgraph_show_callee_;
@@ -569,8 +571,11 @@
std::vector<char> stack(r.stack_user_data.data.begin(),
r.stack_user_data.data.begin() + r.stack_user_data.data.size());
ArchType arch = GetArchForAbi(ScopedCurrentArch::GetCurrentArch(), r.regs_user_data.abi);
+ // Normally do strict arch check when unwinding stack. But allow unwinding 32-bit processes
+ // on 64-bit devices for system wide profiling.
+ bool strict_arch_check = !system_wide_collection_;
std::vector<uint64_t> unwind_ips =
- UnwindCallChain(arch, *sample->thread, regs, stack);
+ UnwindCallChain(arch, *sample->thread, regs, stack, strict_arch_check);
if (!unwind_ips.empty()) {
ips.push_back(PERF_CONTEXT_USER);
ips.insert(ips.end(), unwind_ips.begin(), unwind_ips.end());
@@ -647,6 +652,20 @@
std::vector<std::string> cmdline = record_file_reader_->ReadCmdlineFeature();
if (!cmdline.empty()) {
record_cmdline_ = android::base::Join(cmdline, ' ');
+ // TODO: the code to detect system wide collection option is fragile, remove it once we can
+ // do cross unwinding.
+ for (size_t i = 0; i < cmdline.size(); i++) {
+ std::string& s = cmdline[i];
+ if (s == "-a") {
+ system_wide_collection_ = true;
+ break;
+ } else if (s == "--call-graph" || s == "--cpu" || s == "-e" || s == "-f" || s == "-F" ||
+ s == "-j" || s == "-m" || s == "-o" || s == "-p" || s == "-t") {
+ i++;
+ } else if (!s.empty() && s[0] != '-') {
+ break;
+ }
+ }
}
return true;
}
diff --git a/simpleperf/cmd_stat_test.cpp b/simpleperf/cmd_stat_test.cpp
index 27f1f09..1935b5f 100644
--- a/simpleperf/cmd_stat_test.cpp
+++ b/simpleperf/cmd_stat_test.cpp
@@ -35,9 +35,7 @@
}
TEST(stat_cmd, system_wide_option) {
- if (IsRoot()) {
- ASSERT_TRUE(StatCmd()->Run({"-a", "sleep", "1"}));
- }
+ TEST_IN_ROOT(ASSERT_TRUE(StatCmd()->Run({"-a", "sleep", "1"})));
}
TEST(stat_cmd, verbose_option) {
@@ -45,9 +43,7 @@
}
TEST(stat_cmd, tracepoint_event) {
- if (IsRoot()) {
- ASSERT_TRUE(StatCmd()->Run({"-a", "-e", "sched:sched_switch", "sleep", "1"}));
- }
+ TEST_IN_ROOT(ASSERT_TRUE(StatCmd()->Run({"-a", "-e", "sched:sched_switch", "sleep", "1"})));
}
TEST(stat_cmd, event_modifier) {
@@ -87,7 +83,5 @@
TEST(stat_cmd, cpu_option) {
ASSERT_TRUE(StatCmd()->Run({"--cpu", "0", "sleep", "1"}));
- if (IsRoot()) {
- ASSERT_TRUE(StatCmd()->Run({"--cpu", "0", "-a", "sleep", "1"}));
- }
+ TEST_IN_ROOT(ASSERT_TRUE(StatCmd()->Run({"--cpu", "0", "-a", "sleep", "1"})));
}
diff --git a/simpleperf/dwarf_unwind.cpp b/simpleperf/dwarf_unwind.cpp
index 39bbef1..10baeb6 100644
--- a/simpleperf/dwarf_unwind.cpp
+++ b/simpleperf/dwarf_unwind.cpp
@@ -95,11 +95,12 @@
}
std::vector<uint64_t> UnwindCallChain(ArchType arch, const ThreadEntry& thread,
- const RegSet& regs, const std::vector<char>& stack) {
+ const RegSet& regs, const std::vector<char>& stack,
+ bool strict_arch_check) {
std::vector<uint64_t> result;
- if (arch != GetBuildArch()) {
+ if (!IsArchTheSame(arch, GetBuildArch(), strict_arch_check)) {
LOG(FATAL) << "simpleperf is built in arch " << GetArchString(GetBuildArch())
- << ", and can't do stack unwinding for arch " << GetArchString(arch);
+ << ", and can't do stack unwinding for arch " << GetArchString(arch);
return result;
}
uint64_t sp_reg_value;
diff --git a/simpleperf/dwarf_unwind.h b/simpleperf/dwarf_unwind.h
index 4e3ffd1..c725723 100644
--- a/simpleperf/dwarf_unwind.h
+++ b/simpleperf/dwarf_unwind.h
@@ -24,6 +24,6 @@
struct ThreadEntry;
std::vector<uint64_t> UnwindCallChain(ArchType arch, const ThreadEntry& thread, const RegSet& regs,
- const std::vector<char>& stack);
+ const std::vector<char>& stack, bool strict_arch_check);
#endif // SIMPLE_PERF_DWARF_UNWIND_H_
diff --git a/simpleperf/get_test_data.h b/simpleperf/get_test_data.h
index c380a7d..57a6e64 100644
--- a/simpleperf/get_test_data.h
+++ b/simpleperf/get_test_data.h
@@ -24,8 +24,6 @@
std::string GetTestData(const std::string& filename);
const std::string& GetTestDataDir();
-bool IsRoot();
-
// The source code of elf and elf_with_mini_debug_info is testdata/elf_file_source.cpp.
static const std::string ELF_FILE = "elf";
static const std::string ELF_FILE_WITH_MINI_DEBUG_INFO = "elf_with_mini_debug_info";
diff --git a/simpleperf/nonlinux_support/nonlinux_support.cpp b/simpleperf/nonlinux_support/nonlinux_support.cpp
index 7551d36..37833ed 100644
--- a/simpleperf/nonlinux_support/nonlinux_support.cpp
+++ b/simpleperf/nonlinux_support/nonlinux_support.cpp
@@ -21,7 +21,7 @@
#include "environment.h"
std::vector<uint64_t> UnwindCallChain(ArchType, const ThreadEntry&, const RegSet&,
- const std::vector<char>&) {
+ const std::vector<char>&, bool) {
return std::vector<uint64_t>();
}
diff --git a/simpleperf/perf_regs.cpp b/simpleperf/perf_regs.cpp
index 5d43db5..e96b83c 100644
--- a/simpleperf/perf_regs.cpp
+++ b/simpleperf/perf_regs.cpp
@@ -67,6 +67,25 @@
return "unknown";
}
+// If strict_check, must have arch1 == arch2.
+// Otherwise, allow X86_32 with X86_64, ARM with ARM64.
+bool IsArchTheSame(ArchType arch1, ArchType arch2, bool strict_check) {
+ if (strict_check) {
+ return arch1 == arch2;
+ }
+ switch (arch1) {
+ case ARCH_X86_32:
+ case ARCH_X86_64:
+ return arch2 == ARCH_X86_32 || arch2 == ARCH_X86_64;
+ case ARCH_ARM64:
+ case ARCH_ARM:
+ return arch2 == ARCH_ARM64 || arch2 == ARCH_ARM;
+ default:
+ break;
+ }
+ return arch1 == arch2;
+}
+
uint64_t GetSupportedRegMask(ArchType arch) {
switch (arch) {
case ARCH_X86_32:
diff --git a/simpleperf/perf_regs.h b/simpleperf/perf_regs.h
index 98c925f..7705e50 100644
--- a/simpleperf/perf_regs.h
+++ b/simpleperf/perf_regs.h
@@ -58,6 +58,7 @@
ArchType GetArchType(const std::string& arch);
ArchType GetArchForAbi(ArchType machine_arch, int abi);
std::string GetArchString(ArchType arch);
+bool IsArchTheSame(ArchType arch1, ArchType arch2, bool strict_check);
uint64_t GetSupportedRegMask(ArchType arch);
std::string GetRegName(size_t regno, ArchType arch);
diff --git a/simpleperf/test_util.h b/simpleperf/test_util.h
index 61a0ec7..15d11cb 100644
--- a/simpleperf/test_util.h
+++ b/simpleperf/test_util.h
@@ -28,3 +28,14 @@
void ParseSymbol(const ElfFileSymbol& symbol, std::map<std::string, ElfFileSymbol>* symbols);
void CheckElfFileSymbols(const std::map<std::string, ElfFileSymbol>& symbols);
+
+bool IsRoot();
+
+#define TEST_IN_ROOT(TestStatement) \
+ do { \
+ if (IsRoot()) { \
+ TestStatement; \
+ } else { \
+ GTEST_LOG_(INFO) << "Didn't test \"" << #TestStatement << "\" requires root privileges"; \
+ } \
+ } while (0)
