From ddd3da0d8f092a24d2a8a9a1e211d60292b7da35 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Fri, 20 May 2022 03:30:09 +0000 Subject: Add AIDL HAL stack traces to bug-reports Bug: 233130219 Test: adb bugreport; check that camera provider has stack traces in VM traces Ignore-AOSP-First: I will cherry-pick after the topic is submitted. Change-Id: I1e087600ac764191a91eb9b1f11e51f21bce19e3 Signed-off-by: Jayant Chowdhary --- cmds/dumpstate/dumpstate.cpp | 2 +- cmds/dumpsys/tests/dumpsys_test.cpp | 1 + libs/binder/IServiceManager.cpp | 19 +++++++++++ libs/binder/include/binder/IServiceManager.h | 6 ++++ libs/dumputils/Android.bp | 6 +++- libs/dumputils/dump_utils.cpp | 45 ++++++++++++++++++++------- libs/dumputils/include/dumputils/dump_utils.h | 2 +- libs/fakeservicemanager/ServiceManager.cpp | 4 +++ libs/fakeservicemanager/ServiceManager.h | 4 +++ 9 files changed, 75 insertions(+), 14 deletions(-) diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 890c15f895..c5cb0f666c 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -2084,7 +2084,7 @@ Dumpstate::RunStatus Dumpstate::DumpTraces(const char** path) { int timeout_failures = 0; bool dalvik_found = false; - const std::set hal_pids = get_interesting_hal_pids(); + const std::set hal_pids = get_interesting_pids(); struct dirent* d; while ((d = readdir(proc.get()))) { diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp index 49c1318945..f0c19b93ec 100644 --- a/cmds/dumpsys/tests/dumpsys_test.cpp +++ b/cmds/dumpsys/tests/dumpsys_test.cpp @@ -65,6 +65,7 @@ class ServiceManagerMock : public IServiceManager { const sp&)); MOCK_METHOD2(unregisterForNotifications, status_t(const String16&, const sp&)); + MOCK_METHOD0(getServiceDebugInfo, std::vector()); protected: MOCK_METHOD0(onAsBinder, IBinder*()); }; diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index ea2f8d2274..fd2d86857e 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -99,6 +99,8 @@ public: status_t unregisterForNotifications(const String16& service, const sp& cb) override; + + std::vector getServiceDebugInfo() override; // for legacy ABI const String16& getInterfaceDescriptor() const override { return mTheRealServiceManager->getInterfaceDescriptor(); @@ -543,6 +545,23 @@ status_t ServiceManagerShim::unregisterForNotifications(const String16& name, return OK; } +std::vector ServiceManagerShim::getServiceDebugInfo() { + std::vector serviceDebugInfos; + std::vector ret; + if (Status status = mTheRealServiceManager->getServiceDebugInfo(&serviceDebugInfos); + !status.isOk()) { + ALOGW("%s Failed to get ServiceDebugInfo", __FUNCTION__); + return ret; + } + for (const auto& serviceDebugInfo : serviceDebugInfos) { + IServiceManager::ServiceDebugInfo retInfo; + retInfo.pid = serviceDebugInfo.debugPid; + retInfo.name = serviceDebugInfo.name; + ret.emplace_back(retInfo); + } + return ret; +} + #ifndef __ANDROID__ // ServiceManagerShim for host. Implements the old libbinder android::IServiceManager API. // The internal implementation of the AIDL interface android::os::IServiceManager calls into diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h index bb55831ec2..413c97f349 100644 --- a/libs/binder/include/binder/IServiceManager.h +++ b/libs/binder/include/binder/IServiceManager.h @@ -134,6 +134,12 @@ public: virtual status_t unregisterForNotifications(const String16& name, const sp& callback) = 0; + + struct ServiceDebugInfo { + std::string name; + int pid; + }; + virtual std::vector getServiceDebugInfo() = 0; }; sp defaultServiceManager(); diff --git a/libs/dumputils/Android.bp b/libs/dumputils/Android.bp index acda402993..09fbdea0e8 100644 --- a/libs/dumputils/Android.bp +++ b/libs/dumputils/Android.bp @@ -26,6 +26,7 @@ cc_library { shared_libs: [ "libbase", + "libbinder", "libhidlbase", "liblog", "libutils", @@ -33,7 +34,10 @@ cc_library { srcs: ["dump_utils.cpp"], - cflags: ["-Wall", "-Werror"], + cflags: [ + "-Wall", + "-Werror", + ], export_include_dirs: [ "include", diff --git a/libs/dumputils/dump_utils.cpp b/libs/dumputils/dump_utils.cpp index 29c788bca3..0f1a02a553 100644 --- a/libs/dumputils/dump_utils.cpp +++ b/libs/dumputils/dump_utils.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -52,8 +53,8 @@ static const char* debuggable_native_processes_to_dump[] = { NULL, }; -/* list of hal interface to dump containing process during native dumps */ -static const char* hal_interfaces_to_dump[] { +/* list of hidl hal interface to dump containing process during native dumps */ +static const char* hidl_hal_interfaces_to_dump[] { "android.hardware.audio@4.0::IDevicesFactory", "android.hardware.audio@5.0::IDevicesFactory", "android.hardware.audio@6.0::IDevicesFactory", @@ -82,6 +83,11 @@ static const char* hal_interfaces_to_dump[] { NULL, }; +/* list of hal interface to dump containing process during native dumps */ +static const std::vector aidl_interfaces_to_dump { + "android.hardware.camera.provider.ICameraProvider", +}; + /* list of extra hal interfaces to dump containing process during native dumps */ // This is filled when dumpstate is called. static std::set extra_hal_interfaces_to_dump; @@ -104,7 +110,7 @@ static void read_extra_hals_to_dump_from_property() { // check if interface is included in either default hal list or extra hal list bool should_dump_hal_interface(const std::string& interface) { - for (const char** i = hal_interfaces_to_dump; *i; i++) { + for (const char** i = hidl_hal_interfaces_to_dump; *i; i++) { if (interface == *i) { return true; } @@ -130,14 +136,26 @@ bool should_dump_native_traces(const char* path) { return false; } -std::set get_interesting_hal_pids() { +static void get_interesting_aidl_pids(std::set &pids) { + using ServiceDebugInfo = android::IServiceManager::ServiceDebugInfo; + auto sm = android::defaultServiceManager(); + std::vector serviceDebugInfos = sm->getServiceDebugInfo(); + for (const auto & serviceDebugInfo : serviceDebugInfos) { + for (const auto &aidl_prefix : aidl_interfaces_to_dump) { + // Check for prefix match with aidl interface to dump + if (serviceDebugInfo.name.rfind(aidl_prefix, 0) == 0) { + pids.insert(serviceDebugInfo.pid); + } + } + } +} + +static void get_interesting_hidl_pids(std::set &pids) { using android::hidl::manager::V1_0::IServiceManager; using android::sp; using android::hardware::Return; sp manager = IServiceManager::getService(); - std::set pids; - read_extra_hals_to_dump_from_property(); Return ret = manager->debugDump([&](auto& hals) { @@ -146,11 +164,9 @@ std::set get_interesting_hal_pids() { continue; } - if (!should_dump_hal_interface(info.interfaceName)) { - continue; + if (should_dump_hal_interface(info.interfaceName)) { + pids.insert(info.pid); } - - pids.insert(info.pid); } }); @@ -158,7 +174,14 @@ std::set get_interesting_hal_pids() { ALOGE("Could not get list of HAL PIDs: %s\n", ret.description().c_str()); } - return pids; // whether it was okay or not + return; +} + +std::set get_interesting_pids() { + std::set interesting_pids; + get_interesting_hidl_pids(interesting_pids); + get_interesting_aidl_pids(interesting_pids); + return interesting_pids; } bool IsZygote(int pid) { diff --git a/libs/dumputils/include/dumputils/dump_utils.h b/libs/dumputils/include/dumputils/dump_utils.h index 25f712733a..7c5329d01b 100644 --- a/libs/dumputils/include/dumputils/dump_utils.h +++ b/libs/dumputils/include/dumputils/dump_utils.h @@ -21,7 +21,7 @@ bool should_dump_native_traces(const char* path); -std::set get_interesting_hal_pids(); +std::set get_interesting_pids(); bool IsZygote(int pid); diff --git a/libs/fakeservicemanager/ServiceManager.cpp b/libs/fakeservicemanager/ServiceManager.cpp index 61e4a98846..6c6d7f3641 100644 --- a/libs/fakeservicemanager/ServiceManager.cpp +++ b/libs/fakeservicemanager/ServiceManager.cpp @@ -94,4 +94,8 @@ status_t ServiceManager::unregisterForNotifications(const String16&, return INVALID_OPERATION; } +std::vector ServiceManager::getServiceDebugInfo() { + std::vector ret; + return ret; +} } // namespace android diff --git a/libs/fakeservicemanager/ServiceManager.h b/libs/fakeservicemanager/ServiceManager.h index 6d6e008c4f..e0af5d4ba8 100644 --- a/libs/fakeservicemanager/ServiceManager.h +++ b/libs/fakeservicemanager/ServiceManager.h @@ -20,6 +20,7 @@ #include #include +#include namespace android { @@ -58,6 +59,9 @@ public: status_t unregisterForNotifications(const String16& name, const sp& callback) override; + + std::vector getServiceDebugInfo() override; + private: std::map> mNameToService; }; -- cgit v1.2.3-59-g8ed1b From 45913f2d16bf978cba1cc644372c85977de454d6 Mon Sep 17 00:00:00 2001 From: Hsin-Yi Chen Date: Thu, 26 May 2022 14:04:06 +0800 Subject: RESTRICT AUTOMERGE Ignore weak symbol difference in libui ABI check The diff_flags make the ABI checker ignore the weak symbol difference caused by PGO. Test: make libui.vendor Bug: 230076879 Change-Id: I81de5b6109e3980abffcc3ff4aa90b9debd206e3 --- libs/ui/Android.bp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libs/ui/Android.bp b/libs/ui/Android.bp index a9380c6e79..d138495036 100644 --- a/libs/ui/Android.bp +++ b/libs/ui/Android.bp @@ -243,6 +243,10 @@ cc_library_shared { ], afdo: true, + + header_abi_checker: { + diff_flags: ["-allow-adding-removing-weak-symbols"], + }, } cc_library_headers { -- cgit v1.2.3-59-g8ed1b From 006509ad7d6cb5ab1f6a3533dd55db47afd65abb Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 27 May 2022 12:48:37 +0900 Subject: Update dumpstate code that dumps BPF maps. dumpsys netd trafficcontroller no longer does anything useful because that information has moved to the mainline module. Run dumpsys connectivity trafficcontroller instead, which ouputs the same information. Test: none Bug: 234084920 Change-Id: I7b50ffcc62f04c336379fbbd7ea0106a91aac15f (cherry picked from commit 807f2ddf2c9e7d23a8bfdc083fe75fd5ad5ddc98) Merged-In: I7b50ffcc62f04c336379fbbd7ea0106a91aac15f --- cmds/dumpstate/dumpstate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index c5cb0f666c..2b94b71a7f 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1648,7 +1648,7 @@ static Dumpstate::RunStatus dumpstate() { DumpPacketStats(); - RunDumpsys("EBPF MAP STATS", {"netd", "trafficcontroller"}); + RunDumpsys("EBPF MAP STATS", {"connectivity", "trafficcontroller"}); DoKmsg(); -- cgit v1.2.3-59-g8ed1b