simpleperf: improve warning msg.

The warning of missing symbol for each lib is shown twice,
once in recording, and once in reporting. This is redundant and
maybe misleading. Because when reporting we actually don't know
if we have collected enough symbols in perf.data. So move warning
of missing symbols to debug info if we have symbols in perf.data.

Use customized StderrLogger, because simpleperf doesn't need
time,pid,tid information in log.

Bug: http://b/29574526
Test: run simpleperf manually and run simpleperf_unit_test.
Change-Id: I9baf6d8fdcd63907681f2daa45b8fad6bf7e2516

diff --git a/simpleperf/command.cpp b/simpleperf/command.cpp
index 034163f..0d071ab 100644 (file)
--- a/simpleperf/command.cpp
+++ b/simpleperf/command.cpp
@@ -98,8 +98,15 @@ class CommandRegister {
 
 CommandRegister command_register;
 
+static void StderrLogger(android::base::LogId, android::base::LogSeverity severity,
+                         const char*, const char* file, unsigned int line, const char* message) {
+  static const char log_characters[] = "VDIWEFF";
+  char severity_char = log_characters[severity];
+  fprintf(stderr, "simpleperf %c %s:%u] %s\n", severity_char, file, line, message);
+}
+
 bool RunSimpleperfCmd(int argc, char** argv) {
-  android::base::InitLogging(argv, android::base::StderrLogger);
+  android::base::InitLogging(argv, StderrLogger);
   std::vector<std::string> args;
   android::base::LogSeverity log_severity = android::base::INFO;
 

diff --git a/simpleperf/dso.cpp b/simpleperf/dso.cpp
index 4f202bc..9080640 100644 (file)
--- a/simpleperf/dso.cpp
+++ b/simpleperf/dso.cpp
@@ -153,7 +153,8 @@ Dso::Dso(DsoType type, const std::string& path, bool force_64bit)
       min_vaddr_(std::numeric_limits<uint64_t>::max()),
       is_loaded_(false),
       dump_id_(UINT_MAX),
-      symbol_dump_id_(0) {
+      symbol_dump_id_(0),
+      symbol_warning_loglevel_(android::base::WARNING) {
   if (type_ == DSO_KERNEL) {
     min_vaddr_ = 0;
   }
@@ -287,6 +288,8 @@ void Dso::Load() {
     // dumped_symbols,  so later we can merge them with symbols read from file system.
     dumped_symbols = std::move(symbols_);
     symbols_.clear();
+    // Don't warn missing symbol table if we have dumped symbols in perf.data.
+    symbol_warning_loglevel_ = android::base::DEBUG;
   }
   bool result = false;
   switch (type_) {
@@ -354,11 +357,10 @@ bool Dso::CheckReadSymbolResult(ElfStatus result, const std::string& filename) {
       return true;
     }
     // Lacking symbol table isn't considered as an error but worth reporting.
-    LOG(WARNING) << filename << " doesn't contain symbol table";
+    LOG(symbol_warning_loglevel_) << filename << " doesn't contain symbol table";
     return true;
   } else {
-    LOG(WARNING) << "failed to read symbols from " << filename
-                 << ": " << result;
+    LOG(symbol_warning_loglevel_) << "failed to read symbols from " << filename << ": " << result;
     return false;
   }
 }
@@ -380,7 +382,7 @@ bool Dso::LoadKernel() {
       }
     }
     if (all_zero) {
-      LOG(WARNING)
+      LOG(symbol_warning_loglevel_)
           << "Symbol addresses in /proc/kallsyms on device are all zero. "
              "`echo 0 >/proc/sys/kernel/kptr_restrict` if possible.";
       symbols_.clear();
@@ -396,8 +398,8 @@ bool Dso::LoadKernel() {
       }
       bool match = (build_id == real_build_id);
       if (!match) {
-        LOG(WARNING) << "failed to read symbols from /proc/kallsyms: Build id "
-                     << "mismatch";
+        LOG(symbol_warning_loglevel_) << "failed to read symbols from /proc/kallsyms: Build id "
+                                      << "mismatch";
         return false;
       }
     }
@@ -417,8 +419,8 @@ bool Dso::LoadKernel() {
       }
     }
     if (all_zero) {
-      LOG(WARNING) << "Symbol addresses in /proc/kallsyms are all zero. "
-                      "`echo 0 >/proc/sys/kernel/kptr_restrict` if possible.";
+      LOG(symbol_warning_loglevel_) << "Symbol addresses in /proc/kallsyms are all zero. "
+                                       "`echo 0 >/proc/sys/kernel/kptr_restrict` if possible.";
       symbols_.clear();
       return false;
     }

diff --git a/simpleperf/dso.h b/simpleperf/dso.h
index f41b140..cb0e51d 100644 (file)
--- a/simpleperf/dso.h
+++ b/simpleperf/dso.h
@@ -22,6 +22,7 @@
 #include <unordered_map>
 #include <vector>
 
+#include <android-base/logging.h>
 #include <android-base/test_utils.h>
 
 #include "build_id.h"
@@ -182,6 +183,7 @@ class Dso {
   uint32_t dump_id_;
   // Used to assign dump_id for symbols in current dso.
   uint32_t symbol_dump_id_;
+  android::base::LogSeverity symbol_warning_loglevel_;
 };
 
 const char* DsoTypeToString(DsoType dso_type);