Merge "Snap for 7236715 from 32f48257eecb1a391a4bcec1fe9c9da72d13e44a to sdk-release" into sdk-release

9 files changed, 87 insertions, 60 deletions

diff --git a/profcollectd/binder/com/android/server/profcollect/IProfCollectd.aidl b/profcollectd/binder/com/android/server/profcollect/IProfCollectd.aidl
index bd3b76f8..9d4985b4 100644
--- a/profcollectd/binder/com/android/server/profcollect/IProfCollectd.aidl
+++ b/profcollectd/binder/com/android/server/profcollect/IProfCollectd.aidl
@@ -23,5 +23,6 @@ interface IProfCollectd {
     void trace_once(@utf8InCpp String tag);
     void process(boolean blocking);
     @utf8InCpp String report();
+    void delete_report(@utf8InCpp String report);
     @utf8InCpp String get_supported_provider();
 }
diff --git a/profcollectd/libprofcollectd/service.rs b/profcollectd/libprofcollectd/service.rs
index 3a6b8d13..4bc08970 100644
--- a/profcollectd/libprofcollectd/service.rs
+++ b/profcollectd/libprofcollectd/service.rs
@@ -16,16 +16,15 @@
 
 //! ProfCollect Binder service implementation.
 
-use anyhow::{Context, Error, Result};
+use anyhow::{bail, Context, Error, Result};
 use binder::public_api::Result as BinderResult;
 use binder::Status;
 use profcollectd_aidl_interface::aidl::com::android::server::profcollect::IProfCollectd::IProfCollectd;
 use std::ffi::CString;
 use std::fs::{create_dir, read_to_string, remove_dir_all, remove_file, write};
-use std::{
-    str::FromStr,
-    sync::{Mutex, MutexGuard},
-};
+use std::path::PathBuf;
+use std::str::FromStr;
+use std::sync::{Mutex, MutexGuard};
 
 use crate::config::{
     Config, CONFIG_FILE, OLD_REPORT_OUTPUT_FILE, PROFILE_OUTPUT_DIR, REPORT_OUTPUT_DIR,
@@ -87,11 +86,29 @@ impl IProfCollectd for ProfcollectdBinderService {
             .context("Failed to create profile report.")
             .map_err(err_to_binder_status)
     }
+    fn delete_report(&self, report_name: &str) -> BinderResult<()> {
+        verify_report_name(&report_name).map_err(err_to_binder_status)?;
+
+        let mut report = PathBuf::from(&*REPORT_OUTPUT_DIR);
+        report.push(report_name);
+        report.set_extension("zip");
+        remove_file(&report).ok();
+        Ok(())
+    }
     fn get_supported_provider(&self) -> BinderResult<String> {
         Ok(self.lock().scheduler.get_trace_provider_name().to_string())
     }
 }
 
