Make ProcessStat collection more robust.

- Handle the case where start time is 0.
- Handle the case where getting uptime failed.
- Use 64bit int. (32bit is enough to record a time less than 24 days, but
  just in case.)

Bug: 315061143
Test: m test-art-host-gtest-art_runtime_tests
Change-Id: I3ac546f6921b8d6add2491170b5d8f50cc7169c1

3 files changed, 88 insertions, 13 deletions

diff --git a/runtime/exec_utils.cc b/runtime/exec_utils.cc
index 1021429460..b4d6553279 100644
--- a/runtime/exec_utils.cc
+++ b/runtime/exec_utils.cc
@@ -21,9 +21,6 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
-#include <ctime>
-#include <string_view>
-
 #ifdef __BIONIC__
 #include <sys/pidfd.h>
 #endif
@@ -32,8 +29,12 @@
 #include <climits>
 #include <condition_variable>
 #include <cstdint>
+#include <cstring>
+#include <ctime>
 #include <mutex>
+#include <optional>
 #include <string>
+#include <string_view>
 #include <thread>
 #include <vector>
 
@@ -232,6 +233,11 @@ bool ParseProcStat(const std::string& stat_content,
       !ParseInt(stat_fields[21 - kSkippedFields], &starttime)) {
     return false;
   }
+  if (starttime == 0) {
+    // The start time is the time the process started after system boot, so it's not supposed to be
+    // zero unless the process is `init`.
+    return false;
+  }
   stat->cpu_time_ms = (utime + stime + cutime + cstime) * 1000 / ticks_per_sec;
   stat->wall_time_ms = uptime_ms - starttime * 1000 / ticks_per_sec;
   return true;
@@ -338,9 +344,12 @@ std::string ExecUtils::GetProcStat(pid_t pid) const {
   return stat_content;
 }
 
