From 72a8ea3d3c7c712bad74a8498df129dbabe41b5f Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 25 Oct 2017 09:23:19 -0700 Subject: [PATCH 1/3] bootstat: test: fix Its_Just_So_Hard_reboot Add the test injection to known list, and deal with an error propagation issue. Test: system/core/bootstat/boot_reason_test.sh Its_Just_So_Hard_reboot Bug: 63736262 Change-Id: I4799956978a8884c69c830fcedd7febd143651fd --- bootstat/boot_reason_test.sh | 23 ++++++++++++++++++----- bootstat/bootstat.cpp | 2 ++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/bootstat/boot_reason_test.sh b/bootstat/boot_reason_test.sh index af65135df..e69db12d8 100755 --- a/bootstat/boot_reason_test.sh +++ b/bootstat/boot_reason_test.sh @@ -873,17 +873,30 @@ Its Just So Hard reboot test: - NB: should report reboot,its_just_so_hard - NB: expect log \"... I bootstat: Unknown boot reason: reboot,its_just_so_hard\"" ] test_Its_Just_So_Hard_reboot() { - duration_test + if isDebuggable; then # see below + duration_test + else + duration_test `expr ${DURATION_DEFAULT} + ${DURATION_DEFAULT}` + fi adb shell 'reboot "Its Just So Hard"' wait_for_screen EXPECT_PROPERTY sys.boot.reason reboot,its_just_so_hard EXPECT_PROPERTY persist.sys.boot.reason "reboot,Its Just So Hard" - adb shell su root setprop persist.sys.boot.reason reboot,its_just_so_hard - if checkDebugBuild; then - flag="" + # Do not leave this test with an illegal value in persist.sys.boot.reason + save_ret=${?} # hold on to error code from above two lines + if isDebuggable; then # can do this easy, or we can do this hard. + adb shell su root setprop persist.sys.boot.reason reboot,its_just_so_hard + ( exit ${save_ret} ) # because one can not just do ?=${save_ret} else - flag="--allow_failure" + report_bootstat_logs reboot,its_just_so_hard # report what we have so far + # user build mitigation + adb shell reboot its_just_so_hard + wait_for_screen + ( exit ${save_ret} ) # because one can not just do ?=${save_ret} + EXPECT_PROPERTY sys.boot.reason reboot,its_just_so_hard fi + # Ensure persist.sys.boot.reason now valid, failure here acts as a signal + # that we could choke up following tests. For example test_properties. EXPECT_PROPERTY persist.sys.boot.reason reboot,its_just_so_hard ${flag} report_bootstat_logs reboot,its_just_so_hard } diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp index c1d799e51..2f9e7a788 100644 --- a/bootstat/bootstat.cpp +++ b/bootstat/bootstat.cpp @@ -225,6 +225,8 @@ const std::map kBootReasonMap = { {"reboot,longkey", 85}, {"reboot,2sec", 86}, {"shutdown,thermal,battery", 87}, + {"reboot,its_just_so_hard", 88}, // produced by boot_reason_test + {"reboot,Its Just So Hard", 89}, // produced by boot_reason_test }; // Converts a string value representing the reason the system booted to an From 88d692c09ede2685a670e6b7cb5f2f09cda0e424 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 20 Sep 2017 08:37:46 -0700 Subject: [PATCH 2/3] bootstat: move boot reason validation transformation policy into subroutine. Allow for future policy adjustments. SideEffects: None Test: system/core/bootstat/boot_reason_test.sh Bug: 63736262 Change-Id: I571fb7dafc6b80c75d2809a3da3f9b96784cef06 --- bootstat/bootstat.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp index 2f9e7a788..da1e9d82f 100644 --- a/bootstat/bootstat.cpp +++ b/bootstat/bootstat.cpp @@ -353,10 +353,18 @@ bool addKernelPanicSubReason(const std::string& console, std::string& ret) { char tounderline(char c) { return ::isblank(c) ? '_' : c; } + char toprintable(char c) { return ::isprint(c) ? c : '?'; } +// Cleanup boot_reason regarding acceptable character set +void transformReason(std::string& reason) { + std::transform(reason.begin(), reason.end(), reason.begin(), ::tolower); + std::transform(reason.begin(), reason.end(), reason.begin(), tounderline); + std::transform(reason.begin(), reason.end(), reason.begin(), toprintable); +} + const char system_reboot_reason_property[] = "sys.boot.reason"; const char last_reboot_reason_property[] = LAST_REBOOT_REASON_PROPERTY; const char bootloader_reboot_reason_property[] = "ro.boot.bootreason"; @@ -370,10 +378,7 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { // If sys.boot.reason == ro.boot.bootreason, let's re-evaluate if (reason == ret) ret = ""; - // Cleanup boot_reason regarding acceptable character set - std::transform(reason.begin(), reason.end(), reason.begin(), ::tolower); - std::transform(reason.begin(), reason.end(), reason.begin(), tounderline); - std::transform(reason.begin(), reason.end(), reason.begin(), toprintable); + transformReason(reason); // Is the current system boot reason sys.boot.reason valid? if (!isKnownRebootReason(ret)) ret = ""; @@ -462,13 +467,13 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { pos += strlen(cmd); std::string subReason(content.substr(pos, max_reason_length)); for (pos = 0; pos < subReason.length(); ++pos) { - char c = tounderline(subReason[pos]); + char c = subReason[pos]; if (!::isprint(c) || (c == '\'')) { subReason.erase(pos); break; } - subReason[pos] = ::tolower(c); } + transformReason(subReason); if (subReason != "") { // Will not land "reboot" as that is too blunt. if (isKernelRebootReason(subReason)) { ret = "reboot," + subReason; // User space can't talk kernel reasons. @@ -565,10 +570,7 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { // Content buffer no longer will have console data. Beware if more // checks added below, that depend on parsing console content. content = GetProperty(last_reboot_reason_property); - // Cleanup last_boot_reason regarding acceptable character set - std::transform(content.begin(), content.end(), content.begin(), ::tolower); - std::transform(content.begin(), content.end(), content.begin(), tounderline); - std::transform(content.begin(), content.end(), content.begin(), toprintable); + transformReason(content); // Anything in last is better than 'super-blunt' reboot or shutdown. if ((ret == "") || (ret == "reboot") || (ret == "shutdown") || !isBluntRebootReason(content)) { From dafced93a56352b79c74b016eba437b28bbbe11e Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 20 Sep 2017 08:37:46 -0700 Subject: [PATCH 3/3] bootstat: Do not allow unknown boot reasons to land in first field. If we sniff an unknown boot reason from last kmsg, make sure it has a "reboot," prefix. Test: system/core/bootstat/boot_reason_test.sh Bug: 63736262 Change-Id: Ia1c401b8899d1f0c56bd4f5d8d2d19b7fc889a30 --- bootstat/bootstat.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp index da1e9d82f..07d410c91 100644 --- a/bootstat/bootstat.cpp +++ b/bootstat/bootstat.cpp @@ -477,8 +477,10 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { if (subReason != "") { // Will not land "reboot" as that is too blunt. if (isKernelRebootReason(subReason)) { ret = "reboot," + subReason; // User space can't talk kernel reasons. - } else { + } else if (isKnownRebootReason(subReason)) { ret = subReason; + } else { + ret = "reboot," + subReason; // legitimize unknown reasons } } }