Merge "dumpstate: notes on usage" into main (cbd17640) · Commits · e / devices / smdk4412 / android_frameworks_native

cmds/dumpstate/Android.bp

+7 −5

Original line number	Diff line number	Diff line
		@@ -83,14 +83,16 @@ cc_defaults {
		"aconfig_lib_cc_static_link.defaults",
		"dumpstate_cflag_defaults",
		],
		// See README.md: "Dumpstate philosophy: exec not link"
		// Do not add things here - keep dumpstate as simple as possible and exec where possible.
		shared_libs: [
		"android.hardware.dumpstate@1.0",
		"android.hardware.dumpstate@1.1",
		"android.hardware.dumpstate-V1-ndk",
		"libziparchive",
		"libbase",
		"libbinder",
		"libbinder_ndk",
		"libbinder", // BAD: dumpstate should not link code directly, should only exec binaries
		"libbinder_ndk", // BAD: dumpstate should not link code directly, should only exec binaries
		"libcrypto",
		"libcutils",
		"libdebuggerd_client",
		@@ -98,11 +100,11 @@ cc_defaults {
		"libdumpstateutil",
		"libdumputils",
		"libhardware_legacy",
		"libhidlbase",
		"libhidlbase", // BAD: dumpstate should not link code directly, should only exec binaries
		"liblog",
		"libutils",
		"libvintf",
		"libbinderdebug",
		"libvintf", // BAD: dumpstate should not link code directly, should only exec binaries
		"libbinderdebug", // BAD: dumpstate should not link code directly, should only exec binaries
		"packagemanager_aidl-cpp",
		"server_configurable_flags",
		"device_policy_aconfig_flags_c_lib",

cmds/dumpstate/README.md

+12 −0

Original line number	Diff line number	Diff line
		@@ -21,6 +21,18 @@ Example:
		mmm -j frameworks/native/cmds/dumpstate device/acme/secret_device/dumpstate/ hardware/interfaces/dumpstate
		```

		## Dumpstate philosophy: exec not link

		Never link code directly into dumpstate. Dumpstate should execute many
		binaries and collect the results. In general, code should fail hard fail fast,
		but dumpstate is the last to solve many Android bugs. Oftentimes, failures
		in core Android infrastructure or tools are issues that cause problems in
		bugreport directly, so bugreport should not rely on these tools working.
		We want dumpstate to have as minimal of code loaded in process so that
		only that core subset needs to be bugfree for bugreport to work. Even if
		many pieces of Android break, that should not prevent dumpstate from
		working.

		## To build, deploy, and take a bugreport

		```

cmds/dumpstate/dumpstate.cpp

+16 −0

Original line number	Diff line number	Diff line
		@@ -128,6 +128,9 @@ using android::os::dumpstate::PropertiesHelper;
		using android::os::dumpstate::TaskQueue;
		using android::os::dumpstate::WaitForTask;

		// BAD - See README.md: "Dumpstate philosophy: exec not link"
		// Do not add more complicated variables here, prefer to execute only. Don't link more code here.

		// Keep in sync with
		// frameworks/base/services/core/java/com/android/server/am/ActivityManagerService.java
		static const int TRACE_DUMP_TIMEOUT_MS = 10000; // 10 seconds
		@@ -1266,6 +1269,8 @@ static void DumpKernelMemoryAllocations() {
		}
		}

		// BAD - See README.md: "Dumpstate philosophy: exec not link"
		// This should all be moved into a separate binary rather than have complex logic here.
		static Dumpstate::RunStatus RunDumpsysTextByPriority(const std::string& title, int priority,
		std::chrono::milliseconds timeout,
		std::chrono::milliseconds service_timeout) {
		@@ -1346,6 +1351,8 @@ static Dumpstate::RunStatus RunDumpsysTextNormalPriority(const std::string& titl
		service_timeout);
		}

		// BAD - See README.md: "Dumpstate philosophy: exec not link"
		// This should all be moved into a separate binary rather than have complex logic here.
		static Dumpstate::RunStatus RunDumpsysProto(const std::string& title, int priority,
		std::chrono::milliseconds timeout,
		std::chrono::milliseconds service_timeout) {
		@@ -1427,6 +1434,8 @@ static Dumpstate::RunStatus RunDumpsysNormal() {
		* Dumpstate can pick up later and output to the bugreport. Using STDOUT_FILENO
		* if it's not running in the parallel task.
		*/
		// BAD - See README.md: "Dumpstate philosophy: exec not link"
		// This should all be moved into a separate binary rather than have complex logic here.
		static void DumpHals(int out_fd = STDOUT_FILENO) {
		RunCommand("HARDWARE HALS", {"lshal", "--all", "--types=all"},
		CommandOptions::WithTimeout(10).AsRootIfAvailable().Build(),
		@@ -1483,6 +1492,9 @@ static void DumpHals(int out_fd = STDOUT_FILENO) {
		}
		}

		// BAD - See README.md: "Dumpstate philosophy: exec not link"
		// This should all be moved into a separate binary rather than have complex logic here.
		//
		// Dump all of the files that make up the vendor interface.
		// See the files listed in dumpFileList() for the latest list of files.
		static void DumpVintf() {
		@@ -1512,6 +1524,8 @@ static void DumpExternalFragmentationInfo() {
		printf("------ EXTERNAL FRAGMENTATION INFO ------\n");
		std::ifstream ifs("/proc/buddyinfo");
		auto unusable_index_regex = std::regex{"Node\\s+([0-9]+),\\s+zone\\s+(\\S+)\\s+(.*)"};
		// BAD - See README.md: "Dumpstate philosophy: exec not link"
		// This should all be moved into a separate binary rather than have complex logic here.
		for (std::string line; std::getline(ifs, line);) {
		std::smatch match_results;
		if (std::regex_match(line, match_results, unusable_index_regex)) {
		@@ -2452,6 +2466,8 @@ static dumpstate_hal_aidl::IDumpstateDevice::DumpstateMode GetDumpstateHalModeAi
		return dumpstate_hal_aidl::IDumpstateDevice::DumpstateMode::DEFAULT;
		}

		// BAD - See README.md: "Dumpstate philosophy: exec not link"
		// This should all be moved into a separate binary rather than have complex logic here.
		static void DoDumpstateBoardHidl(
		const sp<dumpstate_hal_hidl_1_0::IDumpstateDevice> dumpstate_hal_1_0,
		const std::vector<::ndk::ScopedFileDescriptor>& dumpstate_fds,