Merge "simpleperf: handle unexpected dso type error."

12 files changed, 40 insertions, 15 deletions

diff --git a/simpleperf/cmd_debug_unwind.cpp b/simpleperf/cmd_debug_unwind.cpp
index 560a1834..c78c3cf8 100644
--- a/simpleperf/cmd_debug_unwind.cpp
+++ b/simpleperf/cmd_debug_unwind.cpp
@@ -146,7 +146,9 @@ class RecordFileProcessor {
     }
 
     // 2. Load feature sections.
-    reader_->LoadBuildIdAndFileFeatures(thread_tree_);
+    if (!reader_->LoadBuildIdAndFileFeatures(thread_tree_)) {
+      return false;
+    }
     ScopedCurrentArch scoped_arch(
         GetArchType(reader_->ReadFeatureString(PerfFileFormat::FEAT_ARCH)));
     unwinder_->LoadMetaInfo(reader_->GetMetaInfoFeature());
diff --git a/simpleperf/cmd_dumprecord.cpp b/simpleperf/cmd_dumprecord.cpp
index 96a0bd81..85e2c598 100644
--- a/simpleperf/cmd_dumprecord.cpp
+++ b/simpleperf/cmd_dumprecord.cpp
@@ -322,7 +322,9 @@ void DumpRecordCommand::DumpAttrSection() {
 
 bool DumpRecordCommand::DumpDataSection() {
   thread_tree_.ShowIpForUnknownSymbol();
-  record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_);
+  if (!record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_)) {
+    return false;
+  }
 
   auto record_callback = [&](std::unique_ptr<Record> r) { return ProcessRecord(r.get()); };
   return record_file_reader_->ReadDataSection(record_callback);
diff --git a/simpleperf/cmd_inject.cpp b/simpleperf/cmd_inject.cpp
index 7477cb6a..ab20ea4c 100644
--- a/simpleperf/cmd_inject.cpp
+++ b/simpleperf/cmd_inject.cpp
@@ -275,7 +275,9 @@ class PerfDataReader {
         thread_tree_.ExcludePid(pid);
       }
     }
-    record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_);
+    if (!record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_)) {
+      return false;
+    }
     if (!record_file_reader_->ReadDataSection([this](auto r) { return ProcessRecord(r.get()); })) {
       return false;
     }