-int64_t ExecUtils::GetUptimeMs() const {
+std::optional<int64_t> ExecUtils::GetUptimeMs(std::string* error_msg) const {
   timespec t;
-  clock_gettime(CLOCK_MONOTONIC, &t);
+  if (clock_gettime(CLOCK_MONOTONIC, &t) != 0) {
+    *error_msg = ART_FORMAT("Failed to get uptime: {}", strerror(errno));
+    return std::nullopt;
+  }
   return t.tv_sec * 1000 + t.tv_nsec / 1000000;
 }
 
@@ -349,14 +358,17 @@ int64_t ExecUtils::GetTicksPerSec() const { return sysconf(_SC_CLK_TCK); }
 bool ExecUtils::GetStat(pid_t pid,
                         /*out*/ ProcessStat* stat,
                         /*out*/ std::string* error_msg) const {
-  int64_t uptime_ms = GetUptimeMs();
+  std::optional<int64_t> uptime_ms = GetUptimeMs(error_msg);
+  if (!uptime_ms.has_value()) {
+    return false;
+  }
   std::string stat_content = GetProcStat(pid);
   if (stat_content.empty()) {
     *error_msg = StringPrintf("Failed to read /proc/%d/stat: %s", pid, strerror(errno));
     return false;
   }
   int64_t ticks_per_sec = GetTicksPerSec();
-  if (!ParseProcStat(stat_content, uptime_ms, ticks_per_sec, stat)) {
+  if (!ParseProcStat(stat_content, *uptime_ms, ticks_per_sec, stat)) {
     *error_msg = StringPrintf("Failed to parse /proc/%d/stat '%s'", pid, stat_content.c_str());
     return false;
   }
diff --git a/runtime/exec_utils.h b/runtime/exec_utils.h
index b83722fd62..e09f285961 100644
--- a/runtime/exec_utils.h
+++ b/runtime/exec_utils.h
@@ -19,7 +19,9 @@
 
 #include <time.h>
 
+#include <cstdint>
 #include <functional>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -29,10 +31,10 @@ namespace art {
 
 struct ProcessStat {
   // The total wall time, in milliseconds, that the process spent, or 0 if failed to get the value.
-  int wall_time_ms = 0;
+  int64_t wall_time_ms = 0;
   // The total CPU time, in milliseconds, that the process and any waited-for children spent, or 0
   // if failed to get the value.
-  int cpu_time_ms = 0;
+  int64_t cpu_time_ms = 0;
 };
 
 struct ExecCallbacks {
@@ -105,7 +107,7 @@ class ExecUtils {
   // Returns the content of `/proc/<pid>/stat`, or an empty string if failed.
   virtual std::string GetProcStat(pid_t pid) const;
 
-  virtual int64_t GetUptimeMs() const;
+  virtual std::optional<int64_t> GetUptimeMs(std::string* error_msg) const;
 
   virtual int64_t GetTicksPerSec() const;
 
diff --git a/runtime/exec_utils_test.cc b/runtime/exec_utils_test.cc
index e89180b572..fc628fc078 100644
--- a/runtime/exec_utils_test.cc
+++ b/runtime/exec_utils_test.cc
@@ -22,9 +22,11 @@
 #include <cstring>
 #include <filesystem>
 #include <memory>
+#include <optional>
 #include <tuple>
 
 #include "android-base/logging.h"
+#include "android-base/result.h"
 #include "android-base/stringprintf.h"
 #include "base/file_utils.h"
 #include "base/memory_tool.h"
@@ -34,6 +36,7 @@
 
 namespace art {
 
+using ::android::base::Result;
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::Gt;
@@ -68,8 +71,19 @@ std::tuple<int, int> GetKernelVersion() {
 class TestingExecUtils : public ExecUtils {
  public:
   MOCK_METHOD(std::string, GetProcStat, (pid_t pid), (const, override));
-  MOCK_METHOD(int64_t, GetUptimeMs, (), (const, override));
+  MOCK_METHOD(Result<int64_t>, DoGetUptimeMs, (), (const));
   MOCK_METHOD(int64_t, GetTicksPerSec, (), (const, override));
+
+  // A workaround to avoid MOCK_METHOD on a method with an `std::string*` parameter, which will lead
+  // to a conflict between gmock and android-base/logging.h (b/132668253).
+  std::optional<int64_t> GetUptimeMs(std::string* error_msg) const override {
+    Result<int64_t> result = DoGetUptimeMs();
+    if (result.ok()) {
+      return *result;
+    }
+    *error_msg = result.error().message();
+    return std::nullopt;
+  }
 };
 
 class AlwaysFallbackExecUtils : public TestingExecUtils {
@@ -204,7 +218,7 @@ TEST_P(ExecUtilsTest, ExecStat) {
       .WillOnce(Return(
           "14963 (a) b) Z 6067 14963 1 0 -1 4228108 105 0 0 0 94 5 0 0 39 19 1 0 162034388 0 0 "
           "18446744073709551615 0 0 0 0 0 0 20999 0 0 1 0 0 17 71 0 0 0 0 0 0 0 0 0 0 0 0 9"));
-  EXPECT_CALL(*exec_utils_, GetUptimeMs()).WillOnce(Return(1620344887ll));
+  EXPECT_CALL(*exec_utils_, DoGetUptimeMs()).WillOnce(Return(1620344887ll));
   EXPECT_CALL(*exec_utils_, GetTicksPerSec()).WillOnce(Return(100));
 
   ASSERT_EQ(
@@ -218,6 +232,53 @@ TEST_P(ExecUtilsTest, ExecStat) {
   EXPECT_EQ(stat.wall_time_ms, 1007);
 }
 
+TEST_P(ExecUtilsTest, ExecStatNoStartTime) {
+  std::vector<std::string> command;
+  command.push_back(GetBin("id"));
+
+  std::string error_msg;
+  ProcessStat stat;
+
+  // The process filename is "a) b".
+  EXPECT_CALL(*exec_utils_, GetProcStat(_))
+      .WillOnce(Return(
+          "14963 (a) b) Z 6067 14963 1 0 -1 4228108 105 0 0 0 94 5 0 0 39 19 1 0 0 0 0 "
+          "18446744073709551615 0 0 0 0 0 0 20999 0 0 1 0 0 17 71 0 0 0 0 0 0 0 0 0 0 0 0 9"));
+  EXPECT_CALL(*exec_utils_, DoGetUptimeMs()).WillOnce(Return(1620344887ll));
+  EXPECT_CALL(*exec_utils_, GetTicksPerSec()).WillOnce(Return(100));
+
+  ASSERT_EQ(
+      exec_utils_
+          ->ExecAndReturnResult(command, /*timeout_sec=*/-1, ExecCallbacks(), &stat, &error_msg)
+          .status,
+      ExecResult::kExited)
+      << error_msg;
+
+  EXPECT_EQ(stat.cpu_time_ms, 0);
+  EXPECT_EQ(stat.wall_time_ms, 0);
+}
+
+TEST_P(ExecUtilsTest, ExecStatNoUptime) {
+  std::vector<std::string> command;
+  command.push_back(GetBin("id"));
+
+  std::string error_msg;
+  ProcessStat stat;
+
+  EXPECT_CALL(*exec_utils_, DoGetUptimeMs())
+      .WillOnce(Return(Result<int64_t>(Errorf("Failed to get uptime"))));
+
+  ASSERT_EQ(
+      exec_utils_
+          ->ExecAndReturnResult(command, /*timeout_sec=*/-1, ExecCallbacks(), &stat, &error_msg)
+          .status,
+      ExecResult::kExited)
+      << error_msg;
+
+  EXPECT_EQ(stat.cpu_time_ms, 0);
+  EXPECT_EQ(stat.wall_time_ms, 0);
+}
+
 TEST_P(ExecUtilsTest, ExecStatFailed) {
   std::vector<std::string> command = SleepCommand(5);
 
@@ -228,7 +289,7 @@ TEST_P(ExecUtilsTest, ExecStatFailed) {
       .WillOnce(Return(
           "14963 (a) b) Z 6067 14963 1 0 -1 4228108 105 0 0 0 94 5 0 0 39 19 1 0 162034388 0 0 "
           "18446744073709551615 0 0 0 0 0 0 20999 0 0 1 0 0 17 71 0 0 0 0 0 0 0 0 0 0 0 0 9"));
-  EXPECT_CALL(*exec_utils_, GetUptimeMs()).WillOnce(Return(1620344887ll));
+  EXPECT_CALL(*exec_utils_, DoGetUptimeMs()).WillOnce(Return(1620344887ll));
   EXPECT_CALL(*exec_utils_, GetTicksPerSec()).WillOnce(Return(100));
 
   // This will always time out.