Merge "Add record_allocs_on_exit option." into main

Author: cferris1000

libc/malloc_debug/Config.cpp

@@ -187,6 +187,10 @@ const std::unordered_map<std::string, Config::OptionInfo> Config::kOptions = {
         "record_allocs_file",
         {0, &Config::SetRecordAllocsFile},
     },
+    {
+        "record_allocs_on_exit",
+        {0, &Config::SetRecordAllocsOnExit},
+    },
 
     {
         "verify_pointers",
@@ -401,6 +405,14 @@ bool Config::SetRecordAllocsFile(const std::string&, const std::string& value) {
   return true;
 }
 
+bool Config::SetRecordAllocsOnExit(const std::string& option, const std::string& value) {
+  if (Config::VerifyValueEmpty(option, value)) {
+    record_allocs_on_exit_ = true;
+    return true;
+  }
+  return false;
+}
+
 bool Config::VerifyValueEmpty(const std::string& option, const std::string& value) {
   if (!value.empty()) {
     // This is not valid.

libc/malloc_debug/Config.h

@@ -98,6 +98,7 @@ class Config {
   int record_allocs_signal() const { return record_allocs_signal_; }
   size_t record_allocs_num_entries() const { return record_allocs_num_entries_; }
   const std::string& record_allocs_file() const { return record_allocs_file_; }
+  bool record_allocs_on_exit() const { return record_allocs_on_exit_; }
 
   int check_unreachable_signal() const { return check_unreachable_signal_; }
 
@@ -139,6 +140,7 @@ class Config {
 
   bool SetRecordAllocs(const std::string& option, const std::string& value);
   bool SetRecordAllocsFile(const std::string& option, const std::string& value);
+  bool SetRecordAllocsOnExit(const std::string& option, const std::string& value);
 
   bool VerifyValueEmpty(const std::string& option, const std::string& value);
 
@@ -170,6 +172,7 @@ class Config {
   int record_allocs_signal_ = 0;
   size_t record_allocs_num_entries_ = 0;
   std::string record_allocs_file_;
+  bool record_allocs_on_exit_ = false;
 
   uint64_t options_ = 0;
   uint8_t fill_alloc_value_;

libc/malloc_debug/README.md

@@ -456,6 +456,19 @@ will be placed.
 
 **NOTE**: This option is not available until the O release of Android.
 
+### record\_allocs\_on\_exit
+This option only has meaning if record\_allocs is set. It indicates that
+when the process terminates, the record file should be created
+automatically.
+
+The only caveat to this option is that when the process terminates,
+the file that will contain the records will be the normal file name
+with **.PID** appended. Where PID is the pid of the process that has
+terminated. This is to avoid cases where a number of processes exit
+at the same time and attempt to write to the same file.
+
+**NOTE**: This option is not available until the V release of Android.
+
 ### verify\_pointers
 Track all live allocations to determine if a pointer is used that does not
 exist. This option is a lightweight way to verify that all

libc/malloc_debug/RecordData.cpp

@@ -131,17 +131,30 @@ void RecordData::WriteData(int, siginfo_t*, void*) {
   record_obj_->WriteEntries();
 }
 
+void RecordData::WriteEntriesOnExit() {
+  if (record_obj_ == nullptr) return;
+
+  // Append the current pid to the file name to avoid multiple processes
+  // writing to the same file.
+  std::string file(record_obj_->file());
+  file += "." + std::to_string(getpid());
+  record_obj_->WriteEntries(file);
+}
+
 void RecordData::WriteEntries() {
+  WriteEntries(file_);
+}
+
+void RecordData::WriteEntries(const std::string& file) {
   std::lock_guard<std::mutex> entries_lock(entries_lock_);
   if (cur_index_ == 0) {
     info_log("No alloc entries to write.");
     return;
   }
 
-  int dump_fd =
-      open(dump_file_.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, 0755);
+  int dump_fd = open(file.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, 0755);
   if (dump_fd == -1) {
-    error_log("Cannot create record alloc file %s: %s", dump_file_.c_str(), strerror(errno));
+    error_log("Cannot create record alloc file %s: %s", file.c_str(), strerror(errno));
     return;
   }
 
@@ -179,7 +192,7 @@ bool RecordData::Initialize(const Config& config) {
 
   entries_.resize(config.record_allocs_num_entries());
   cur_index_ = 0U;
-  dump_file_ = config.record_allocs_file();
+  file_ = config.record_allocs_file();
 
   return true;
 }

libc/malloc_debug/RecordData.h

@@ -162,19 +162,23 @@ class RecordData {
   void AddEntry(const RecordEntry* entry);
   void AddEntryOnly(const RecordEntry* entry);
 
+  const std::string& file() { return file_; }
   pthread_key_t key() { return key_; }
 
+  static void WriteEntriesOnExit();
+
  private:
   static void WriteData(int, siginfo_t*, void*);
   static RecordData* record_obj_;
 
   void WriteEntries();
+  void WriteEntries(const std::string& file);
 
   std::mutex entries_lock_;
   pthread_key_t key_;
   std::vector<std::unique_ptr<const RecordEntry>> entries_;
   size_t cur_index_;
-  std::string dump_file_;
+  std::string file_;
 
   BIONIC_DISALLOW_COPY_AND_ASSIGN(RecordData);
 };

libc/malloc_debug/malloc_debug.cpp

@@ -451,6 +451,10 @@ void debug_finalize() {
     PointerData::LogLeaks();
   }
 
+  if ((g_debug->config().options() & RECORD_ALLOCS) && g_debug->config().record_allocs_on_exit()) {
+    RecordData::WriteEntriesOnExit();
+  }
+
   if ((g_debug->config().options() & BACKTRACE) && g_debug->config().backtrace_dump_on_exit()) {
     debug_dump_heap(android::base::StringPrintf("%s.%d.exit.txt",
                                                 g_debug->config().backtrace_dump_prefix().c_str(),

libc/malloc_debug/tests/malloc_debug_config_tests.cpp

@@ -571,6 +571,25 @@ TEST_F(MallocDebugConfigTest, record_allocs_file) {
   ASSERT_STREQ("", getFakeLogPrint().c_str());
 }
 
+TEST_F(MallocDebugConfigTest, record_allocs_on_exit) {
+  ASSERT_TRUE(InitConfig("record_allocs_on_exit")) << getFakeLogPrint();
+  ASSERT_EQ(0U, config->options());
+  ASSERT_TRUE(config->record_allocs_on_exit());
+
+  ASSERT_STREQ("", getFakeLogBuf().c_str());
+  ASSERT_STREQ("", getFakeLogPrint().c_str());
+}
+
+TEST_F(MallocDebugConfigTest, record_allocs_on_exit_error) {
+  ASSERT_FALSE(InitConfig("record_allocs_on_exit=something")) << getFakeLogPrint();
+
+  ASSERT_STREQ("", getFakeLogBuf().c_str());
+  std::string log_msg(
+      "6 malloc_debug malloc_testing: value set for option 'record_allocs_on_exit' "
+      "which does not take a value\n");
+  ASSERT_STREQ((log_msg + usage_string).c_str(), getFakeLogPrint().c_str());
+}
+
 TEST_F(MallocDebugConfigTest, guard_min_error) {
   ASSERT_FALSE(InitConfig("guard=0"));
 

libc/malloc_debug/tests/malloc_debug_unit_tests.cpp

@@ -185,6 +185,7 @@ std::string ShowDiffs(uint8_t* a, uint8_t* b, size_t size) {
 }
 
 static void VerifyRecords(std::vector<std::string>& expected, std::string& actual) {
+  ASSERT_TRUE(expected.size() != 0);
   size_t offset = 0;
   for (std::string& str : expected) {
     ASSERT_STREQ(str.c_str(), actual.substr(offset, str.size()).c_str());
@@ -1512,7 +1513,7 @@ TEST_F(MallocDebugTest, backtrace_dump_on_exit) {
 
     // Call the exit function manually.
     debug_finalize();
-    exit(0);
+    _exit(0);
   }
   ASSERT_NE(-1, pid);
   ASSERT_EQ(pid, TEMP_FAILURE_RETRY(waitpid(pid, nullptr, 0)));
@@ -1561,7 +1562,7 @@ TEST_F(MallocDebugTest, backtrace_dump_on_exit_shared_backtrace) {
 
     // Call the exit function manually.
     debug_finalize();
-    exit(0);
+    _exit(0);
   }
   ASSERT_NE(-1, pid);
   ASSERT_EQ(pid, TEMP_FAILURE_RETRY(waitpid(pid, nullptr, 0)));
@@ -1619,7 +1620,7 @@ TEST_F(MallocDebugTest, backtrace_full_dump_on_exit) {
 
     // Call the exit function manually.
     debug_finalize();
-    exit(0);
+    _exit(0);
   }
   ASSERT_NE(-1, pid);
   ASSERT_EQ(pid, TEMP_FAILURE_RETRY(waitpid(pid, nullptr, 0)));
@@ -2429,6 +2430,33 @@ TEST_F(MallocDebugTest, record_allocs_write_entries_does_not_allocate) {
   ASSERT_STREQ("", getFakeLogPrint().c_str());
 }
 
+TEST_F(MallocDebugTest, record_allocs_on_exit) {
+  InitRecordAllocs("record_allocs record_allocs_on_exit");
+
+  // The filename created on exit always appends the pid.
+  // Modify the variable so the file is deleted at the end of the test.
+  record_filename += '.' + std::to_string(getpid());
+
+  std::vector<std::string> expected;
+  void* ptr = debug_malloc(100);
+  expected.push_back(android::base::StringPrintf("%d: malloc %p 100", getpid(), ptr));
+  ptr = debug_malloc(200);
+  expected.push_back(android::base::StringPrintf("%d: malloc %p 200", getpid(), ptr));
+  ptr = debug_malloc(400);
+  expected.push_back(android::base::StringPrintf("%d: malloc %p 400", getpid(), ptr));
+
+  // Call the exit function manually.
+  debug_finalize();
+
+  // Read all of the contents.
+  std::string actual;
+  ASSERT_TRUE(android::base::ReadFileToString(record_filename, &actual));
+  VerifyRecords(expected, actual);
+
+  ASSERT_STREQ("", getFakeLogBuf().c_str());
+  ASSERT_STREQ("", getFakeLogPrint().c_str());
+}
+
 TEST_F(MallocDebugTest, verify_pointers) {
   Init("verify_pointers");