simpleperf: fix printing kernel address warning.

1. In cmd_record.cpp, don't read kernel addresses when kernel samples
are not recorded.
2. In kallsyms.cpp, print kernel address warning only once.
3. In kallsyms.cpp, use property only when running as root.

Bug: none
Test: run simpleperf_unit_test
Change-Id: I06de729ff3f26f7cd2115593182d6cb9cdc7f525

7 files changed, 141 insertions, 51 deletions

diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp
index b9fd6657..b4d3696c 100644
--- a/simpleperf/cmd_record.cpp
+++ b/simpleperf/cmd_record.cpp
@@ -1266,9 +1266,12 @@ bool RecordCommand::DumpMaps() {
       map_record_thread_.emplace(*map_record_reader_);
       return true;
     }
-    return map_record_reader_->ReadKernelMaps();
+    if (!event_selection_set_.ExcludeKernel()) {
+      return map_record_reader_->ReadKernelMaps();
+    }
+    return true;
   }
-  if (!map_record_reader_->ReadKernelMaps()) {
+  if (!event_selection_set_.ExcludeKernel() && !map_record_reader_->ReadKernelMaps()) {
     return false;
   }
   // Map from process id to a set of thread ids in that process.
@@ -1725,7 +1728,7 @@ bool RecordCommand::DumpAdditionalFeatures(const std::vector<std::string>& args)
   thread_tree_.ClearThreadAndMap();
   bool kernel_symbols_available = false;
   std::string kallsyms;
-  if (LoadKernelSymbols(&kallsyms)) {
+  if (event_selection_set_.NeedKernelSymbol() && LoadKernelSymbols(&kallsyms)) {
     Dso::SetKallsyms(kallsyms);
     kernel_symbols_available = true;
   }
diff --git a/simpleperf/cmd_record_test.cpp b/simpleperf/cmd_record_test.cpp
index 00b6463f..fa719b45 100644
--- a/simpleperf/cmd_record_test.cpp
+++ b/simpleperf/cmd_record_test.cpp
@@ -20,21 +20,19 @@
 #include <stdlib.h>
 #include <unistd.h>
 
-#include <android-base/file.h>
-#include <android-base/logging.h>
-#include <android-base/stringprintf.h>
-#include <android-base/strings.h>
-
-#if defined(__ANDROID__)
-#include <android-base/properties.h>
-#endif
-
 #include <filesystem>
 #include <map>
 #include <memory>
 #include <regex>
 #include <thread>
 
+#include <android-base/file.h>
+#include <android-base/logging.h>
+#include <android-base/properties.h>
+#include <android-base/stringprintf.h>
+#include <android-base/strings.h>
+#include <android-base/test_utils.h>
+
 #include "ETMRecorder.h"
 #include "ProbeEvents.h"
 #include "cmd_record_impl.h"
@@ -1122,3 +1120,27 @@ TEST(record_cmd, keep_failed_unwinding_result_option) {
   ASSERT_TRUE(RunRecordCmd(
       {"-p", pid, "-g", "--keep-failed-unwinding-result", "--keep-failed-unwinding-debug-info"}));
 }
+
+TEST(record_cmd, kernel_address_warning) {
+  TEST_REQUIRE_NON_ROOT();
+  const std::string warning_msg = "Access to kernel symbol addresses is restricted.";
+  CapturedStderr capture;
+
+  // When excluding kernel samples, no kernel address warning is printed.
+  ResetKernelAddressWarning();
+  TemporaryFile tmpfile;
+  ASSERT_TRUE(RunRecordCmd({"-e", "cpu-clock:u"}, tmpfile.path));
+  capture.Stop();
+  ASSERT_EQ(capture.str().find(warning_msg), std::string::npos);
+
+  // When not excluding kernel samples, kernel address warning is printed once.
+  capture.Reset();
+  capture.Start();
+  ResetKernelAddressWarning();
+  ASSERT_TRUE(RunRecordCmd({"-e", "cpu-clock"}, tmpfile.path));
+  capture.Stop();
+  std::string output = capture.str();
+  auto pos = output.find(warning_msg);
+  ASSERT_NE(pos, std::string::npos);
+  ASSERT_EQ(output.find(warning_msg, pos + warning_msg.size()), std::string::npos);
+}
diff --git a/simpleperf/event_selection_set.cpp b/simpleperf/event_selection_set.cpp
index f1e9d3a0..82d41fd4 100644
--- a/simpleperf/event_selection_set.cpp
+++ b/simpleperf/event_selection_set.cpp
@@ -478,14 +478,7 @@ void EventSelectionSet::SetClockId(int clock_id) {
 }
 
 bool EventSelectionSet::NeedKernelSymbol() const {
-  for (const auto& group : groups_) {
-    for (const auto& selection : group) {
-      if (!selection.event_type_modifier.exclude_kernel) {
-        return true;
-      }
-    }
-  }
-  return false;
+  return !ExcludeKernel();
 }
 
 void EventSelectionSet::SetRecordNotExecutableMaps(bool record) {
diff --git a/simpleperf/kallsyms.cpp b/simpleperf/kallsyms.cpp
index 844730f9..f9a93589 100644
--- a/simpleperf/kallsyms.cpp
+++ b/simpleperf/kallsyms.cpp
@@ -66,33 +66,41 @@ class ScopedKptrUnrestrict {
   ScopedKptrUnrestrict();   // Lowers kptr_restrict if necessary.
   ~ScopedKptrUnrestrict();  // Restores the initial kptr_restrict.
 
-  bool KallsymsAvailable();  // Indicates if access to kallsyms should be successful.
+  // Indicates if access to kallsyms should be successful.
+  bool KallsymsAvailable() { return kallsyms_available_; }
+
+  static void ResetWarning() { kernel_address_warning_printed_ = false; }
 
  private:
-  static bool WriteKptrRestrict(const std::string&);
+  bool WriteKptrRestrict(const std::string& value);
+  void PrintWarning();
 
-  std::string initial_value_;
-  bool use_property_;
-  bool restore_on_dtor_ = true;
+  bool restore_property_ = false;
+  bool restore_restrict_value_ = false;
+  std::string saved_restrict_value_;
   bool kallsyms_available_ = false;
+
+  static bool kernel_address_warning_printed_;
 };
 
+bool ScopedKptrUnrestrict::kernel_address_warning_printed_ = false;
+
 ScopedKptrUnrestrict::ScopedKptrUnrestrict() {
-  use_property_ = GetAndroidVersion() >= 12;
   if (CanReadKernelSymbolAddresses()) {
     // Everything seems to work (e.g., we are running as root and kptr_restrict
     // is < 2). Don't touching anything.
-    restore_on_dtor_ = false;
     kallsyms_available_ = true;
     return;
   }
 
-  if (use_property_) {
-    bool ret = android::base::SetProperty(kLowerPtrRestrictAndroidProp, "1");
-    if (!ret) {
-      LOG(ERROR) << "Unable to set " << kLowerPtrRestrictAndroidProp << " to 1.";
+  if (GetAndroidVersion() >= 12 && IsRoot()) {
+    // Enable kernel addresses by setting property.
+    if (!android::base::SetProperty(kLowerPtrRestrictAndroidProp, "1")) {
+      LOG(DEBUG) << "Unable to set " << kLowerPtrRestrictAndroidProp << " to 1.";
+      PrintWarning();
       return;
     }
+    restore_property_ = true;
     // Init takes some time to react to the property change.
     // Unfortunately, we cannot read kptr_restrict because of SELinux. Instead,
     // we detect this by reading the initial lines of kallsyms and checking
@@ -104,55 +112,59 @@ ScopedKptrUnrestrict::ScopedKptrUnrestrict() {
         return;
       }
     }
-    LOG(ERROR) << "kallsyms addresses are still masked after setting "
+    LOG(DEBUG) << "kallsyms addresses are still masked after setting "
                << kLowerPtrRestrictAndroidProp;
+    PrintWarning();
     return;
   }
 
   // Otherwise, read the kptr_restrict value and lower it if needed.
-  bool read_res = android::base::ReadFileToString(kPtrRestrictPath, &initial_value_);
-  if (!read_res) {
-    LOG(WARNING) << "Failed to read " << kPtrRestrictPath;
+  if (!android::base::ReadFileToString(kPtrRestrictPath, &saved_restrict_value_)) {
+    LOG(DEBUG) << "Failed to read " << kPtrRestrictPath;
+    PrintWarning();
     return;
   }
 
   // Progressively lower kptr_restrict until we can read kallsyms.
-  for (int value = atoi(initial_value_.c_str()); value > 0; --value) {
-    bool ret = WriteKptrRestrict(std::to_string(value));
-    if (!ret) {
-      LOG(WARNING) << "Access to kernel symbol addresses is restricted. If "
-                   << "possible, please do `echo 0 >/proc/sys/kernel/kptr_restrict` "
-                   << "to fix this.";
-      return;
+  for (int value = atoi(saved_restrict_value_.c_str()); value > 0; --value) {
+    if (!WriteKptrRestrict(std::to_string(value))) {
+      break;
     }
+    restore_restrict_value_ = true;
     if (CanReadKernelSymbolAddresses()) {
       kallsyms_available_ = true;
       return;
     }
   }
+  PrintWarning();
 }
 
 ScopedKptrUnrestrict::~ScopedKptrUnrestrict() {
-  if (!restore_on_dtor_) return;
-  if (use_property_) {
+  if (restore_property_) {
     android::base::SetProperty(kLowerPtrRestrictAndroidProp, "0");
-  } else if (!initial_value_.empty()) {
-    WriteKptrRestrict(initial_value_);
   }
-}
-
-bool ScopedKptrUnrestrict::KallsymsAvailable() {
-  return kallsyms_available_;
+  if (restore_restrict_value_) {
+    WriteKptrRestrict(saved_restrict_value_);
+  }
 }
 
 bool ScopedKptrUnrestrict::WriteKptrRestrict(const std::string& value) {
   if (!android::base::WriteStringToFile(value, kPtrRestrictPath)) {
-    LOG(WARNING) << "Failed to set " << kPtrRestrictPath << " to " << value;
+    LOG(DEBUG) << "Failed to set " << kPtrRestrictPath << " to " << value;
     return false;
   }
   return true;
 }
 
+void ScopedKptrUnrestrict::PrintWarning() {
+  if (!kernel_address_warning_printed_) {
+    kernel_address_warning_printed_ = true;
+    LOG(WARNING) << "Access to kernel symbol addresses is restricted. If "
+                 << "possible, please do `echo 0 >/proc/sys/kernel/kptr_restrict` "
+                 << "to fix this.";
+  }
+}
+
 }  // namespace
 
 std::vector<KernelMmap> GetLoadedModules() {
@@ -221,6 +233,10 @@ bool LoadKernelSymbols(std::string* kallsyms) {
   return false;
 }
 
+void ResetKernelAddressWarning() {
+  ScopedKptrUnrestrict::ResetWarning();
+}
+
 #endif  // defined(__linux__)
 
 bool ProcessKernelSymbols(std::string& symbol_data,
diff --git a/simpleperf/kallsyms.h b/simpleperf/kallsyms.h
index cb1b5a6a..33016e6d 100644
--- a/simpleperf/kallsyms.h
+++ b/simpleperf/kallsyms.h
@@ -49,6 +49,9 @@ uint64_t GetKernelStartAddress();
 // be restored. This usually requires root privileges.
 bool LoadKernelSymbols(std::string* kallsyms);
 
+// only for testing
+void ResetKernelAddressWarning();
+
 #endif  // defined(__linux__)
 
 }  // namespace simpleperf
diff --git a/simpleperf/kallsyms_test.cpp b/simpleperf/kallsyms_test.cpp
index e97327c6..d45aeaa6 100644
--- a/simpleperf/kallsyms_test.cpp
+++ b/simpleperf/kallsyms_test.cpp
@@ -16,6 +16,8 @@
 
 #include <gtest/gtest.h>
 
+#include <android-base/test_utils.h>
+
 #include "get_test_data.h"
 #include "kallsyms.h"
 #include "test_util.h"
@@ -73,4 +75,47 @@ TEST(kallsyms, LoadKernelSymbols) {
   std::string kallsyms;
   ASSERT_TRUE(LoadKernelSymbols(&kallsyms));
 }
+
+TEST(kallsyms, print_warning) {
+  TEST_REQUIRE_NON_ROOT();
+  const std::string warning_msg = "Access to kernel symbol addresses is restricted.";
+  CapturedStderr capture;
+
+  // Call each function requiring kernel addresses once. Check if the warning is printed.
+  ResetKernelAddressWarning();
+  ASSERT_EQ(0, GetKernelStartAddress());
+  capture.Stop();
+  ASSERT_NE(capture.str().find(warning_msg), std::string::npos);
+
+  capture.Reset();
+  capture.Start();
+  ResetKernelAddressWarning();
+  std::string kallsyms;
+  ASSERT_FALSE(LoadKernelSymbols(&kallsyms));
+  capture.Stop();
+  ASSERT_NE(capture.str().find(warning_msg), std::string::npos);
+
+  capture.Reset();
+  capture.Start();
+  ResetKernelAddressWarning();
+  ASSERT_TRUE(GetLoadedModules().empty());
+  capture.Stop();
+  ASSERT_NE(capture.str().find(warning_msg), std::string::npos);
+
+  // Call functions requiring kernel addresses more than once.
+  // Check if the kernel address warning is only printed once.
+  capture.Reset();
+  capture.Start();
+  ResetKernelAddressWarning();
+  for (int i = 0; i < 2; i++) {
+    ASSERT_EQ(0, GetKernelStartAddress());
+    ASSERT_FALSE(LoadKernelSymbols(&kallsyms));
+    ASSERT_TRUE(GetLoadedModules().empty());
+  }
+  capture.Stop();
+  std::string output = capture.str();
+  auto pos = output.find(warning_msg);
+  ASSERT_NE(pos, std::string::npos);
+  ASSERT_EQ(output.find(warning_msg, pos + warning_msg.size()), std::string::npos);
+}
 #endif  // defined(__ANDROID__)
diff --git a/simpleperf/test_util.h b/simpleperf/test_util.h
index d3d9b469..662c3b2f 100644
--- a/simpleperf/test_util.h
+++ b/simpleperf/test_util.h
@@ -54,6 +54,14 @@ void CheckElfFileSymbols(const std::map<std::string, ElfFileSymbol>& symbols);
     }                                                                    \
   } while (0)
 
+#define TEST_REQUIRE_NON_ROOT()                                            \
+  do {                                                                     \
+    if (IsRoot()) {                                                        \
+      GTEST_LOG_(INFO) << "Skip this test as it tests non-root behavior."; \
+      return;                                                              \
+    }                                                                      \
+  } while (0)
+
 #if defined(__ANDROID__)
 #define TEST_REQUIRE_HOST_ROOT()
 #else