17 files changed, 138 insertions, 53 deletions
diff --git a/profcollectd/libprofcollectd/Android.bp b/profcollectd/libprofcollectd/Android.bp
index 2e37af9e..7fb1b567 100644
--- a/profcollectd/libprofcollectd/Android.bp
+++ b/profcollectd/libprofcollectd/Android.bp
@@ -60,4 +60,7 @@ rust_library {
         "libsimpleperf_profcollect_rust",
     ],
     shared_libs: ["libsimpleperf_profcollect"],
+
+    // Enable 'test' feature for more verbose logging and the logging trace provider.
+    // features: ["test"],
 }
diff --git a/profcollectd/libprofcollectd/bindings/libbase/Android.bp b/profcollectd/libprofcollectd/bindings/libbase/Android.bp
index b99d41d8..8e5fb1e0 100644
--- a/profcollectd/libprofcollectd/bindings/libbase/Android.bp
+++ b/profcollectd/libprofcollectd/bindings/libbase/Android.bp
@@ -26,20 +26,25 @@ package {
 cc_library_static {
     name: "libprofcollect_libbase",
     srcs: ["properties.cpp"],
+    generated_headers: ["cxx-bridge-header"],
+    generated_sources: ["libprofcollect_libbase_bridge_code"],
 }
 
-rust_bindgen {
-    name: "libprofcollect_libbase_bindgen",
-    wrapper_src: "properties.hpp",
-    crate_name: "profcollect_libbase_bindgen",
-    source_stem: "bindings",
+genrule {
+    name: "libprofcollect_libbase_bridge_code",
+    tools: ["cxxbridge"],
+    cmd: "$(location cxxbridge) $(in) >> $(out)",
+    srcs: ["lib.rs"],
+    out: ["libprofcollect_libbase_cxx_generated.cc"],
 }
 
 rust_library {
     name: "libprofcollect_libbase_rust",
     crate_name: "profcollect_libbase_rust",
     srcs: ["lib.rs"],
-    rlibs: ["libprofcollect_libbase_bindgen"],
+    rustlibs: [
+        "libcxx",
+    ],
     static_libs: ["libprofcollect_libbase"],
     shared_libs: [
         "libc++",
diff --git a/profcollectd/libprofcollectd/bindings/libbase/lib.rs b/profcollectd/libprofcollectd/bindings/libbase/lib.rs
index 6ac34d58..bdd99a02 100644
--- a/profcollectd/libprofcollectd/bindings/libbase/lib.rs
+++ b/profcollectd/libprofcollectd/bindings/libbase/lib.rs
@@ -14,26 +14,21 @@
 // limitations under the License.
 //
 
-//! This module implements safe wrappers for GetProperty method from libbase.
+//! This module implements safe wrappers for GetProperty and SetProperty from libbase.
 
-use std::ffi::{CStr, CString};
+pub use ffi::{GetProperty, SetProperty};
 
-/// Returns the current value of the system property `key`,
-/// or `default_value` if the property is empty or doesn't exist.
-pub fn get_property<'a>(key: &str, default_value: &'a str) -> &'a str {
-    let key = CString::new(key).unwrap();
-    let default_value = CString::new(default_value).unwrap();
-    unsafe {
-        let cstr = profcollect_libbase_bindgen::GetProperty(key.as_ptr(), default_value.as_ptr());
-        CStr::from_ptr(cstr).to_str().unwrap()
-    }
-}
+/// Safe wrappers for the GetProperty and SetProperty methods from libbase.
+#[cxx::bridge]
+mod ffi {
+    unsafe extern "C++" {
+        include!("properties.hpp");
+
+        /// Returns the current value of the system property `key`,
+        /// or `default_value` if the property is empty or doesn't exist.
+        fn GetProperty(key: &str, default_value: &str) -> String;
 
-/// Sets the system property `key` to `value`.
-pub fn set_property(key: &str, value: &str) {
-    let key = CString::new(key).unwrap();
-    let value = CString::new(value).unwrap();
-    unsafe {
-        profcollect_libbase_bindgen::SetProperty(key.as_ptr(), value.as_ptr());
+        /// Sets the system property `key` to `value`.
+        fn SetProperty(key: &str, value: &str);
     }
 }
diff --git a/profcollectd/libprofcollectd/bindings/libbase/properties.cpp b/profcollectd/libprofcollectd/bindings/libbase/properties.cpp
index 01be7c73..908f19d0 100644
--- a/profcollectd/libprofcollectd/bindings/libbase/properties.cpp
+++ b/profcollectd/libprofcollectd/bindings/libbase/properties.cpp
@@ -17,11 +17,10 @@
 #include "../../../../../libbase/include/android-base/properties.h"
 #include "properties.hpp"
 
-const char* GetProperty(const char* key, const char* default_value) {
-  auto v = android::base::GetProperty(std::string(key), std::string(default_value));
-  return strdup(v.c_str());
+rust::String GetProperty(rust::Str key, rust::Str default_value) {
+  return android::base::GetProperty(std::string(key), std::string(default_value));
 }
 
-void SetProperty(const char* key, const char* value) {
+void SetProperty(rust::Str key, rust::Str value) {
   android::base::SetProperty(std::string(key), std::string(value));
 }
diff --git a/profcollectd/libprofcollectd/bindings/libbase/properties.hpp b/profcollectd/libprofcollectd/bindings/libbase/properties.hpp
index d6785a28..c8ef1f64 100644
--- a/profcollectd/libprofcollectd/bindings/libbase/properties.hpp
+++ b/profcollectd/libprofcollectd/bindings/libbase/properties.hpp
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
-// C declaration for bindgen.
-const char* GetProperty(const char*, const char*);
-void SetProperty(const char*, const char*);
+#pragma once
+
+#include "rust/cxx.h"
+
+rust::String GetProperty(rust::Str, rust::Str);
+void SetProperty(rust::Str, rust::Str);
diff --git a/profcollectd/libprofcollectd/config.rs b/profcollectd/libprofcollectd/config.rs
index d2850e82..c3ab45cf 100644
--- a/profcollectd/libprofcollectd/config.rs
+++ b/profcollectd/libprofcollectd/config.rs
@@ -119,7 +119,7 @@ where
     T::Err: Error + Send + Sync + 'static,
 {
     let default_value = default_value.to_string();
-    let value = profcollect_libbase_rust::get_property(&key, &default_value);
+    let value = profcollect_libbase_rust::GetProperty(&key, &default_value);
     Ok(T::from_str(&value)?)
 }
 
@@ -128,7 +128,7 @@ where
     T: ToString,
 {
     let value = value.to_string();
-    profcollect_libbase_rust::set_property(&key, &value);
+    profcollect_libbase_rust::SetProperty(&key, &value);
 }
 
 fn generate_random_node_id() -> MacAddr6 {
diff --git a/profcollectd/libprofcollectd/lib.rs b/profcollectd/libprofcollectd/lib.rs
index aacbf8e3..f417bd8c 100644
--- a/profcollectd/libprofcollectd/lib.rs
+++ b/profcollectd/libprofcollectd/lib.rs
@@ -23,6 +23,9 @@ mod service;
 mod simpleperf_etm_trace_provider;
 mod trace_provider;
 
+#[cfg(feature = "test")]
+mod logging_trace_provider;
+
 use anyhow::{Context, Result};
 use profcollectd_aidl_interface::aidl::com::android::server::profcollect::IProfCollectd::{
     self, BnProfCollectd,
@@ -90,9 +93,8 @@ pub fn report() -> Result<String> {
 
 /// Inits logging for Android
 pub fn init_logging() {
+    let min_log_level = if cfg!(feature = "test") { log::Level::Info } else { log::Level::Error };
     android_logger::init_once(
-        android_logger::Config::default()
-            .with_tag("profcollectd")
-            .with_min_level(log::Level::Error),
+        android_logger::Config::default().with_tag("profcollectd").with_min_level(min_log_level),
     );
 }
diff --git a/profcollectd/libprofcollectd/logging_trace_provider.rs b/profcollectd/libprofcollectd/logging_trace_provider.rs
new file mode 100644
index 00000000..1325855d
--- /dev/null
+++ b/profcollectd/libprofcollectd/logging_trace_provider.rs
@@ -0,0 +1,56 @@
+//
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+//! Logging trace provider for development and testing purposes.
+
+use anyhow::Result;
+use std::path::Path;
+use std::time::Duration;
+use trace_provider::TraceProvider;
+
+use crate::trace_provider;
+
+static LOGGING_TRACEFILE_EXTENSION: &str = "loggingtrace";
+
+pub struct LoggingTraceProvider {}
+
+impl TraceProvider for LoggingTraceProvider {
+    fn get_name(&self) -> &'static str {
+        "logging"
+    }
+
+    fn trace(&self, trace_dir: &Path, tag: &str, sampling_period: &Duration) {
+        let trace_file = trace_provider::get_path(trace_dir, tag, LOGGING_TRACEFILE_EXTENSION);
+
+        log::info!(
+            "Trace event triggered, tag {}, sampling for {}ms, saving to {}",
+            tag,
+            sampling_period.as_millis(),
+            trace_file.display()
+        );
+    }
+
+    fn process(&self, _trace_dir: &Path, _profile_dir: &Path) -> Result<()> {
+        log::info!("Process event triggered");
+        Ok(())
+    }
+}
+
+impl LoggingTraceProvider {
+    pub fn supported() -> bool {
+        true
+    }
+}
diff --git a/profcollectd/libprofcollectd/simpleperf_etm_trace_provider.rs b/profcollectd/libprofcollectd/simpleperf_etm_trace_provider.rs
index 2038a5be..d3baaa36 100644
--- a/profcollectd/libprofcollectd/simpleperf_etm_trace_provider.rs
+++ b/profcollectd/libprofcollectd/simpleperf_etm_trace_provider.rs
@@ -35,12 +35,10 @@ impl TraceProvider for SimpleperfEtmTraceProvider {
     }
 
     fn trace(&self, trace_dir: &Path, tag: &str, sampling_period: &Duration) {
-        let mut trace_file = PathBuf::from(trace_dir);
-        trace_file.push(trace_provider::construct_file_name(tag));
-        trace_file.set_extension(ETM_TRACEFILE_EXTENSION);
+        let trace_file = trace_provider::get_path(trace_dir, tag, ETM_TRACEFILE_EXTENSION);
 
         simpleperf_profcollect::record(
-            &trace_file,
+            &*trace_file,
             sampling_period,
             simpleperf_profcollect::RecordScope::BOTH,
         );
diff --git a/profcollectd/libprofcollectd/trace_provider.rs b/profcollectd/libprofcollectd/trace_provider.rs
index ed0d56f5..44d4cee9 100644
--- a/profcollectd/libprofcollectd/trace_provider.rs
+++ b/profcollectd/libprofcollectd/trace_provider.rs
@@ -18,12 +18,15 @@
 
 use anyhow::{anyhow, Result};
 use chrono::Utc;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 use std::time::Duration;
 
 use crate::simpleperf_etm_trace_provider::SimpleperfEtmTraceProvider;
 
+#[cfg(feature = "test")]
+use crate::logging_trace_provider::LoggingTraceProvider;
+
 pub trait TraceProvider {
     fn get_name(&self) -> &'static str;
     fn trace(&self, trace_dir: &Path, tag: &str, sampling_period: &Duration);
@@ -36,9 +39,19 @@ pub fn get_trace_provider() -> Result<Arc<Mutex<dyn TraceProvider + Send>>> {
         return Ok(Arc::new(Mutex::new(SimpleperfEtmTraceProvider {})));
     }
 
+    #[cfg(feature = "test")]
+    if LoggingTraceProvider::supported() {
+        log::info!("logging trace provider registered.");
+        return Ok(Arc::new(Mutex::new(LoggingTraceProvider {})));
+    }
+
     Err(anyhow!("No trace provider found for this device."))
 }
 
-pub fn construct_file_name(tag: &str) -> String {
-    format!("{}_{}", Utc::now().format("%Y%m%d-%H%M%S"), tag)
+pub fn get_path(dir: &Path, tag: &str, ext: &str) -> Box<Path> {
+    let filename = format!("{}_{}", Utc::now().format("%Y%m%d-%H%M%S"), tag);
+    let mut trace_file = PathBuf::from(dir);
+    trace_file.push(filename);
+    trace_file.set_extension(ext);
+    trace_file.into_boxed_path()
 }
diff --git a/simpleperf/cmd_dumprecord.cpp b/simpleperf/cmd_dumprecord.cpp
index 9a5586bb..142ae5cb 100644
--- a/simpleperf/cmd_dumprecord.cpp
+++ b/simpleperf/cmd_dumprecord.cpp
@@ -284,10 +284,6 @@ void DumpRecordCommand::DumpFileHeader() {
                   << "doesn't match expected header size " << sizeof(header);
   }
   printf("attr_size: %" PRId64 "\n", header.attr_size);
-  if (header.attr_size != sizeof(FileAttr)) {
-    LOG(WARNING) << "record file attr size " << header.attr_size
-                 << " doesn't match expected attr size " << sizeof(FileAttr);
-  }
   printf("attrs[file section]: offset %" PRId64 ", size %" PRId64 "\n", header.attrs.offset,
          header.attrs.size);
   printf("data[file section]: offset %" PRId64 ", size %" PRId64 "\n", header.data.offset,
diff --git a/simpleperf/cmd_dumprecord_test.cpp b/simpleperf/cmd_dumprecord_test.cpp
index e6dc5c65..e6cc5e23 100644
--- a/simpleperf/cmd_dumprecord_test.cpp
+++ b/simpleperf/cmd_dumprecord_test.cpp
@@ -77,3 +77,7 @@ TEST(cmd_dump, etm_data) {
   // Check if we can decode etm data into instruction range elements.
   ASSERT_NE(data.find("OCSD_GEN_TRC_ELEM_INSTR_RANGE"), std::string::npos);
 }
+
+TEST(cmd_dump, dump_arm_regs_recorded_in_arm64) {
+  ASSERT_TRUE(DumpCmd()->Run({GetTestData("perf_with_arm_regs.data")}));
+}
diff --git a/simpleperf/perf_regs.cpp b/simpleperf/perf_regs.cpp
index cccd136f..59db45f6 100644
--- a/simpleperf/perf_regs.cpp
+++ b/simpleperf/perf_regs.cpp
@@ -144,9 +144,10 @@ std::string GetRegName(size_t regno, ArchType arch) {
       if (reg >= PERF_REG_ARM_R0 && reg <= PERF_REG_ARM_R10) {
         return android::base::StringPrintf("r%d", reg - PERF_REG_ARM_R0);
       }
-      auto it = arm_reg_map.find(reg);
-      CHECK(it != arm_reg_map.end()) << "unknown reg " << reg;
-      return it->second;
+      if (auto it = arm_reg_map.find(reg); it != arm_reg_map.end()) {
+        return it->second;
+      }
+      FALLTHROUGH_INTENDED;
     }
     case ARCH_ARM64: {
       if (reg >= PERF_REG_ARM64_X0 && reg <= PERF_REG_ARM64_X29) {
diff --git a/simpleperf/record_file.h b/simpleperf/record_file.h
index 8733c277..9c1a22ff 100644
--- a/simpleperf/record_file.h
+++ b/simpleperf/record_file.h
@@ -50,6 +50,16 @@ struct FileFeature {
 
   FileFeature() {}
 
+  void Clear() {
+    path.clear();
+    type = DSO_UNKNOWN_FILE;
+    min_vaddr = 0;
+    file_offset_of_min_vaddr = 0;
+    symbols.clear();
+    symbol_ptrs.clear();
+    dex_file_offsets.clear();
+  }
+
   DISALLOW_COPY_AND_ASSIGN(FileFeature);
 };
 
diff --git a/simpleperf/record_file_reader.cpp b/simpleperf/record_file_reader.cpp
index 22277d40..5bf80cab 100644
--- a/simpleperf/record_file_reader.cpp
+++ b/simpleperf/record_file_reader.cpp
@@ -488,6 +488,7 @@ std::vector<uint64_t> RecordFileReader::ReadAuxTraceFeature() {
 }
 
 bool RecordFileReader::ReadFileFeature(size_t& read_pos, FileFeature* file) {
+  file->Clear();
   if (HasFeature(FEAT_FILE)) {
     return ReadFileV1Feature(read_pos, file);
   }
@@ -534,7 +535,6 @@ bool RecordFileReader::ReadFileV1Feature(size_t& read_pos, FileFeature* file) {
   MoveFromBinaryFormat(file->min_vaddr, p);
   uint32_t symbol_count;
   MoveFromBinaryFormat(symbol_count, p);
-  file->symbols.clear();
   file->symbols.reserve(symbol_count);
   for (uint32_t i = 0; i < symbol_count; ++i) {
     uint64_t start_vaddr;
@@ -545,7 +545,6 @@ bool RecordFileReader::ReadFileV1Feature(size_t& read_pos, FileFeature* file) {
     p += name.size() + 1;
     file->symbols.emplace_back(name, start_vaddr, len);
   }
-  file->dex_file_offsets.clear();
   if (file->type == DSO_DEX_FILE) {
     uint32_t offset_count;
     MoveFromBinaryFormat(offset_count, p);
@@ -592,7 +591,6 @@ bool RecordFileReader::ReadFileV2Feature(size_t& read_pos, FileFeature* file) {
   file->path = proto_file.path();
   file->type = static_cast<DsoType>(proto_file.type());
   file->min_vaddr = proto_file.min_vaddr();
-  file->symbols.clear();
   file->symbols.reserve(proto_file.symbol_size());
   for (size_t i = 0; i < proto_file.symbol_size(); i++) {
     const auto& proto_symbol = proto_file.symbol(i);
diff --git a/simpleperf/record_file_test.cpp b/simpleperf/record_file_test.cpp
index 31c942c2..bf0436ad 100644
--- a/simpleperf/record_file_test.cpp
+++ b/simpleperf/record_file_test.cpp
@@ -246,8 +246,10 @@ TEST_F(RecordFileTest, write_file2_feature_section) {
     if (file.type == DSO_DEX_FILE) {
       ASSERT_EQ(file.dex_file_offsets, expected_file.dex_file_offsets);
     } else if (file.type == DSO_ELF_FILE) {
+      ASSERT_TRUE(file.dex_file_offsets.empty());
       ASSERT_EQ(file.file_offset_of_min_vaddr, expected_file.file_offset_of_min_vaddr);
     } else if (file.type == DSO_KERNEL_MODULE) {
+      ASSERT_TRUE(file.dex_file_offsets.empty());
       ASSERT_EQ(file.file_offset_of_min_vaddr, expected_file.file_offset_of_min_vaddr);
     }
   }
diff --git a/simpleperf/testdata/perf_with_arm_regs.data b/simpleperf/testdata/perf_with_arm_regs.data
new file mode 100644
index 00000000..b12ac506
--- /dev/null
+++ b/simpleperf/testdata/perf_with_arm_regs.data