Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 77f0c550 authored by Treehugger Robot's avatar Treehugger Robot Committed by Automerger Merge Worker
Browse files

Merge "dumpstate: notes on usage" into main am: cbd17640 

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/3446594



Change-Id: I84be324af663196406a217a87cef2e0353fe3f34
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 771272fc cbd17640

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,