commit 5fe73268f3813144ed7ae29d5b91c26ae8b2f0ca
author Tom Cherry <tomcherry@google.com> Tue Feb 20 15:40:59 2018 -0800
committer Tom Cherry <tomcherry@google.com> Tue Feb 20 16:02:13 2018 -0800
tree 5b421af08544f44b179ae53585ec604b9fed88cf
parent 60ddedff0b34f0058780b8cdbdeacef667526d08

Do not munmap in MmapFile::~MmapFile

Having any destructor with a global variable in bionic is causing
some issues. Since we don't actually need to munmap in this case, we
remove the destructor to work around that issue.

A small class is used to still munmap during tests.

Bug: 73485611
Test: bionic unit tests
Change-Id: Ibcd45e9b1ab22d187ecfc2738bb87244250d81ea

libc/bionic/grp_pwd_file.cpp

diff --git a/libc/bionic/grp_pwd_file.cpp b/libc/bionic/grp_pwd_file.cpp
index 911daea..c17dbb7 100644
--- a/libc/bionic/grp_pwd_file.cpp
+++ b/libc/bionic/grp_pwd_file.cpp
@@ -193,10 +193,13 @@
   lock_.init(false);
 }
 
-MmapFile::~MmapFile() {
+void MmapFile::Unmap() {
   if (status_ == FileStatus::Initialized) {
     size_t size = end_ - start_ + 1;
     munmap(const_cast<char*>(start_), size);
+    status_ = FileStatus::Uninitialized;
+    start_ = nullptr;
+    end_ = nullptr;
   }
 }
 

libc/bionic/grp_pwd_file.h

diff --git a/libc/bionic/grp_pwd_file.h b/libc/bionic/grp_pwd_file.h
index 93dd852..048cd82 100644
--- a/libc/bionic/grp_pwd_file.h
+++ b/libc/bionic/grp_pwd_file.h
@@ -38,12 +38,12 @@
 class MmapFile {
  public:
   MmapFile(const char* filename);
-  ~MmapFile();
 
   template <typename Line>
   bool FindById(uid_t uid, Line* line);
   template <typename Line>
   bool FindByName(const char* name, Line* line);
+  void Unmap();
 
   DISALLOW_COPY_AND_ASSIGN(MmapFile);
 
@@ -73,6 +73,9 @@
 
   bool FindById(uid_t id, passwd_state_t* passwd_state);
   bool FindByName(const char* name, passwd_state_t* passwd_state);
+  void Unmap() {
+    mmap_file_.Unmap();
+  }
 
   DISALLOW_COPY_AND_ASSIGN(PasswdFile);
 
@@ -86,6 +89,9 @@
 
   bool FindById(gid_t id, group_state_t* group_state);
   bool FindByName(const char* name, group_state_t* group_state);
+  void Unmap() {
+    mmap_file_.Unmap();
+  }
 
   DISALLOW_COPY_AND_ASSIGN(GroupFile);
 

tests/grp_pwd_file_test.cpp

diff --git a/tests/grp_pwd_file_test.cpp b/tests/grp_pwd_file_test.cpp
index 79f9a10..d6f3c9f 100644
--- a/tests/grp_pwd_file_test.cpp
+++ b/tests/grp_pwd_file_test.cpp
@@ -23,6 +23,19 @@
 #if defined(__BIONIC__)
 #include "../libc/bionic/grp_pwd_file.cpp"
 
+template <typename T>
+class FileUnmapper {
+ public:
+  FileUnmapper(T& file) : file_(file) {
+  }
+  ~FileUnmapper() {
+    file_.Unmap();
+  }
+
+ private:
+  T& file_;
+};
+
 void FindAndCheckPasswdEntry(PasswdFile* file, const char* name, uid_t uid, gid_t gid,
                              const char* dir, const char* shell) {
   passwd_state_t name_passwd_state;
@@ -82,6 +95,7 @@
   write(file.fd, test_string, sizeof(test_string) - 1);
 
   PasswdFile passwd_file(file.filename);
+  FileUnmapper unmapper(passwd_file);
 
   FindAndCheckPasswdEntry(&passwd_file, "name", 1, 2, "dir", "shell");
 
@@ -101,6 +115,7 @@
   write(file.fd, test_string, sizeof(test_string) - 1);
 
   GroupFile group_file(file.filename);
+  FileUnmapper unmapper(group_file);
 
   FindAndCheckGroupEntry(&group_file, "name", 1);
 
@@ -136,6 +151,7 @@
   write(file.fd, test_string, sizeof(test_string) - 1);
 
   PasswdFile passwd_file(file.filename);
+  FileUnmapper unmapper(passwd_file);
 
   FindAndCheckPasswdEntry(&passwd_file, "first", 1, 2, "dir", "shell");
   FindAndCheckPasswdEntry(&passwd_file, "middle-ish", 13, 4, "/", "/system/bin/sh");
@@ -171,6 +187,7 @@
   write(file.fd, test_string, sizeof(test_string) - 1);
 
   GroupFile group_file(file.filename);
+  FileUnmapper unmapper(group_file);
 
   FindAndCheckGroupEntry(&group_file, "first", 1);
   FindAndCheckGroupEntry(&group_file, "middle-ish", 6);