diff --git a/simpleperf/cmd_kmem.cpp b/simpleperf/cmd_kmem.cpp
index ba8246c2..81746ba5 100644
--- a/simpleperf/cmd_kmem.cpp
+++ b/simpleperf/cmd_kmem.cpp
@@ -543,7 +543,9 @@ void KmemCommand::ReadEventAttrsFromRecordFile() {
 }
 
 bool KmemCommand::ReadFeaturesFromRecordFile() {
-  record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_);
+  if (!record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_)) {
+    return false;
+  }
   std::string arch = record_file_reader_->ReadFeatureString(PerfFileFormat::FEAT_ARCH);
   if (!arch.empty()) {
     record_file_arch_ = GetArchType(arch);
diff --git a/simpleperf/cmd_report.cpp b/simpleperf/cmd_report.cpp
index d22e670f..f68eddce 100644
--- a/simpleperf/cmd_report.cpp
+++ b/simpleperf/cmd_report.cpp
@@ -895,7 +895,9 @@ bool ReportCommand::ReadEventAttrFromRecordFile() {
 }
 
 bool ReportCommand::ReadFeaturesFromRecordFile() {
-  record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_);
+  if (!record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_)) {
+    return false;
+  }
 
   std::string arch = record_file_reader_->ReadFeatureString(PerfFileFormat::FEAT_ARCH);
   if (!arch.empty()) {
diff --git a/simpleperf/cmd_report_sample.cpp b/simpleperf/cmd_report_sample.cpp
index 6c240484..1026dbbe 100644
--- a/simpleperf/cmd_report_sample.cpp
+++ b/simpleperf/cmd_report_sample.cpp
@@ -515,7 +515,9 @@ bool ReportSampleCommand::OpenRecordFile() {
   if (record_file_reader_ == nullptr) {
     return false;
   }
-  record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_);
+  if (!record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_)) {
+    return false;
+  }
   auto& meta_info = record_file_reader_->GetMetaInfoFeature();
   if (auto it = meta_info.find("trace_offcpu"); it != meta_info.end()) {
     trace_offcpu_ = it->second == "true";
diff --git a/simpleperf/dso.cpp b/simpleperf/dso.cpp
index 71d40c38..386febab 100644
--- a/simpleperf/dso.cpp
+++ b/simpleperf/dso.cpp
@@ -948,9 +948,9 @@ std::unique_ptr<Dso> Dso::CreateDso(DsoType dso_type, const std::string& dso_pat
     case DSO_UNKNOWN_FILE:
       return std::unique_ptr<Dso>(new UnknownDso(dso_path));
     default:
-      LOG(FATAL) << "Unexpected dso_type " << static_cast<int>(dso_type);
+      LOG(ERROR) << "Unexpected dso_type " << static_cast<int>(dso_type);
+      return nullptr;
   }
-  return nullptr;
 }
 
 std::unique_ptr<Dso> Dso::CreateDsoWithBuildId(DsoType dso_type, const std::string& dso_path,
@@ -967,7 +967,7 @@ std::unique_ptr<Dso> Dso::CreateDsoWithBuildId(DsoType dso_type, const std::stri
       dso.reset(new KernelModuleDso(dso_path, 0, 0, nullptr));
       break;
     default:
-      LOG(FATAL) << "Unexpected dso_type " << static_cast<int>(dso_type);
+      LOG(ERROR) << "Unexpected dso_type " << static_cast<int>(dso_type);
       return nullptr;
   }
   dso->debug_file_path_ = debug_elf_file_finder_.FindDebugFile(dso_path, false, build_id);
diff --git a/simpleperf/record_file.h b/simpleperf/record_file.h
index 3c972f09..0f436a2e 100644
--- a/simpleperf/record_file.h
+++ b/simpleperf/record_file.h
@@ -192,7 +192,7 @@ class RecordFileReader {
   std::string GetClockId();
   std::optional<DebugUnwindFeature> ReadDebugUnwindFeature();
 
-  void LoadBuildIdAndFileFeatures(ThreadTree& thread_tree);
+  bool LoadBuildIdAndFileFeatures(ThreadTree& thread_tree);
 
   bool ReadAuxData(uint32_t cpu, uint64_t aux_offset, void* buf, size_t size);
 
diff --git a/simpleperf/record_file_reader.cpp b/simpleperf/record_file_reader.cpp
index 7fe8a898..7a3d22a5 100644
--- a/simpleperf/record_file_reader.cpp
+++ b/simpleperf/record_file_reader.cpp
@@ -708,7 +708,7 @@ std::optional<DebugUnwindFeature> RecordFileReader::ReadDebugUnwindFeature() {
   return std::nullopt;
 }
 
-void RecordFileReader::LoadBuildIdAndFileFeatures(ThreadTree& thread_tree) {
+bool RecordFileReader::LoadBuildIdAndFileFeatures(ThreadTree& thread_tree) {
   std::vector<BuildIdRecord> records = ReadBuildIdFeature();
   std::vector<std::pair<std::string, BuildId>> build_ids;
   for (auto& r : records) {
@@ -720,9 +720,12 @@ void RecordFileReader::LoadBuildIdAndFileFeatures(ThreadTree& thread_tree) {
     FileFeature file_feature;
     size_t read_pos = 0;
     while (ReadFileFeature(read_pos, &file_feature)) {
-      thread_tree.AddDsoInfo(file_feature);
+      if (!thread_tree.AddDsoInfo(file_feature)) {
+        return false;
+      }
     }
   }
+  return true;
 }
 
 bool RecordFileReader::ReadAuxData(uint32_t cpu, uint64_t aux_offset, void* buf, size_t size) {
diff --git a/simpleperf/report_lib_interface.cpp b/simpleperf/report_lib_interface.cpp
index 15699cb5..d94d6a76 100644
--- a/simpleperf/report_lib_interface.cpp
+++ b/simpleperf/report_lib_interface.cpp
@@ -326,7 +326,9 @@ bool ReportLib::OpenRecordFileIfNecessary() {
     if (record_file_reader_ == nullptr) {
       return false;
     }
-    record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_);
+    if (!record_file_reader_->LoadBuildIdAndFileFeatures(thread_tree_)) {
+      return false;
+    }
     auto& meta_info = record_file_reader_->GetMetaInfoFeature();
     if (auto it = meta_info.find("trace_offcpu"); it != meta_info.end() && it->second == "true") {
       // If recorded with --trace-offcpu, default is to report on-off-cpu samples.
diff --git a/simpleperf/thread_tree.cpp b/simpleperf/thread_tree.cpp
index be346a89..f4f3037c 100644
--- a/simpleperf/thread_tree.cpp
+++ b/simpleperf/thread_tree.cpp
@@ -159,6 +159,7 @@ void ThreadTree::AddThreadMap(int pid, int tid, uint64_t start_addr, uint64_t le
                               const std::string& filename, uint32_t flags) {
   ThreadEntry* thread = FindThreadOrNew(pid, tid);
   Dso* dso = FindUserDsoOrNew(filename, start_addr);
+  CHECK(dso != nullptr);
   InsertMap(*thread->maps, MapEntry(start_addr, len, pgoff, dso, false, flags));
 }
 
@@ -197,6 +198,9 @@ Dso* ThreadTree::FindUserDsoOrNew(const std::string& filename, uint64_t start_ad
   if (it == user_dso_tree_.end()) {
     bool force_64bit = start_addr > UINT_MAX;
     std::unique_ptr<Dso> dso = Dso::CreateDso(dso_type, filename, force_64bit);
+    if (!dso) {
+      return nullptr;
+    }
     auto pair = user_dso_tree_.insert(std::make_pair(filename, std::move(dso)));
     CHECK(pair.second);
     it = pair.first;
@@ -347,7 +351,7 @@ void ThreadTree::ClearThreadAndMap() {
   map_storage_.clear();
 }
 
-void ThreadTree::AddDsoInfo(FileFeature& file) {
+bool ThreadTree::AddDsoInfo(FileFeature& file) {
   DsoType dso_type = file.type;
   Dso* dso = nullptr;
   if (dso_type == DSO_KERNEL) {
@@ -357,11 +361,15 @@ void ThreadTree::AddDsoInfo(FileFeature& file) {
   } else {
     dso = FindUserDsoOrNew(file.path, 0, dso_type);
   }
+  if (!dso) {
+    return false;
+  }
   dso->SetMinExecutableVaddr(file.min_vaddr, file.file_offset_of_min_vaddr);
   dso->SetSymbols(&file.symbols);
   for (uint64_t offset : file.dex_file_offsets) {
     dso->AddDexFileOffset(offset);
   }
+  return true;
 }
 
 void ThreadTree::AddDexFileOffset(const std::string& file_path, uint64_t dex_file_offset) {
diff --git a/simpleperf/thread_tree.h b/simpleperf/thread_tree.h
index 33e85450..ecaf93c9 100644
--- a/simpleperf/thread_tree.h
+++ b/simpleperf/thread_tree.h
@@ -131,7 +131,7 @@ class ThreadTree {
   // Clear thread and map information, but keep loaded dso information. It saves
   // the time to reload dso information.
   void ClearThreadAndMap();
-  void AddDsoInfo(FileFeature& file);
+  bool AddDsoInfo(FileFeature& file);
   void AddDexFileOffset(const std::string& file_path, uint64_t dex_file_offset);
 
   // Update thread tree with information provided by record.