[llvm] [unittests][Support] Fix FileSystemTest for BSD semantics (PR #181487)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Jessica Clarke (jrtc27)

<details>
<summary>Changes</summary>

BSD semantics (also available on Linux with mount -o bsdgroups/grpid) for file/directory creation are to inherit the group ID from the parent directory, rather than using the set-group-ID bit. When running these tests on FreeBSD (and likely other BSDs), temporary files and directories are created underneath /tmp, which is root:wheel (0:0), and so they, and all their descendants, have a group of wheel.

For FileSystemTest.RemoveDirectoriesNoExePerm, in order to allow traversing the directory which has just been made non-executable, i.e. non-searchable, so that it can be deleted, the test first sets its permissions to all_perms. However, all_perms is not just the user/group/owner read/write/execute bits, it also includes the set-user-ID, set-group-ID and sticky bits. Since the directory on FreeBSD has a group of wheel, any users not in the wheel group cannot set the set-group-ID bit, so this will fail with EPERM, leaving the directory non-executable, and the following removal will fail, due to the foo child not being searchable, and then the test harness cleanup will fail due to the directory not being empty. This test does not actually need any of these extra permissions to be set, so use all_all instead, which is just the normal permission bits, and therefore can be performed.

Similarly, for FileSystemTest.permissions, we try to set the set-group-ID bit on the temporary file. However, in this case, this is a deliberate API feature being tested, and so we should make sure it works. Therefore, explicitly set the group on the file at the start of the test to be the effective group ID, i.e. what Linux will default to.

Fixes: 79f34ae7fe92 ("[llvm] Fix assertion when stat fails in remove_directories"
Fixes: 566fdf4a2a89 ("[Support] Add support for getting file system permissions on Windows and implement sys::fs::set/getPermissions to work with them")

---
Full diff: https://github.com/llvm/llvm-project/pull/181487.diff


1 Files Affected:

- (modified) llvm/unittests/Support/Path.cpp (+17-1) 


``````````diff
diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index 08c0c55404d29..b27ed6f950b10 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -41,6 +41,7 @@
 #ifdef LLVM_ON_UNIX
 #include <pwd.h>
 #include <sys/stat.h>
+#include <unistd.h>
 #endif
 
 using namespace llvm;
@@ -838,7 +839,7 @@ TEST_F(FileSystemTest, RemoveDirectoriesNoExePerm) {
   // It's expected that the directory exists, but some environments appear to
   // allow the removal despite missing the 'x' permission, so be flexible.
   if (fs::exists(Twine(TestDirectory) + "/noexeperm")) {
-    fs::setPermissions(Twine(TestDirectory) + "/noexeperm", fs::all_perms);
+    fs::setPermissions(Twine(TestDirectory) + "/noexeperm", fs::all_all);
     ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory) + "/noexeperm",
                                            /*IgnoreErrors=*/false));
   }
@@ -2217,6 +2218,21 @@ TEST_F(FileSystemTest, permissions) {
   ASSERT_NO_ERROR(fs::createTemporaryFile("prefix", "temp", FD, TempPath));
   FileRemover Cleanup(TempPath);
 
+  // BSD semantics (also available on Linux via mount -o bsdgroups/grpid) are
+  // for the group ID to be inherited, rather than only if the directory's
+  // set-group-ID bit is set, and so if the temporary file is created in a
+  // directory whose group is one the current user is not in (e.g. /tmp,
+  // typically root:wheel, i.e. 0:0, for a non-admin user) we will be unable to
+  // test setting the set-group-ID bit on it. Force the group to the effective
+  // group ID, i.e. the default Linux semantics, so we can perform tests
+  // setting the set-group-ID bit below. Note getegid() has no reserved return
+  // value to signify an error, it always succeeds.
+#ifdef LLVM_ON_UNIX
+  fs::file_status Status;
+  ASSERT_NO_ERROR(fs::status(FD, Status));
+  ASSERT_NO_ERROR(fs::changeFileOwnership(FD, Status.getUser(), getegid()));
+#endif
+
   // Make sure it exists.
   ASSERT_TRUE(fs::exists(Twine(TempPath)));
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/181487