Clean up shm segments created by shm_test.

This test suite was creating shm segments without ensuring they were
cleaned up. Shm segments outlive the process creating them, so on a
standard linux machine the test was leaving segments behind after each
run. This would often cause failures as test cases would be affected
by the cases that ran before them and left unexpected segments lying
around.

Also skip some assertions around memory usage when running on a Linux
host, as we can't reason about external users of shm segments.

PiperOrigin-RevId: 225435523
Change-Id: Ia299dacf59045002436f5e30dcc131f679bb7272

1 files changed, 123 insertions, 66 deletions

diff --git a/test/syscalls/linux/shm.cc b/test/syscalls/linux/shm.cc
index 9f57476c9..9d6d636d5 100644
--- a/test/syscalls/linux/shm.cc
+++ b/test/syscalls/linux/shm.cc
@@ -33,14 +33,6 @@ using ::testing::_;
 
 const uint64_t kAllocSize = kPageSize * 128ULL;
 
-PosixErrorOr<int> Shmget(key_t key, size_t size, int shmflg) {
-  int id = shmget(key, size, shmflg);
-  if (id == -1) {
-    return PosixError(errno, "shmget() failed");
-  }
-  return id;
-}
-
 PosixErrorOr<char*> Shmat(int shmid, const void* shmaddr, int shmflg) {
   const intptr_t addr =
       reinterpret_cast<intptr_t>(shmat(shmid, shmaddr, shmflg));
@@ -67,57 +59,109 @@ PosixErrorOr<int> Shmctl(int shmid, int cmd, T* buf) {
   return ret;
 }
 
+// ShmSegment is a RAII object for automatically cleaning up shm segments.
+class ShmSegment {
+ public:
+  explicit ShmSegment(int id) : id_(id) {}
+
+  ~ShmSegment() {
+    if (id_ >= 0) {
+      EXPECT_NO_ERRNO(Rmid());
+      id_ = -1;
+    }
+  }
+
+  ShmSegment(ShmSegment&& other) : id_(other.release()) {}
+
+  ShmSegment& operator=(ShmSegment&& other) {
+    id_ = other.release();
+    return *this;
+  }
+
+  ShmSegment(ShmSegment const& other) = delete;
+  ShmSegment& operator=(ShmSegment const& other) = delete;
+
+  int id() const { return id_; }
+
+  int release() {
+    int id = id_;
+    id_ = -1;
+    return id;
+  }
+
+  PosixErrorOr<int> Rmid() {
+    RETURN_IF_ERRNO(Shmctl<void>(id_, IPC_RMID, nullptr));
+    return release();
+  }
+
+ private:
+  int id_ = -1;
+};
+
+PosixErrorOr<int> ShmgetRaw(key_t key, size_t size, int shmflg) {
+  int id = shmget(key, size, shmflg);
+  if (id == -1) {
+    return PosixError(errno, "shmget() failed");
+  }
+  return id;
+}
+
+PosixErrorOr<ShmSegment> Shmget(key_t key, size_t size, int shmflg) {
+  ASSIGN_OR_RETURN_ERRNO(int id, ShmgetRaw(key, size, shmflg));
+  return ShmSegment(id);
+}
+
 TEST(ShmTest, AttachDetach) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
   struct shmid_ds attr;
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
   EXPECT_EQ(attr.shm_segsz, kAllocSize);
   EXPECT_EQ(attr.shm_nattch, 0);
 
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
   EXPECT_EQ(attr.shm_nattch, 1);
 
-  const char* addr2 = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  const char* addr2 = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
   EXPECT_EQ(attr.shm_nattch, 2);
 
   ASSERT_NO_ERRNO(Shmdt(addr));
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
   EXPECT_EQ(attr.shm_nattch, 1);
 
   ASSERT_NO_ERRNO(Shmdt(addr2));
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
   EXPECT_EQ(attr.shm_nattch, 0);
 }
 
 TEST(ShmTest, LookupByKey) {
   const TempPath keyfile = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
   const key_t key = ftok(keyfile.path().c_str(), 1);
-  const int id =
+  const ShmSegment shm =
       ASSERT_NO_ERRNO_AND_VALUE(Shmget(key, kAllocSize, IPC_CREAT | 0777));
-  const int id2 = ASSERT_NO_ERRNO_AND_VALUE(Shmget(key, kAllocSize, 0777));
-  EXPECT_EQ(id, id2);
+  const int id2 = ASSERT_NO_ERRNO_AND_VALUE(ShmgetRaw(key, kAllocSize, 0777));
+  EXPECT_EQ(shm.id(), id2);
 }
 
 TEST(ShmTest, DetachedSegmentsPersist) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
   addr[0] = 'x';
   ASSERT_NO_ERRNO(Shmdt(addr));
 
   // We should be able to re-attach to the same segment and get our data back.
