From f56bbb2bc0d65190bdb660ab359eb4697e928245 Mon Sep 17 00:00:00 2001 From: mhasank Date: Tue, 26 May 2020 18:02:39 -0700 Subject: [PATCH] arc: Implement smaller dumpstate output Adds a -A commandline option to indicate a much smaller set of dumpstate output safe for ARC++ bug reports. Implements output of system logcat, event logs, networking, and dropbox system server crash/system app crashes only. Bug: 142684959 Bug: 136273873 Test: atest dumpstate_test Change-Id: I192be7ed841cee0a8847e1057209ef2b164bab07 --- cmds/dumpstate/dumpstate.cpp | 68 ++++++++++++++++++++++--- cmds/dumpstate/dumpstate.h | 2 + cmds/dumpstate/main.cpp | 2 +- cmds/dumpstate/tests/dumpstate_test.cpp | 42 +++++++++++++++ 4 files changed, 106 insertions(+), 8 deletions(-) diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 9ba4819ee8..ba8dc46596 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -1362,6 +1362,53 @@ static void DumpExternalFragmentationInfo() { printf("\n"); } +static void DumpstateArcOnly() { + // Trimmed-down version of dumpstate to only include a whitelisted + // set of logs (system log, event log, and system server / system app + // crashes, and ARC networking logs). See b/136273873 and b/138459828 + // for context. New sections must be first approved by Chrome OS Privacy + // and then added to server side cros monitoring PII scrubber before adding + // them here. See cl/312126645 for an example. + DurationReporter duration_reporter("DUMPSTATE"); + unsigned long timeout_ms; + // calculate timeout + timeout_ms = logcat_timeout({"main", "system", "crash"}); + RunCommand("SYSTEM LOG", + {"logcat", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); + timeout_ms = logcat_timeout({"events"}); + RunCommand( + "EVENT LOG", + {"logcat", "-b", "events", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"}, + CommandOptions::WithTimeoutInMs(timeout_ms).Build()); + + printf("========================================================\n"); + printf("== Networking Service\n"); + printf("========================================================\n"); + + // ARC networking service implements dumpsys by reusing the 'wifi' service name. + // The top-level handler is implemented in handleDump() in + // vendor/google_arc/libs/arc-services/src/com/android/server/arc/net/ArcNetworkService.java. + // It outputs a subset of Android system server state relevant for debugging ARC + // connectivity issues, in a PII-free manner. See b/147270970. + RunDumpsys("DUMPSYS NETWORK_SERVICE_LIMITED", {"wifi", "-a"}, + CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10)); + + printf("========================================================\n"); + printf("== Dropbox crashes\n"); + printf("========================================================\n"); + + RunDumpsys("DROPBOX SYSTEM SERVER CRASHES", {"dropbox", "-p", "system_server_crash"}); + RunDumpsys("DROPBOX SYSTEM APP CRASHES", {"dropbox", "-p", "system_app_crash"}); + + printf("========================================================\n"); + printf("== Final progress (pid %d): %d/%d (estimated %d)\n", ds.pid_, ds.progress_->Get(), + ds.progress_->GetMax(), ds.progress_->GetInitialMax()); + printf("========================================================\n"); + printf("== dumpstate: done (id %d)\n", ds.id_); + printf("========================================================\n"); +} + // Dumps various things. Returns early with status USER_CONSENT_DENIED if user denies consent // via the consent they are shown. Ignores other errors that occur while running various // commands. The consent checking is currently done around long running tasks, which happen to @@ -2048,7 +2095,7 @@ void Dumpstate::DumpstateBoard() { static void ShowUsage() { fprintf(stderr, "usage: dumpstate [-h] [-b soundfile] [-e soundfile] [-d] [-p] " - "[-z] [-s] [-S] [-q] [-P] [-R] [-V version]\n" + "[-z] [-s] [-S] [-q] [-P] [-R] [-A] [-V version]\n" " -h: display this help message\n" " -b: play sound file instead of vibrate, at beginning of job\n" " -e: play sound file instead of vibrate, at end of job\n" @@ -2061,6 +2108,7 @@ static void ShowUsage() { " -P: send broadcast when started and do progress updates\n" " -R: take bugreport in remote mode (requires -z and -d, shouldn't be used with -P)\n" " -w: start binder service and make it wait for a call to startBugreport\n" + " -A: output limited information that is safe for submission in ARC++ bugreports\n" " -v: prints the dumpstate header and exit\n"); } @@ -2306,13 +2354,12 @@ static void LogDumpOptions(const Dumpstate::DumpOptions& options) { "do_zip_file: %d do_vibrate: %d use_socket: %d use_control_socket: %d do_screenshot: %d " "is_remote_mode: %d show_header_only: %d do_start_service: %d telephony_only: %d " "wifi_only: %d do_progress_updates: %d fd: %d bugreport_mode: %s dumpstate_hal_mode: %s " - "args: %s\n", + "arc_only: %d args: %s\n", options.do_zip_file, options.do_vibrate, options.use_socket, options.use_control_socket, options.do_screenshot, options.is_remote_mode, options.show_header_only, - options.do_start_service, - options.telephony_only, options.wifi_only, options.do_progress_updates, - options.bugreport_fd.get(), options.bugreport_mode.c_str(), - toString(options.dumpstate_hal_mode).c_str(), options.args.c_str()); + options.do_start_service, options.telephony_only, options.wifi_only, + options.do_progress_updates, options.bugreport_fd.get(), options.bugreport_mode.c_str(), + toString(options.dumpstate_hal_mode).c_str(), options.arc_only, options.args.c_str()); } void Dumpstate::DumpOptions::Initialize(BugreportMode bugreport_mode, @@ -2334,7 +2381,7 @@ void Dumpstate::DumpOptions::Initialize(BugreportMode bugreport_mode, Dumpstate::RunStatus Dumpstate::DumpOptions::Initialize(int argc, char* argv[]) { RunStatus status = RunStatus::OK; int c; - while ((c = getopt(argc, argv, "dho:svqzpPBRSV:w")) != -1) { + while ((c = getopt(argc, argv, "dho:svqzpAPBRSV:w")) != -1) { switch (c) { // clang-format off case 'd': do_add_date = true; break; @@ -2346,6 +2393,7 @@ Dumpstate::RunStatus Dumpstate::DumpOptions::Initialize(int argc, char* argv[]) case 'p': do_screenshot = true; break; case 'P': do_progress_updates = true; break; case 'R': is_remote_mode = true; break; + case 'A': arc_only = true; break; case 'V': break; // compatibility no-op case 'w': // This was already processed @@ -2630,6 +2678,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, // duration is logged into MYLOG instead. PrintHeader(); + // TODO(nandana) reduce code repetition in if branches if (options_->telephony_only) { MaybeTakeEarlyScreenshot(); onUiIntensiveBugreportDumpsFinished(calling_uid, calling_package); @@ -2641,6 +2690,11 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, onUiIntensiveBugreportDumpsFinished(calling_uid, calling_package); MaybeCheckUserConsent(calling_uid, calling_package); DumpstateWifiOnly(); + } else if (options_->arc_only) { + MaybeTakeEarlyScreenshot(); + onUiIntensiveBugreportDumpsFinished(calling_uid, calling_package); + MaybeCheckUserConsent(calling_uid, calling_package); + DumpstateArcOnly(); } else { // Invoke critical dumpsys first to preserve system state, before doing anything else. RunDumpsysCritical(); diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 28d8936828..c8c9f452f0 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -377,6 +377,8 @@ class Dumpstate { bool do_start_service = false; bool telephony_only = false; bool wifi_only = false; + // Trimmed-down version of dumpstate to only include whitelisted logs. + bool arc_only = false; // Whether progress updates should be published. bool do_progress_updates = false; // The mode we'll use when calling IDumpstateDevice::dumpstateBoard. diff --git a/cmds/dumpstate/main.cpp b/cmds/dumpstate/main.cpp index 68d373377c..f1342a5b53 100644 --- a/cmds/dumpstate/main.cpp +++ b/cmds/dumpstate/main.cpp @@ -30,7 +30,7 @@ bool ShouldStartServiceAndWait(int argc, char* argv[]) { bool do_wait = false; int c; // Keep flags in sync with Dumpstate::DumpOptions::Initialize. - while ((c = getopt(argc, argv, "wdho:svqzpPBRSV:")) != -1 && !do_wait) { + while ((c = getopt(argc, argv, "dho:svqzpAPBRSV:w")) != -1 && !do_wait) { switch (c) { case 'w': do_wait = true; diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index 9871a3bd6c..7078521b52 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -179,6 +179,7 @@ TEST_F(DumpOptionsTest, InitializeNone) { EXPECT_FALSE(options_.do_screenshot); EXPECT_FALSE(options_.do_progress_updates); EXPECT_FALSE(options_.is_remote_mode); + EXPECT_FALSE(options_.arc_only); EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT); } @@ -206,6 +207,7 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) { EXPECT_FALSE(options_.do_progress_updates); EXPECT_FALSE(options_.is_remote_mode); EXPECT_FALSE(options_.use_socket); + EXPECT_FALSE(options_.arc_only); EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT); } @@ -231,6 +233,7 @@ TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) { EXPECT_FALSE(options_.do_screenshot); EXPECT_FALSE(options_.do_progress_updates); EXPECT_FALSE(options_.is_remote_mode); + EXPECT_FALSE(options_.arc_only); EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT); } @@ -249,6 +252,7 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) { EXPECT_FALSE(options_.is_remote_mode); EXPECT_FALSE(options_.use_socket); EXPECT_FALSE(options_.do_start_service); + EXPECT_FALSE(options_.arc_only); } TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { @@ -266,6 +270,7 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { EXPECT_FALSE(options_.show_header_only); EXPECT_FALSE(options_.is_remote_mode); EXPECT_FALSE(options_.use_socket); + EXPECT_FALSE(options_.arc_only); } TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { @@ -282,6 +287,7 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { EXPECT_FALSE(options_.show_header_only); EXPECT_FALSE(options_.do_progress_updates); EXPECT_FALSE(options_.use_socket); + EXPECT_FALSE(options_.arc_only); } TEST_F(DumpOptionsTest, InitializeWearBugReport) { @@ -299,6 +305,7 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) { EXPECT_FALSE(options_.show_header_only); EXPECT_FALSE(options_.is_remote_mode); EXPECT_FALSE(options_.use_socket); + EXPECT_FALSE(options_.arc_only); } TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { @@ -316,6 +323,7 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { EXPECT_FALSE(options_.show_header_only); EXPECT_FALSE(options_.is_remote_mode); EXPECT_FALSE(options_.use_socket); + EXPECT_FALSE(options_.arc_only); } TEST_F(DumpOptionsTest, InitializeWifiBugReport) { @@ -333,6 +341,37 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) { EXPECT_FALSE(options_.do_progress_updates); EXPECT_FALSE(options_.is_remote_mode); EXPECT_FALSE(options_.use_socket); + EXPECT_FALSE(options_.arc_only); +} + +TEST_F(DumpOptionsTest, InitializeArcOnlyBugreport) { + // clang-format off + char* argv[] = { + const_cast("dumpstatez"), + const_cast("-S"), + const_cast("-d"), + const_cast("-z"), + const_cast("-q"), + const_cast("-A") + }; + // clang-format on + + Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv); + + EXPECT_EQ(status, Dumpstate::RunStatus::OK); + EXPECT_TRUE(options_.do_add_date); + EXPECT_TRUE(options_.do_zip_file); + EXPECT_TRUE(options_.use_control_socket); + EXPECT_FALSE(options_.do_vibrate); + EXPECT_TRUE(options_.arc_only); + + // Other options retain default values + EXPECT_FALSE(options_.show_header_only); + EXPECT_FALSE(options_.do_screenshot); + EXPECT_FALSE(options_.do_progress_updates); + EXPECT_FALSE(options_.is_remote_mode); + EXPECT_FALSE(options_.use_socket); + EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT); } TEST_F(DumpOptionsTest, InitializeDefaultBugReport) { @@ -361,6 +400,7 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) { EXPECT_FALSE(options_.is_remote_mode); EXPECT_FALSE(options_.use_socket); EXPECT_FALSE(options_.wifi_only); + EXPECT_FALSE(options_.arc_only); } TEST_F(DumpOptionsTest, InitializePartial1) { @@ -390,6 +430,7 @@ TEST_F(DumpOptionsTest, InitializePartial1) { EXPECT_FALSE(options_.do_screenshot); EXPECT_FALSE(options_.do_progress_updates); EXPECT_FALSE(options_.is_remote_mode); + EXPECT_FALSE(options_.arc_only); EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT); } @@ -419,6 +460,7 @@ TEST_F(DumpOptionsTest, InitializePartial2) { EXPECT_FALSE(options_.do_zip_file); EXPECT_FALSE(options_.use_socket); EXPECT_FALSE(options_.use_control_socket); + EXPECT_FALSE(options_.arc_only); EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT); }