+/// Verify that the report name is valid, i.e. not a relative path component, to prevent potential
+/// attack.
+fn verify_report_name(report_name: &str) -> Result<()> {
+    match report_name.chars().all(|c| c.is_ascii_hexdigit() || c == '-') {
+        true => Ok(()),
+        false => bail!("Not a report name: {}", report_name),
+    }
+}
+
 impl ProfcollectdBinderService {
     pub fn new() -> Result<Self> {
         let new_scheduler = Scheduler::new()?;
diff --git a/simpleperf/OWNERS b/simpleperf/OWNERS
index 860538d4..32f8fc65 100644
--- a/simpleperf/OWNERS
+++ b/simpleperf/OWNERS
@@ -1,2 +1,3 @@
 enh@google.com
 yabinc@google.com
+include platform/prebuilts/clang/host/linux-x86:/OWNERS
diff --git a/simpleperf/cmd_report_sample.cpp b/simpleperf/cmd_report_sample.cpp
index 68fa4bd8..186b120f 100644
--- a/simpleperf/cmd_report_sample.cpp
+++ b/simpleperf/cmd_report_sample.cpp
@@ -667,6 +667,10 @@ void ReportSampleCommand::AddUnwindingResultInProtobuf(
       // These error_codes shouldn't happen in simpleperf's use of libunwindstack.
       error_code = proto::Sample_UnwindingResult::ERROR_UNKNOWN;
       break;
+    default:
+      LOG(ERROR) << "unknown unwinding error code: " << unwinding_result.error_code;
+      error_code = proto::Sample_UnwindingResult::ERROR_UNKNOWN;
+      break;
   }
   proto_unwinding_result->set_error_code(error_code);
 }
diff --git a/simpleperf/dso.cpp b/simpleperf/dso.cpp
index 77a8b15f..d3b7ffba 100644
--- a/simpleperf/dso.cpp
+++ b/simpleperf/dso.cpp
@@ -463,9 +463,12 @@ class DexFileDso : public Dso {
 
   std::vector<Symbol> LoadSymbolsImpl() override {
     std::vector<Symbol> symbols;
-    std::vector<DexFileSymbol> dex_file_symbols;
     auto tuple = SplitUrlInApk(debug_file_path_);
     bool status = false;
+    auto symbol_callback = [&](DexFileSymbol* dex_symbol) {
+      symbols.emplace_back(std::string_view(dex_symbol->name, dex_symbol->name_size),
+                           dex_symbol->addr, dex_symbol->size);
+    };
     if (std::get<0>(tuple)) {
       std::unique_ptr<ArchiveHelper> ahelper = ArchiveHelper::CreateInstance(std::get<1>(tuple));
       ZipEntry entry;
@@ -473,10 +476,10 @@ class DexFileDso : public Dso {
       if (ahelper && ahelper->FindEntry(std::get<2>(tuple), &entry) &&
           ahelper->GetEntryData(entry, &data)) {
         status = ReadSymbolsFromDexFileInMemory(data.data(), data.size(), dex_file_offsets_,
-                                                &dex_file_symbols);
+                                                symbol_callback);
       }
     } else {
-      status = ReadSymbolsFromDexFile(debug_file_path_, dex_file_offsets_, &dex_file_symbols);
+      status = ReadSymbolsFromDexFile(debug_file_path_, dex_file_offsets_, symbol_callback);
     }
     if (!status) {
       android::base::LogSeverity level =
@@ -485,9 +488,6 @@ class DexFileDso : public Dso {
       return symbols;
     }
     LOG(VERBOSE) << "Read symbols from " << debug_file_path_ << " successfully";
-    for (auto& symbol : dex_file_symbols) {
-      symbols.emplace_back(symbol.name, symbol.offset, symbol.len);
-    }
     SortAndFixSymbols(symbols);
     return symbols;
   }
diff --git a/simpleperf/nonlinux_support/nonlinux_support.cpp b/simpleperf/nonlinux_support/nonlinux_support.cpp
index 823b6e5d..2268e301 100644
--- a/simpleperf/nonlinux_support/nonlinux_support.cpp
+++ b/simpleperf/nonlinux_support/nonlinux_support.cpp
@@ -32,12 +32,12 @@ bool CanRecordRawData() {
 }
 
 bool ReadSymbolsFromDexFileInMemory(void*, uint64_t, const std::vector<uint64_t>&,
-                                    std::vector<DexFileSymbol>*) {
+                                    const std::function<void(DexFileSymbol*)>&) {
   return true;
 }
 
 bool ReadSymbolsFromDexFile(const std::string&, const std::vector<uint64_t>&,
-                            std::vector<DexFileSymbol>*) {
+                            const std::function<void(DexFileSymbol*)>&) {
   return true;
 }
 
diff --git a/simpleperf/read_dex_file.cpp b/simpleperf/read_dex_file.cpp
index 055a4082..d4d50e07 100644
--- a/simpleperf/read_dex_file.cpp
+++ b/simpleperf/read_dex_file.cpp
@@ -31,27 +31,20 @@
 namespace simpleperf {
 
 static bool ReadSymbols(
-    const std::vector<uint64_t>& dex_file_offsets, std::vector<DexFileSymbol>* symbols,
-    const std::function<std::unique_ptr<art_api::dex::DexFile>(uint64_t offset)>& open_file_cb) {
+    const std::vector<uint64_t>& dex_file_offsets,
+    const std::function<std::unique_ptr<art_api::dex::DexFile>(uint64_t offset)>& open_file_cb,
+    const std::function<void(DexFileSymbol*)>& symbol_cb) {
   for (uint64_t offset : dex_file_offsets) {
     std::unique_ptr<art_api::dex::DexFile> dex_file = open_file_cb(offset);
     if (dex_file == nullptr) {
       return false;
     }
 
-    std::vector<art_api::dex::MethodInfo> file_syms = dex_file->GetAllMethodInfos(false);
-
-    // Adjust offsets to be from the start of the combined file.
-    for (art_api::dex::MethodInfo& sym : file_syms) {
-      sym.offset += offset;
-    }
-
-    if (symbols->empty()) {
-      *symbols = std::move(file_syms);
-    } else {
-      symbols->reserve(symbols->size() + file_syms.size());
-      std::move(std::begin(file_syms), std::end(file_syms), std::back_inserter(*symbols));
-    }
+    auto callback = [&](DexFileSymbol* symbol) {
+      symbol->addr += offset;
+      symbol_cb(symbol);
+    };
+    dex_file->GetAllMethodInfos(callback);
   }
 
   return true;
@@ -59,9 +52,10 @@ static bool ReadSymbols(
 
 bool ReadSymbolsFromDexFileInMemory(void* addr, uint64_t size,
                                     const std::vector<uint64_t>& dex_file_offsets,
-                                    std::vector<DexFileSymbol>* symbols) {
+                                    const std::function<void(DexFileSymbol*)>& symbol_callback) {
   return ReadSymbols(
-      dex_file_offsets, symbols, [&](uint64_t offset) -> std::unique_ptr<art_api::dex::DexFile> {
+      dex_file_offsets,
+      [&](uint64_t offset) -> std::unique_ptr<art_api::dex::DexFile> {
         size_t max_file_size;
         if (__builtin_sub_overflow(size, offset, &max_file_size)) {
           return nullptr;
@@ -75,28 +69,31 @@ bool ReadSymbolsFromDexFileInMemory(void* addr, uint64_t size,
           return nullptr;
         }
         return dex_file;
-      });
+      },
+      symbol_callback);
 }
 
 bool ReadSymbolsFromDexFile(const std::string& file_path,
                             const std::vector<uint64_t>& dex_file_offsets,
-                            std::vector<DexFileSymbol>* symbols) {
+                            const std::function<void(DexFileSymbol*)>& symbol_callback) {
   android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(file_path.c_str(), O_RDONLY | O_CLOEXEC)));
   if (fd == -1) {
     return false;
   }
-  return ReadSymbols(dex_file_offsets, symbols,
-                     [&](uint64_t offset) -> std::unique_ptr<art_api::dex::DexFile> {
-                       std::string error_msg;
-                       std::unique_ptr<art_api::dex::DexFile> dex_file =
-                           art_api::dex::DexFile::OpenFromFd(fd, offset, file_path, &error_msg);
-                       if (dex_file == nullptr) {
-                         LOG(WARNING) << "Failed to read dex file symbols from '" << file_path
-                                      << "': " << error_msg;
-                         return nullptr;
-                       }
-                       return dex_file;
-                     });
+  return ReadSymbols(
+      dex_file_offsets,
+      [&](uint64_t offset) -> std::unique_ptr<art_api::dex::DexFile> {
+        std::string error_msg;
+        std::unique_ptr<art_api::dex::DexFile> dex_file =
+            art_api::dex::DexFile::OpenFromFd(fd, offset, file_path, &error_msg);
+        if (dex_file == nullptr) {
+          LOG(WARNING) << "Failed to read dex file symbols from '" << file_path
+                       << "': " << error_msg;
+          return nullptr;
+        }
+        return dex_file;
+      },
+      symbol_callback);
 }
 
 }  // namespace simpleperf
diff --git a/simpleperf/read_dex_file.h b/simpleperf/read_dex_file.h
index ebddc87d..e5b97904 100644
--- a/simpleperf/read_dex_file.h
+++ b/simpleperf/read_dex_file.h
@@ -28,22 +28,24 @@
 
 namespace simpleperf {
 
-#ifndef NO_LIBDEXFILE_SUPPORT
-using DexFileSymbol = art_api::dex::MethodInfo;
-#else
+#ifdef NO_LIBDEXFILE_SUPPORT
 struct DexFileSymbol {
-  uint64_t offset;
-  uint64_t len;
-  std::string name;
+  size_t sizeof_struct;
+  uint32_t addr;
+  uint32_t size;
+  const char* name;
+  size_t name_size;
 };
+#else
+using DexFileSymbol = ExtDexFileMethodInfo;
 #endif
 
 bool ReadSymbolsFromDexFileInMemory(void* addr, uint64_t size,
                                     const std::vector<uint64_t>& dex_file_offsets,
-                                    std::vector<DexFileSymbol>* symbols);
+                                    const std::function<void(DexFileSymbol*)>& symbol_callback);
 bool ReadSymbolsFromDexFile(const std::string& file_path,
                             const std::vector<uint64_t>& dex_file_offsets,
-                            std::vector<DexFileSymbol>* symbols);
+                            const std::function<void(DexFileSymbol*)>& symbol_callback);
 
 }  // namespace simpleperf
 
diff --git a/simpleperf/read_dex_file_test.cpp b/simpleperf/read_dex_file_test.cpp
index 3d0e900f..5d71bc35 100644
--- a/simpleperf/read_dex_file_test.cpp
+++ b/simpleperf/read_dex_file_test.cpp
@@ -20,6 +20,7 @@
 
 #include <algorithm>
 
+#include "dso.h"
 #include "get_test_data.h"
 #include "test_util.h"
 #include "utils.h"
@@ -27,13 +28,17 @@
 using namespace simpleperf;
 
 TEST(read_dex_file, smoke) {
-  std::vector<DexFileSymbol> symbols;
-  ASSERT_TRUE(ReadSymbolsFromDexFile(GetTestData("base.vdex"), {0x28}, &symbols));
+  std::vector<Symbol> symbols;
+  auto symbol_callback = [&](DexFileSymbol* symbol) {
+    symbols.emplace_back(std::string_view(symbol->name, symbol->name_size), symbol->addr,
+                         symbol->size);
+  };
+  ASSERT_TRUE(ReadSymbolsFromDexFile(GetTestData("base.vdex"), {0x28}, symbol_callback));
   ASSERT_EQ(12435u, symbols.size());
-  DexFileSymbol target;
-  target.offset = 0x6c77e;
-  target.len = 0x16;
-  target.name = art_api::dex::DexString(
-      "com.example.simpleperf.simpleperfexamplewithnative.MixActivity$1.run");
-  ASSERT_NE(std::find(symbols.begin(), symbols.end(), target), symbols.end());
+  auto it = std::find_if(symbols.begin(), symbols.end(),
+                         [](const Symbol& symbol) { return symbol.addr == 0x6c77e; });
+  ASSERT_NE(it, symbols.end());
+  ASSERT_EQ(it->addr, 0x6c77e);
+  ASSERT_EQ(it->len, 0x16);
+  ASSERT_STREQ(it->Name(), "com.example.simpleperf.simpleperfexamplewithnative.MixActivity$1.run");
 }