-  addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
   EXPECT_EQ(addr[0], 'x');
   ASSERT_NO_ERRNO(Shmdt(addr));
 }
 
 TEST(ShmTest, MultipleDetachFails) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
   ASSERT_NO_ERRNO(Shmdt(addr));
   EXPECT_THAT(Shmdt(addr), PosixErrorIs(EINVAL, _));
 }
@@ -128,7 +172,7 @@ TEST(ShmTest, IpcStat) {
 
   const time_t start = time(nullptr);
 
-  const int id =
+  const ShmSegment shm =
       ASSERT_NO_ERRNO_AND_VALUE(Shmget(key, kAllocSize, IPC_CREAT | 0777));
 
   const uid_t uid = getuid();
@@ -136,7 +180,7 @@ TEST(ShmTest, IpcStat) {
   const pid_t pid = getpid();
 
   struct shmid_ds attr;
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
 
   EXPECT_EQ(attr.shm_perm.__key, key);
   EXPECT_EQ(attr.shm_perm.uid, uid);
@@ -163,8 +207,8 @@ TEST(ShmTest, IpcStat) {
   absl::SleepFor(absl::Seconds(1));
   const time_t pre_attach = time(nullptr);
 
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
 
   EXPECT_GE(attr.shm_atime, pre_attach);
   EXPECT_EQ(attr.shm_dtime, 0);
@@ -176,7 +220,7 @@ TEST(ShmTest, IpcStat) {
   const time_t pre_detach = time(nullptr);
 
   ASSERT_NO_ERRNO(Shmdt(addr));
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
 
   EXPECT_LT(attr.shm_atime, pre_detach);
   EXPECT_GE(attr.shm_dtime, pre_detach);
@@ -191,7 +235,8 @@ TEST(ShmTest, ShmStat) {
   // general Linux host.
   SKIP_IF(!IsRunningOnGvisor());
 
-  ASSERT_NO_ERRNO(Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
+      Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
   struct shmid_ds attr;
   ASSERT_NO_ERRNO(Shmctl(1, SHM_STAT, &attr));
   // This does the same thing as IPC_STAT, so only test that the syscall
@@ -225,14 +270,14 @@ TEST(ShmTest, ShmInfo) {
     EXPECT_EQ(info.shm_swp, 0);
   }
 
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
 
   ASSERT_NO_ERRNO(Shmctl(1, SHM_INFO, &info));
 
   if (IsRunningOnGvisor()) {
-    ASSERT_NO_ERRNO(Shmctl(id, SHM_INFO, &info));
+    ASSERT_NO_ERRNO(Shmctl(shm.id(), SHM_INFO, &info));
     EXPECT_EQ(info.used_ids, 1);
     EXPECT_EQ(info.shm_tot, kAllocSize / kPageSize);
     EXPECT_EQ(info.shm_rss, kAllocSize / kPageSize);
@@ -243,28 +288,28 @@ TEST(ShmTest, ShmInfo) {
 }
 
 TEST(ShmTest, ShmCtlSet) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
 
   struct shmid_ds attr;
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
   ASSERT_EQ(attr.shm_perm.mode, 0777);
 
   attr.shm_perm.mode = 0766;
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_SET, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_SET, &attr));
 
-  ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
+  ASSERT_NO_ERRNO(Shmctl(shm.id(), IPC_STAT, &attr));
   ASSERT_EQ(attr.shm_perm.mode, 0766);
 
   ASSERT_NO_ERRNO(Shmdt(addr));
 }
 
 TEST(ShmTest, RemovedSegmentsAreMarkedDeleted) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
-  ASSERT_NO_ERRNO(Shmctl<void>(id, IPC_RMID, nullptr));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
+  const int id = ASSERT_NO_ERRNO_AND_VALUE(shm.Rmid());
   struct shmid_ds attr;
   ASSERT_NO_ERRNO(Shmctl(id, IPC_STAT, &attr));
   EXPECT_NE(attr.shm_perm.mode & SHM_DEST, 0);
@@ -272,29 +317,32 @@ TEST(ShmTest, RemovedSegmentsAreMarkedDeleted) {
 }
 
 TEST(ShmTest, RemovedSegmentsAreDestroyed) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
 
   const uint64_t alloc_pages = kAllocSize / kPageSize;
 
   struct shm_info info;
-  ASSERT_NO_ERRNO(Shmctl(1, SHM_INFO, &info));
+  ASSERT_NO_ERRNO(Shmctl(0 /*ignored*/, SHM_INFO, &info));
   const uint64_t before = info.shm_tot;
 
-  ASSERT_NO_ERRNO(Shmctl<void>(id, IPC_RMID, nullptr));
+  ASSERT_NO_ERRNO(shm.Rmid());
   ASSERT_NO_ERRNO(Shmdt(addr));
 
-  ASSERT_NO_ERRNO(Shmctl(1, SHM_INFO, &info));
-  const uint64_t after = info.shm_tot;
-  EXPECT_EQ(after, before - alloc_pages);
+  ASSERT_NO_ERRNO(Shmctl(0 /*ignored*/, SHM_INFO, &info));
+  if (IsRunningOnGvisor()) {
+    // No guarantees on system-wide shm memory usage on a generic linux host.
+    const uint64_t after = info.shm_tot;
+    EXPECT_EQ(after, before - alloc_pages);
+  }
 }
 
 TEST(ShmTest, AllowsAttachToRemovedSegmentWithRefs) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
-  ASSERT_NO_ERRNO(Shmctl<void>(id, IPC_RMID, nullptr));
+  const char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
+  const int id = ASSERT_NO_ERRNO_AND_VALUE(shm.Rmid());
   const char* addr2 = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
   ASSERT_NO_ERRNO(Shmdt(addr));
   ASSERT_NO_ERRNO(Shmdt(addr2));
@@ -303,17 +351,17 @@ TEST(ShmTest, AllowsAttachToRemovedSegmentWithRefs) {
 TEST(ShmTest, RemovedSegmentsAreNotDiscoverable) {
   const TempPath keyfile = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
   const key_t key = ftok(keyfile.path().c_str(), 1);
-  const int id =
+  ShmSegment shm =
       ASSERT_NO_ERRNO_AND_VALUE(Shmget(key, kAllocSize, IPC_CREAT | 0777));
-  ASSERT_NO_ERRNO(Shmctl<void>(id, IPC_RMID, nullptr));
+  ASSERT_NO_ERRNO(shm.Rmid());
   EXPECT_THAT(Shmget(key, kAllocSize, 0777), PosixErrorIs(ENOENT, _));
 }
 
 TEST(ShmDeathTest, ReadonlySegment) {
   SetupGvisorDeathTest();
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, SHM_RDONLY));
+  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, SHM_RDONLY));
   // Reading succeeds.
   static_cast<void>(addr[0]);
   // Writing fails.
@@ -329,9 +377,16 @@ TEST(ShmDeathTest, SegmentNotAccessibleAfterDetach) {
   SetupGvisorDeathTest();
 
   const auto rest = [&] {
-    const int id = ASSERT_NO_ERRNO_AND_VALUE(
+    ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
         Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-    char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+    char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
+
+    // Mark the segment as destroyed so it's automatically cleaned up when we
+    // crash below. We can't rely on the standard cleanup since the destructor
+    // will not run after the SIGSEGV. Note that this doesn't destroy the
+    // segment immediately since we're still attached to it.
+    ASSERT_NO_ERRNO(shm.Rmid());
+
     addr[0] = 'x';
     ASSERT_NO_ERRNO(Shmdt(addr));
 
@@ -366,7 +421,7 @@ TEST(ShmTest, RequestingUnalignedSizeSucceeds) {
 TEST(ShmTest, RequestingDuplicateCreationFails) {
   const TempPath keyfile = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
   const key_t key = ftok(keyfile.path().c_str(), 1);
-  ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(key, kAllocSize, IPC_CREAT | IPC_EXCL | 0777));
   EXPECT_THAT(Shmget(key, kAllocSize, IPC_CREAT | IPC_EXCL | 0777),
               PosixErrorIs(EEXIST, _));
@@ -377,16 +432,17 @@ TEST(ShmTest, SegmentsSizeFixedOnCreation) {
   const key_t key = ftok(keyfile.path().c_str(), 1);
 
   // Base segment.
-  const int id =
+  const ShmSegment shm =
       ASSERT_NO_ERRNO_AND_VALUE(Shmget(key, kAllocSize, IPC_CREAT | 0777));
 
   // Ask for the same segment at half size. This succeeds.
-  const int id2 = ASSERT_NO_ERRNO_AND_VALUE(Shmget(key, kAllocSize / 2, 0777));
+  const int id2 =
+      ASSERT_NO_ERRNO_AND_VALUE(ShmgetRaw(key, kAllocSize / 2, 0777));
 
   // Ask for the same segment at double size.
   EXPECT_THAT(Shmget(key, kAllocSize * 2, 0777), PosixErrorIs(EINVAL, _));
 
-  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
   char* addr2 = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id2, nullptr, 0));
 
   // We have 2 different maps...
@@ -402,9 +458,9 @@ TEST(ShmTest, SegmentsSizeFixedOnCreation) {
 }
 
 TEST(ShmTest, PartialUnmap) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  const ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
   EXPECT_THAT(munmap(addr + (kAllocSize / 4), kAllocSize / 2),
               SyscallSucceeds());
   ASSERT_NO_ERRNO(Shmdt(addr));
@@ -417,15 +473,16 @@ TEST(ShmTest, GracefullyFailOnZeroLenSegmentCreation) {
 }
 
 TEST(ShmTest, NoDestructionOfAttachedSegmentWithMultipleRmid) {
-  const int id = ASSERT_NO_ERRNO_AND_VALUE(
+  ShmSegment shm = ASSERT_NO_ERRNO_AND_VALUE(
       Shmget(IPC_PRIVATE, kAllocSize, IPC_CREAT | 0777));
-  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
-  char* addr2 = ASSERT_NO_ERRNO_AND_VALUE(Shmat(id, nullptr, 0));
+  char* addr = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
+  char* addr2 = ASSERT_NO_ERRNO_AND_VALUE(Shmat(shm.id(), nullptr, 0));
 
   // There should be 2 refs to the segment from the 2 attachments, and a single
   // self-reference. Mark the segment as destroyed more than 3 times through
   // shmctl(RMID). If there's a bug with the ref counting, this should cause the
   // count to drop to zero.
+  int id = shm.release();
   for (int i = 0; i < 6; ++i) {
     ASSERT_NO_ERRNO(Shmctl<void>(id, IPC_RMID, nullptr));
   }