From 7605233f146d044959ac125067aefc16ada8ed50 Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Mon, 14 Jun 2021 11:55:31 +0900 Subject: Add ramdisk_available to init_first_stage's deps Bug: 187196593 Test: boot Change-Id: I3b4b8c4758d5e710d3c98dd138b0893a7b320601 Merged-In: I3b4b8c4758d5e710d3c98dd138b0893a7b320601 --- ext4_utils/Android.bp | 1 + libfec/Android.bp | 1 + libfscrypt/Android.bp | 1 + squashfs_utils/Android.bp | 1 + 4 files changed, 4 insertions(+) diff --git a/ext4_utils/Android.bp b/ext4_utils/Android.bp index 10aa765f..80f70b9a 100644 --- a/ext4_utils/Android.bp +++ b/ext4_utils/Android.bp @@ -20,6 +20,7 @@ license { cc_library { name: "libext4_utils", host_supported: true, + ramdisk_available: true, recovery_available: true, srcs: [ "ext4_utils.cpp", diff --git a/libfec/Android.bp b/libfec/Android.bp index 6bd44dd9..0b464b01 100644 --- a/libfec/Android.bp +++ b/libfec/Android.bp @@ -70,6 +70,7 @@ cc_library { name: "libfec", defaults: ["libfec_default"], host_supported: true, + ramdisk_available: true, recovery_available: true, target: { diff --git a/libfscrypt/Android.bp b/libfscrypt/Android.bp index afdc7f4e..343ccef3 100644 --- a/libfscrypt/Android.bp +++ b/libfscrypt/Android.bp @@ -19,6 +19,7 @@ license { cc_library { name: "libfscrypt", + ramdisk_available: true, recovery_available: true, srcs: [ "fscrypt.cpp", diff --git a/squashfs_utils/Android.bp b/squashfs_utils/Android.bp index 4a6fb5fa..b2d493b7 100644 --- a/squashfs_utils/Android.bp +++ b/squashfs_utils/Android.bp @@ -21,6 +21,7 @@ cc_library { name: "libsquashfs_utils", cflags: ["-Werror"], host_supported: true, + ramdisk_available: true, recovery_available: true, srcs: [ "squashfs_utils.c", -- cgit v1.2.3 From 28ab69f68a07eb314017f09c7fb20cd07b4e40e9 Mon Sep 17 00:00:00 2001 From: Pirama Arumuga Nainar Date: Mon, 21 Jun 2021 21:22:38 -0700 Subject: Mark constructor as static Bug: http://b/183606176 Mark the constructor init_profile_extras as static so each library gets its own copy of the constructor. Also get rid of the init_profile_extras_once flag which is unnecessary. The same fix could be applied to profile-extras.cpp used for GCOV but I'm skipping it since we don't officially support gcov and can be cleaned up along with the rest of the build system support. Test: verify that symbols from libjavacrypto.so (e.g. *NativeCrypto*) are written from system_server Change-Id: Ieedbeb609fd63963d76a067c2bc0291af7c04b1c --- toolchain-extras/Android.bp | 3 +-- toolchain-extras/profile-clang-extras.cpp | 21 ++++++--------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/toolchain-extras/Android.bp b/toolchain-extras/Android.bp index 007817ec..220f3e34 100644 --- a/toolchain-extras/Android.bp +++ b/toolchain-extras/Android.bp @@ -129,11 +129,10 @@ cc_test { srcs: [ "profile-clang-extras-test.cpp", ], - static_libs: [ + whole_static_libs: [ "libprofile-clang-extras", ], ldflags: [ - "-uinit_profile_extras", "-Wl,--wrap,open", ], native_coverage: false, diff --git a/toolchain-extras/profile-clang-extras.cpp b/toolchain-extras/profile-clang-extras.cpp index 89c18b2a..bb713e18 100644 --- a/toolchain-extras/profile-clang-extras.cpp +++ b/toolchain-extras/profile-clang-extras.cpp @@ -36,24 +36,15 @@ static void llvm_signal_handler(__unused int signum) { } } -__attribute__((weak)) int init_profile_extras_once = 0; - // Initialize libprofile-extras: -// - Install a signal handler that triggers __llvm_profile_write_file on . -// -// We want this initializer to run during load time. // -// Just marking init_profile_extras() with __attribute__((constructor)) isn't -// enough since the linker drops it from its output since no other symbol from -// this static library is referenced. +// - Install a signal handler that triggers __llvm_profile_write_file on +// . // -// We force the linker to include init_profile_extras() by passing -// '-uinit_profile_extras' to the linker (in build/soong). -__attribute__((constructor)) int init_profile_extras(void) { - if (init_profile_extras_once) - return 0; - init_profile_extras_once = 1; - +// We want this initializer to run during load time. In addition to marking +// this function as a constructor, we link this library with `--whole-archive` +// to force this function to be included in the output. +static __attribute__((constructor)) int init_profile_extras(void) { if (chained_signal_handler != SIG_ERR) { return -1; } -- cgit v1.2.3 From 3c75ec976b63902a83ad7dc3352f26cda60e4ac3 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Tue, 29 Jun 2021 11:55:06 +0000 Subject: Revert "simpleperf: update testdata used for testing reading dex files." This reverts commit 371ed7afc58655c639ab685f2a1bd20ca8c0ca14. Reason for revert: version 002 of vdex has been reverted Bug: 191480616 Bug: 192327398 Change-Id: I360b65111812156a0d95fad33fa59e224cc43276 Merged-In: I360b65111812156a0d95fad33fa59e224cc43276 (cherry picked from commit 3ae1f42b51362b333ab0378bc44522bfbb81e1d2) --- simpleperf/dso_test.cpp | 36 +++++++++++++---------------- simpleperf/get_test_data.h | 28 ---------------------- simpleperf/read_dex_file_test.cpp | 24 ++++++++----------- simpleperf/testdata/base.vdex | Bin 0 -> 2046136 bytes simpleperf/testdata/base_with_cdex_v1.vdex | Bin 2046136 -> 0 bytes simpleperf/testdata/base_with_cdex_v2.vdex | Bin 2626976 -> 0 bytes 6 files changed, 25 insertions(+), 63 deletions(-) create mode 100644 simpleperf/testdata/base.vdex delete mode 100644 simpleperf/testdata/base_with_cdex_v1.vdex delete mode 100644 simpleperf/testdata/base_with_cdex_v2.vdex diff --git a/simpleperf/dso_test.cpp b/simpleperf/dso_test.cpp index c8618036..02958ce6 100644 --- a/simpleperf/dso_test.cpp +++ b/simpleperf/dso_test.cpp @@ -152,28 +152,24 @@ TEST(DebugElfFileFinder, build_id_mismatch) { TEST(dso, dex_file_dso) { #if defined(__linux__) for (DsoType dso_type : {DSO_DEX_FILE, DSO_ELF_FILE}) { - for (const DexFileTestData& entry : dex_file_test_data) { - if (entry.filename == "base_with_cdex_v1.vdex") { - continue; // TODO: reenable it. - } - std::unique_ptr dso = Dso::CreateDso(dso_type, GetTestData(entry.filename)); - ASSERT_TRUE(dso); - dso->AddDexFileOffset(entry.dexfile_offset); - ASSERT_EQ(DSO_DEX_FILE, dso->type()); - const Symbol* symbol = dso->FindSymbol(entry.symbol_addr); - ASSERT_NE(symbol, nullptr); - ASSERT_EQ(symbol->addr, entry.symbol_addr); - ASSERT_EQ(symbol->len, entry.symbol_len); - ASSERT_STREQ(symbol->DemangledName(), entry.symbol_name.c_str()); - uint64_t min_vaddr; - uint64_t file_offset_of_min_vaddr; - dso->GetMinExecutableVaddr(&min_vaddr, &file_offset_of_min_vaddr); - ASSERT_EQ(min_vaddr, 0); - ASSERT_EQ(file_offset_of_min_vaddr, 0); - } + std::unique_ptr dso = Dso::CreateDso(dso_type, GetTestData("base.vdex")); + ASSERT_TRUE(dso); + dso->AddDexFileOffset(0x28); + ASSERT_EQ(DSO_DEX_FILE, dso->type()); + const Symbol* symbol = dso->FindSymbol(0x6c77e); + ASSERT_NE(symbol, nullptr); + ASSERT_EQ(symbol->addr, static_cast(0x6c77e)); + ASSERT_EQ(symbol->len, static_cast(0x16)); + ASSERT_STREQ(symbol->DemangledName(), + "com.example.simpleperf.simpleperfexamplewithnative.MixActivity$1.run"); + uint64_t min_vaddr; + uint64_t file_offset_of_min_vaddr; + dso->GetMinExecutableVaddr(&min_vaddr, &file_offset_of_min_vaddr); + ASSERT_EQ(min_vaddr, 0); + ASSERT_EQ(file_offset_of_min_vaddr, 0); // Don't crash on not exist zip entry. - std::unique_ptr dso = Dso::CreateDso(dso_type, GetTestData("base.zip!/not_exist_entry")); + dso = Dso::CreateDso(dso_type, GetTestData("base.zip!/not_exist_entry")); ASSERT_TRUE(dso); ASSERT_EQ(nullptr, dso->FindSymbol(0)); } diff --git a/simpleperf/get_test_data.h b/simpleperf/get_test_data.h index b6a7999e..1ef663b9 100644 --- a/simpleperf/get_test_data.h +++ b/simpleperf/get_test_data.h @@ -149,32 +149,4 @@ static const std::string PERF_DATA_WITH_IP_ZERO_IN_CALLCHAIN = // generated by `simpleperf record -e cs-etm:u ./etm_test_loop` static const std::string PERF_DATA_ETM_TEST_LOOP = "etm/perf.data"; -struct DexFileTestData { - std::string filename; - uint64_t dexfile_offset; - size_t symbol_count; - // One symbol in the dex file. - uint64_t symbol_addr; - uint64_t symbol_len; - std::string symbol_name; -}; - -static DexFileTestData dex_file_test_data[] = { - DexFileTestData{ - "base_with_cdex_v1.vdex", - 0x28, - 12435, - 0x6c77e, - 0x16, - "com.example.simpleperf.simpleperfexamplewithnative.MixActivity$1.run", - }, - DexFileTestData{ - "base_with_cdex_v2.vdex", - 0x40, - 16891, - 0x169b84, - 0x25a, - "com.android.calculator2.Calculator.onButtonClick", - }}; - #endif // SIMPLE_PERF_GET_TEST_DATA_H_ diff --git a/simpleperf/read_dex_file_test.cpp b/simpleperf/read_dex_file_test.cpp index fc72ceda..843a964c 100644 --- a/simpleperf/read_dex_file_test.cpp +++ b/simpleperf/read_dex_file_test.cpp @@ -32,18 +32,12 @@ TEST(read_dex_file, smoke) { auto symbol_callback = [&](DexFileSymbol* symbol) { symbols.emplace_back(symbol->name, symbol->addr, symbol->size); }; - for (DexFileTestData& entry : dex_file_test_data) { - if (entry.filename == "base_with_cdex_v1.vdex") { - continue; // TODO: reenable it. - } - ASSERT_TRUE(ReadSymbolsFromDexFile(GetTestData(entry.filename), {entry.dexfile_offset}, - symbol_callback)); - ASSERT_EQ(entry.symbol_count, symbols.size()); - auto it = std::find_if(symbols.begin(), symbols.end(), - [&](const Symbol& symbol) { return symbol.addr == entry.symbol_addr; }); - ASSERT_NE(it, symbols.end()); - ASSERT_EQ(it->addr, entry.symbol_addr); - ASSERT_EQ(it->len, entry.symbol_len); - ASSERT_STREQ(it->Name(), entry.symbol_name.c_str()); - } -} \ No newline at end of file + ASSERT_TRUE(ReadSymbolsFromDexFile(GetTestData("base.vdex"), {0x28}, symbol_callback)); + ASSERT_EQ(12435u, symbols.size()); + 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"); +} diff --git a/simpleperf/testdata/base.vdex b/simpleperf/testdata/base.vdex new file mode 100644 index 00000000..b0ea0184 Binary files /dev/null and b/simpleperf/testdata/base.vdex differ diff --git a/simpleperf/testdata/base_with_cdex_v1.vdex b/simpleperf/testdata/base_with_cdex_v1.vdex deleted file mode 100644 index b0ea0184..00000000 Binary files a/simpleperf/testdata/base_with_cdex_v1.vdex and /dev/null differ diff --git a/simpleperf/testdata/base_with_cdex_v2.vdex b/simpleperf/testdata/base_with_cdex_v2.vdex deleted file mode 100644 index 16ecc6fd..00000000 Binary files a/simpleperf/testdata/base_with_cdex_v2.vdex and /dev/null differ -- cgit v1.2.3 From ecadb8ebedfdcdab617ffb5aad65a2621a023188 Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Thu, 1 Jul 2021 06:50:40 +0000 Subject: Revert "Add ramdisk_available to init_first_stage's deps" Revert submission 15071196-init_first_stage_soong Reason for revert: fixes b/192248690 Reverted Changes: I23cf4f975:Add ramdisk_available to init_first_stage's deps Icd98c7e24:Add ramdisk_available to init_first_stage's deps If9da9ba16:Add ramdisk_available to init_first_stage's deps Ibc8668029:Add ramdisk_available to init_first_stage's deps I3b4b8c475:Add ramdisk_available to init_first_stage's deps I59cd149e0:Completely migrate init first stage to Soong I36d789578:Add ramdisk_available to init_first_stage's deps I2a0daa612:Add BUILD_USES_RECOVERY_AS_BOOT to soong config Ic76c325ce:Directly create ramdisk dirs in ramdisk image rule... I4c5374deb:Add BOARD_BUILD_SYSTEM_ROOT_IMAGE to config vars I8aab5faf3:Add ramdisk_available to init_first_stage's deps I9d5a10661:Add ramdisk_available to init_first_stage's deps Iaa2edeb4a:Add ramdisk_available to init_first_stage's deps I7cb582ca0:Update init_first_stage I06091d15e:Add ramdisk_available to init_first_stage's deps I8bdb8dda3:Add ramdisk_available to init_first_stage's deps I7436b8dd1:Add ramdisk_available to init_first_stage's deps I39693fd86:Add ramdisk_available to init_first_stage's deps I0a9ba90f0:Add ramdisk_available to init_first_stage's deps Ib66b4c4ea:Add ramdisk_available to init_first_stage's deps I31ce63d23:Add ramdisk_available to init_first_stage's deps Icb580f97c:Add ramdisk_available to init_first_stage's deps I044a075b7:Add ramdisk_available to init_first_stage's deps I33164a7e7:Fix ndk and aml arch order Ib8d92904a:Add ramdisk_available to sysprop_library Ibc3516453:Add install_in_root to cc_binary Change-Id: I50e4ecb6b4c5196955f7f97f887cdca352b4f56f --- ext4_utils/Android.bp | 1 - libfec/Android.bp | 1 - libfscrypt/Android.bp | 1 - squashfs_utils/Android.bp | 1 - 4 files changed, 4 deletions(-) diff --git a/ext4_utils/Android.bp b/ext4_utils/Android.bp index 80f70b9a..10aa765f 100644 --- a/ext4_utils/Android.bp +++ b/ext4_utils/Android.bp @@ -20,7 +20,6 @@ license { cc_library { name: "libext4_utils", host_supported: true, - ramdisk_available: true, recovery_available: true, srcs: [ "ext4_utils.cpp", diff --git a/libfec/Android.bp b/libfec/Android.bp index 0b464b01..6bd44dd9 100644 --- a/libfec/Android.bp +++ b/libfec/Android.bp @@ -70,7 +70,6 @@ cc_library { name: "libfec", defaults: ["libfec_default"], host_supported: true, - ramdisk_available: true, recovery_available: true, target: { diff --git a/libfscrypt/Android.bp b/libfscrypt/Android.bp index 343ccef3..afdc7f4e 100644 --- a/libfscrypt/Android.bp +++ b/libfscrypt/Android.bp @@ -19,7 +19,6 @@ license { cc_library { name: "libfscrypt", - ramdisk_available: true, recovery_available: true, srcs: [ "fscrypt.cpp", diff --git a/squashfs_utils/Android.bp b/squashfs_utils/Android.bp index b2d493b7..4a6fb5fa 100644 --- a/squashfs_utils/Android.bp +++ b/squashfs_utils/Android.bp @@ -21,7 +21,6 @@ cc_library { name: "libsquashfs_utils", cflags: ["-Werror"], host_supported: true, - ramdisk_available: true, recovery_available: true, srcs: [ "squashfs_utils.c", -- cgit v1.2.3 From 29b4145bff8bc65f46c3a830f0f6cb5c6422ff80 Mon Sep 17 00:00:00 2001 From: Joel Galenson Date: Mon, 12 Jul 2021 14:18:53 -0700 Subject: Migrate profcollectd's libbase bindings from bindgen to cxx. This should both simplify the code and fix some errors in the existing implementation. Bug: 182498247 Test: Build Change-Id: Id3f7f518922e745715aef1286be684bd3e9c16fe Merged-In: Id3f7f518922e745715aef1286be684bd3e9c16fe (cherry picked from commit 93dd9936881424d2ed4f061bd7d06f8d74163010) --- .../libprofcollectd/bindings/libbase/Android.bp | 17 +++++++----- .../libprofcollectd/bindings/libbase/lib.rs | 31 +++++++++------------- .../bindings/libbase/properties.cpp | 7 +++-- .../bindings/libbase/properties.hpp | 9 ++++--- profcollectd/libprofcollectd/config.rs | 4 +-- 5 files changed, 35 insertions(+), 33 deletions(-) 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 { -- cgit v1.2.3 From 1d2f2761cd1770bc040596e706335b74572e76de Mon Sep 17 00:00:00 2001 From: Yi Kong Date: Tue, 20 Jul 2021 16:55:39 +0800 Subject: profcollectd: Remove reports past retention period Manually generated profcollect reports (through `profcollectctl report`) and reports that failed to upload for whatever reason need to be cleaned up periodically, to save disk space as well as to minimise privacy leak surface. Test: manual Bug: 178561556 Change-Id: I54f09d1738f7b3c3b763251af83133bda5c214ae Merged-In: I54f09d1738f7b3c3b763251af83133bda5c214ae (cherry picked from commit 8dffc1208417a18870b33ce92c03a8f77da8dbee) --- profcollectd/libprofcollectd/config.rs | 2 ++ profcollectd/libprofcollectd/report.rs | 11 ++++++++++- profcollectd/libprofcollectd/service.rs | 31 ++++++++++++++++++++++++++++--- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/profcollectd/libprofcollectd/config.rs b/profcollectd/libprofcollectd/config.rs index c3ab45cf..11098fe1 100644 --- a/profcollectd/libprofcollectd/config.rs +++ b/profcollectd/libprofcollectd/config.rs @@ -29,6 +29,8 @@ use std::time::Duration; const PROFCOLLECT_CONFIG_NAMESPACE: &str = "profcollect_native_boot"; const PROFCOLLECT_NODE_ID_PROPERTY: &str = "persist.profcollectd.node_id"; +pub const REPORT_RETENTION_SECS: u64 = 14 * 24 * 60 * 60; // 14 days. + lazy_static! { pub static ref TRACE_OUTPUT_DIR: &'static Path = Path::new("/data/misc/profcollectd/trace/"); pub static ref PROFILE_OUTPUT_DIR: &'static Path = Path::new("/data/misc/profcollectd/output/"); diff --git a/profcollectd/libprofcollectd/report.rs b/profcollectd/libprofcollectd/report.rs index c37993b7..a67d500b 100644 --- a/profcollectd/libprofcollectd/report.rs +++ b/profcollectd/libprofcollectd/report.rs @@ -23,7 +23,7 @@ use std::fs::{self, File, Permissions}; use std::io::{Read, Write}; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; -use std::time::SystemTime; +use std::time::{Duration, SystemTime}; use uuid::v1::{Context, Timestamp}; use uuid::Uuid; use zip::write::FileOptions; @@ -82,3 +82,12 @@ fn get_report_filename(node_id: &MacAddr6) -> Result { let uuid = Uuid::new_v1(ts, &node_id.as_bytes())?; Ok(uuid.to_string()) } + +/// Get report creation timestamp through its filename (version 1 UUID). +pub fn get_report_ts(filename: &str) -> Result { + let uuid_ts = Uuid::parse_str(filename)? + .to_timestamp() + .ok_or_else(|| anyhow!("filename is not a valid V1 UUID."))? + .to_unix(); + Ok(SystemTime::UNIX_EPOCH + Duration::new(uuid_ts.0, uuid_ts.1)) +} diff --git a/profcollectd/libprofcollectd/service.rs b/profcollectd/libprofcollectd/service.rs index 0b9cca1e..04f30f89 100644 --- a/profcollectd/libprofcollectd/service.rs +++ b/profcollectd/libprofcollectd/service.rs @@ -21,16 +21,17 @@ 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::{copy, create_dir, read_to_string, remove_dir_all, remove_file, write}; +use std::fs::{copy, create_dir, read_dir, read_to_string, remove_dir_all, remove_file, write}; use std::path::PathBuf; use std::str::FromStr; use std::sync::{Mutex, MutexGuard}; +use std::time::Duration; use crate::config::{ Config, BETTERBUG_CACHE_DIR_PREFIX, BETTERBUG_CACHE_DIR_SUFFIX, CONFIG_FILE, - PROFILE_OUTPUT_DIR, REPORT_OUTPUT_DIR, TRACE_OUTPUT_DIR, + PROFILE_OUTPUT_DIR, REPORT_OUTPUT_DIR, REPORT_RETENTION_SECS, TRACE_OUTPUT_DIR, }; -use crate::report::pack_report; +use crate::report::{get_report_ts, pack_report}; use crate::scheduler::Scheduler; fn err_to_binder_status(msg: Error) -> Status { @@ -155,6 +156,30 @@ impl ProfcollectdBinderService { write(*CONFIG_FILE, &new_config.to_string())?; } + // Clear profile reports out of rentention period. + for report in read_dir(*REPORT_OUTPUT_DIR)? { + let report = report?.path(); + let report_name = report + .file_stem() + .and_then(|f| f.to_str()) + .ok_or_else(|| anyhow!("Malformed path {}", report.display()))?; + let report_ts = get_report_ts(report_name); + if let Err(e) = report_ts { + log::error!( + "Cannot decode creation timestamp for report {}, caused by {}, deleting", + report_name, + e + ); + remove_file(report)?; + continue; + } + let report_age = report_ts.unwrap().elapsed()?; + if report_age > Duration::from_secs(REPORT_RETENTION_SECS) { + log::info!("Report {} past rentention period, deleting", report_name); + remove_file(report)?; + } + } + Ok(ProfcollectdBinderService { lock: Mutex::new(Lock { scheduler: new_scheduler, config: new_config }), }) -- cgit v1.2.3 From d09092fd6d2632e6e2787af06ad6fdb69099966c Mon Sep 17 00:00:00 2001 From: Joel Galenson Date: Thu, 22 Jul 2021 10:22:28 -0700 Subject: Migrate profcollectd's libflags bindings from bindgen to cxx. This should both simplify the code and fix some errors in the existing implementation. Bug: 182498247 Test: Build Change-Id: I2a5bcbcac407ba0bb41d4884b99fe8a55113ae38 (cherry picked from commit c8fb4177717b50c3c5789011e4ab795771a62102) --- .../libprofcollectd/bindings/libflags/Android.bp | 15 +++++---- .../bindings/libflags/get_flags.cpp | 13 +++----- .../bindings/libflags/get_flags.hpp | 7 ++-- .../libprofcollectd/bindings/libflags/lib.rs | 37 ++++++++++------------ profcollectd/libprofcollectd/config.rs | 2 +- 5 files changed, 36 insertions(+), 38 deletions(-) diff --git a/profcollectd/libprofcollectd/bindings/libflags/Android.bp b/profcollectd/libprofcollectd/bindings/libflags/Android.bp index 102a6c05..fb846678 100644 --- a/profcollectd/libprofcollectd/bindings/libflags/Android.bp +++ b/profcollectd/libprofcollectd/bindings/libflags/Android.bp @@ -26,20 +26,23 @@ package { cc_library_static { name: "libprofcollect_libflags", srcs: ["get_flags.cpp"], + generated_headers: ["cxx-bridge-header"], + generated_sources: ["libprofcollect_libflags_bridge_code"], } -rust_bindgen { - name: "libprofcollect_libflags_bindgen", - wrapper_src: "get_flags.hpp", - crate_name: "profcollect_libflags_bindgen", - source_stem: "bindings", +genrule { + name: "libprofcollect_libflags_bridge_code", + tools: ["cxxbridge"], + cmd: "$(location cxxbridge) $(in) >> $(out)", + srcs: ["lib.rs"], + out: ["libprofcollect_libflags_cxx_generated.cc"], } rust_library { name: "libprofcollect_libflags_rust", crate_name: "profcollect_libflags_rust", srcs: ["lib.rs"], - rlibs: ["libprofcollect_libflags_bindgen"], + rustlibs: ["libcxx"], static_libs: ["libprofcollect_libflags"], shared_libs: [ "libc++", diff --git a/profcollectd/libprofcollectd/bindings/libflags/get_flags.cpp b/profcollectd/libprofcollectd/bindings/libflags/get_flags.cpp index e5ce65a1..d8bab841 100644 --- a/profcollectd/libprofcollectd/bindings/libflags/get_flags.cpp +++ b/profcollectd/libprofcollectd/bindings/libflags/get_flags.cpp @@ -17,12 +17,9 @@ #include "../../../../../server_configurable_flags/libflags/include/server_configurable_flags/get_flags.h" #include "get_flags.hpp" -const char* GetServerConfigurableFlag(const char* experiment_category_name, - const char* experiment_flag_name, - const char* default_value) { - auto v = server_configurable_flags::GetServerConfigurableFlag( - std::string(experiment_category_name), - std::string(experiment_flag_name), - std::string(default_value)); - return strdup(v.c_str()); +rust::String GetServerConfigurableFlag(rust::Str experiment_category_name, + rust::Str experiment_flag_name, rust::Str default_value) { + return server_configurable_flags::GetServerConfigurableFlag(std::string(experiment_category_name), + std::string(experiment_flag_name), + std::string(default_value)); } diff --git a/profcollectd/libprofcollectd/bindings/libflags/get_flags.hpp b/profcollectd/libprofcollectd/bindings/libflags/get_flags.hpp index ab3be989..a42c52e8 100644 --- a/profcollectd/libprofcollectd/bindings/libflags/get_flags.hpp +++ b/profcollectd/libprofcollectd/bindings/libflags/get_flags.hpp @@ -14,5 +14,8 @@ * limitations under the License. */ -// C declaration for bindgen. -const char* GetServerConfigurableFlag(const char*, const char*, const char*); +#pragma once + +#include "rust/cxx.h" + +rust::String GetServerConfigurableFlag(rust::Str, rust::Str, rust::Str); diff --git a/profcollectd/libprofcollectd/bindings/libflags/lib.rs b/profcollectd/libprofcollectd/bindings/libflags/lib.rs index c8875cb1..c6435bbd 100644 --- a/profcollectd/libprofcollectd/bindings/libflags/lib.rs +++ b/profcollectd/libprofcollectd/bindings/libflags/lib.rs @@ -17,27 +17,22 @@ //! This module implements safe wrappers for GetServerConfigurableFlag method //! from libflags. -use std::ffi::{CStr, CString}; +pub use ffi::GetServerConfigurableFlag; -/// Use the category name and flag name registered in SettingsToPropertiesMapper.java -/// to query the experiment flag value. This method will return default_value if -/// querying fails. -/// Note that for flags from Settings.Global, experiment_category_name should -/// always be global_settings. -pub fn get_server_configurable_flag<'a>( - experiment_category_name: &str, - experiment_flag_name: &str, - default_value: &'a str, -) -> &'a str { - let experiment_category_name = CString::new(experiment_category_name).unwrap(); - let experiment_flag_name = CString::new(experiment_flag_name).unwrap(); - let default_value = CString::new(default_value).unwrap(); - unsafe { - let cstr = profcollect_libflags_bindgen::GetServerConfigurableFlag( - experiment_category_name.as_ptr(), - experiment_flag_name.as_ptr(), - default_value.as_ptr(), - ); - CStr::from_ptr(cstr).to_str().unwrap() +#[cxx::bridge] +mod ffi { + unsafe extern "C++" { + include!("get_flags.hpp"); + + /// Use the category name and flag name registered in SettingsToPropertiesMapper.java + /// to query the experiment flag value. This method will return default_value if + /// querying fails. + /// Note that for flags from Settings.Global, experiment_category_name should + /// always be global_settings. + fn GetServerConfigurableFlag( + experiment_category_name: &str, + experiment_flag_name: &str, + default_value: &str, + ) -> String; } } diff --git a/profcollectd/libprofcollectd/config.rs b/profcollectd/libprofcollectd/config.rs index 11098fe1..e8afb37f 100644 --- a/profcollectd/libprofcollectd/config.rs +++ b/profcollectd/libprofcollectd/config.rs @@ -107,7 +107,7 @@ where T::Err: Error + Send + Sync + 'static, { let default_value = default_value.to_string(); - let config = profcollect_libflags_rust::get_server_configurable_flag( + let config = profcollect_libflags_rust::GetServerConfigurableFlag( &PROFCOLLECT_CONFIG_NAMESPACE, &key, &default_value, -- cgit v1.2.3 From ee3097ce79889c5e0bbaec34cda8becebcc9f64f Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Thu, 22 Jul 2021 13:33:24 -0700 Subject: simpleperf: replace InCloudAndroid() with runtime check. On x86 and x86_64, it's likely to run on an emulator or vm without hardware perf counters. It's hard to enumerate them all. So check if hardware perf counters are available at runtime. Bug: 191277482 Test: run simpleperf_unit_test Change-Id: I38cc89bc7f81f8d6d6165a4b8f344c0660c10364 (cherry picked from commit 1f6f51aee77235b1fb374a917f0b46c9c09f0933) --- simpleperf/test_util.cpp | 41 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/simpleperf/test_util.cpp b/simpleperf/test_util.cpp index c2a21960..547eaf90 100644 --- a/simpleperf/test_util.cpp +++ b/simpleperf/test_util.cpp @@ -48,29 +48,6 @@ bool IsInNativeAbi() { return in_native_abi == 1; } -static bool InCloudAndroid() { -#if defined(__i386__) || defined(__x86_64__) -#if defined(__ANDROID__) - std::string prop_value = android::base::GetProperty("ro.build.flavor", ""); - if (android::base::StartsWith(prop_value, "cf_x86_phone") || - android::base::StartsWith(prop_value, "aosp_cf_x86_phone") || - android::base::StartsWith(prop_value, "cf_x86_64_phone") || - android::base::StartsWith(prop_value, "aosp_cf_x86_64_phone")) { - return true; - } - // aosp_x86* builds may also run on cloud Android. Detect it by checking - /// if cpu-cycles isn't supported. - if (android::base::StartsWith(prop_value, "aosp_x86")) { - const simpleperf::EventType* type = simpleperf::FindEventTypeByName("cpu-cycles", false); - CHECK(type != nullptr); - perf_event_attr attr = CreateDefaultPerfEventAttr(*type); - return !IsEventAttrSupported(attr, "cpu-cycles"); - } -#endif -#endif - return false; -} - #if defined(__arm__) // Check if we can get a non-zero instruction event count by monitoring current thread. static bool HasNonZeroInstructionEventCount() { @@ -96,18 +73,28 @@ static bool HasNonZeroInstructionEventCount() { #endif // defined(__arm__) bool HasHardwareCounter() { +#if defined(__linux__) static int has_hw_counter = -1; if (has_hw_counter == -1) { - // Cloud Android doesn't have hardware counters. - has_hw_counter = InCloudAndroid() ? 0 : 1; -#if defined(__arm__) + has_hw_counter = 1; +#if defined(__x86__) || defined(__x86_64__) + // On x86 and x86_64, it's likely to run on an emulator or vm without hardware perf counters. + // It's hard to enumerate them all. So check the support at runtime. + const simpleperf::EventType* type = simpleperf::FindEventTypeByName("cpu-cycles", false); + CHECK(type != nullptr); + perf_event_attr attr = CreateDefaultPerfEventAttr(*type); + has_hw_counter = IsEventAttrSupported(attr, "cpu-cycles") ? 1 : 0; +#elif defined(__arm__) // For arm32 devices, external non-invasive debug signal controls PMU counters. Once it is // disabled for security reason, we always get zero values for PMU counters. And we want to // skip hardware counter tests once we detect it. has_hw_counter &= HasNonZeroInstructionEventCount() ? 1 : 0; -#endif +#endif // defined(__arm__) } return has_hw_counter == 1; +#else // defined(__linux__) + return false; +#endif // defined(__linux__) } bool HasPmuCounter() { -- cgit v1.2.3