Merge pull request #2448 from Sonicadvance1/optimize_openat

EmulatedFiles: Optimize openat handler

Source/Common/FDUtils.h

@@ -23,4 +23,10 @@ std::optional<std::string> get_fdpath(int fd) {
   return std::nullopt;
 }
 
+inline
+int get_fdpath(int fd, char *SymlinkPath) {
+  auto Path = fmt::format("/proc/self/fd/{}", fd);
+  return readlinkat(AT_FDCWD, Path.c_str(), SymlinkPath, PATH_MAX);
+}
+
 }

Source/Tests/LinuxSyscalls/EmulatedFiles/EmulatedFiles.cpp

@@ -711,48 +711,68 @@ namespace FEX::EmulatedFile {
   }
 
   int32_t EmulatedFDManager::OpenAt(int dirfs, const char *pathname, int flags, uint32_t mode) {
-    std::string Path{};
-    if (((pathname && pathname[0] != '/') || // If pathname exists then it must not be absolute
-        !pathname) &&
-        dirfs != AT_FDCWD) {
-      // Passed in a dirfd that isn't magic FDCWD
-      // We need to get the path from the fd now
-      Path = FEX::get_fdpath(dirfs).value_or("");
+    char Tmp[PATH_MAX];
+    const char *Path{};
 
-      if (pathname) {
-        if (!Path.empty()) {
-          // If the path returned empty then we don't need a separator
-          Path += "/";
-        }
-        Path += pathname;
-      }
+    auto Creator = FDReadCreators.end();
+    if (pathname) {
+      Creator = FDReadCreators.find(pathname);
     }
-    else {
-      if (!pathname || strlen(pathname) == 0) {
-        return -1;
+
+    if (Creator == FDReadCreators.end()) {
+      if (((pathname && pathname[0] != '/') || // If pathname exists then it must not be absolute
+          !pathname) &&
+          dirfs != AT_FDCWD) {
+        // Passed in a dirfd that isn't magic FDCWD
+        // We need to get the path from the fd now
+        auto PathLength = FEX::get_fdpath(dirfs, Tmp);
+        if (PathLength != -1) {
+          if (pathname) {
+            Tmp[PathLength] = '/';
+            PathLength += 1;
+            strncpy(&Tmp[PathLength], pathname, PATH_MAX - PathLength);
+          }
+          else {
+            Tmp[PathLength] = '\0';
+          }
+          Path = Tmp;
+        }
+        else if (pathname) {
+          Path = pathname;
+        }
       }
-      else if (pathname) {
+      else {
+        if (!pathname || pathname[0] == 0) {
+          return -1;
+        }
+
         Path = pathname;
       }
+
+      bool exists = access(Path, F_OK) == 0;
+      bool RealPathExists = false;
+
+      if (exists) {
+        // If realpath fails then the temporary buffer is in an undefined state.
+        // Need to use another temporary just in-case realpath doesn't succeed.
+        char ExistsTempPath[PATH_MAX];
+        char *RealPath = realpath(Path, ExistsTempPath);
+        if (RealPath) {
+          RealPathExists = true;
+          Creator = FDReadCreators.find(realpath(Path, Tmp));
+        }
+      }
+
+      if (!RealPathExists) {
+        Creator = FDReadCreators.find(std::filesystem::path(Path).lexically_normal().string());
+      }
+
+      if (Creator == FDReadCreators.end()) {
+        return -1;
+      }
     }
 
-    std::error_code ec;
-    bool exists = access(Path.c_str(), F_OK) == 0;
-    if (ec) {
-      return -1;
-    }
-    string cpath = exists ? std::filesystem::canonical(Path, ec)
-      : std::filesystem::path(Path).lexically_normal(); // *Note: this doesn't transform to absolute
-
-    if (ec) {
-      return -1;
-    }
-    auto Creator = FDReadCreators.find(cpath);
-    if (Creator == FDReadCreators.end()) {
-      return -1;
-    }
-
-    return Creator->second(CTX, dirfs, Path.c_str(), flags, mode);
+    return Creator->second(CTX, dirfs, Path, flags, mode);
   }
 
   int32_t EmulatedFDManager::ProcAuxv(FEXCore::Context::Context* ctx, int32_t fd, const char* pathname, int32_t flags, mode_t mode)

Source/Tests/LinuxSyscalls/FileManagement.cpp

@@ -393,11 +393,13 @@ std::string FileManager::GetEmulatedPath(const char *pathname, bool FollowSymlin
   return Path;
 }
 
-std::optional<std::pair<int, const char*>> FileManager::GetEmulatedFDPath(const char *pathname, bool FollowSymlink, FDPathTmpData &TmpFilename) {
+std::pair<int, const char*> FileManager::GetEmulatedFDPath(const char *pathname, bool FollowSymlink, FDPathTmpData &TmpFilename) {
+  constexpr auto NoEntry = std::make_pair(-1, nullptr);
+
   if (!pathname || // If no pathname
       pathname[0] != '/' || // If relative
       pathname[1] == 0) { // If we are getting root
-    return std::nullopt;
+    return NoEntry;
   }
 
   auto thunkOverlay = ThunkOverlays.find(pathname);
@@ -407,7 +409,7 @@ std::optional<std::pair<int, const char*>> FileManager::GetEmulatedFDPath(const
 
   if (RootFSFD == AT_FDCWD) {
     // If RootFS doesn't exist
-    return std::nullopt;
+    return NoEntry;
   }
 
   // Starting subpath is the pathname passed in.
@@ -480,8 +482,8 @@ uint64_t FileManager::Open(const char *pathname, [[maybe_unused]] int flags, [[m
   if (fd == -1) {
     FDPathTmpData TmpFilename;
     auto Path = GetEmulatedFDPath(SelfPath, true, TmpFilename);
-    if (Path.has_value()) {
-      fd = ::openat(Path->first, Path->second, flags, mode);
+    if (Path.first != -1) {
+      fd = ::openat(Path.first, Path.second, flags, mode);
     }
 
     if (fd == -1) {
@@ -528,8 +530,8 @@ uint64_t FileManager::Stat(const char *pathname, void *buf) {
   // Stat follows symlinks
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, true, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::fstatat(Path->first, Path->second, reinterpret_cast<struct stat*>(buf), 0);
+  if (Path.first != -1) {
+    uint64_t Result = ::fstatat(Path.first, Path.second, reinterpret_cast<struct stat*>(buf), 0);
     if (Result != -1)
       return Result;
   }
@@ -543,8 +545,8 @@ uint64_t FileManager::Lstat(const char *pathname, void *buf) {
   // lstat does not follow symlinks
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, false, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::fstatat(Path->first, Path->second, reinterpret_cast<struct stat*>(buf), AT_SYMLINK_NOFOLLOW);
+  if (Path.first != -1) {
+    uint64_t Result = ::fstatat(Path.first, Path.second, reinterpret_cast<struct stat*>(buf), AT_SYMLINK_NOFOLLOW);
     if (Result != -1)
       return Result;
   }
@@ -559,8 +561,8 @@ uint64_t FileManager::Access(const char *pathname, [[maybe_unused]] int mode) {
   // Access follows symlinks
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, true, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::faccessat(Path->first, Path->second, mode, 0);
+  if (Path.first != -1) {
+    uint64_t Result = ::faccessat(Path.first, Path.second, mode, 0);
     if (Result != -1)
       return Result;
   }
@@ -574,8 +576,8 @@ uint64_t FileManager::FAccessat(int dirfd, const char *pathname, int mode) {
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, true, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::syscall(SYSCALL_DEF(faccessat2), Path->first, Path->second, mode, 0);
+  if (Path.first != -1) {
+    uint64_t Result = ::syscall(SYSCALL_DEF(faccessat2), Path.first, Path.second, mode, 0);
     if (Result != -1)
       return Result;
   }
@@ -589,8 +591,8 @@ uint64_t FileManager::FAccessat2(int dirfd, const char *pathname, int mode, int
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, (flags & AT_SYMLINK_NOFOLLOW) == 0, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::syscall(SYSCALL_DEF(faccessat2), Path->first, Path->second, mode, flags);
+  if (Path.first != -1) {
+    uint64_t Result = ::syscall(SYSCALL_DEF(faccessat2), Path.first, Path.second, mode, flags);
     if (Result != -1)
       return Result;
   }
@@ -614,8 +616,8 @@ uint64_t FileManager::Readlink(const char *pathname, char *buf, size_t bufsiz) {
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(pathname, false, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::readlinkat(Path->first, Path->second, buf, bufsiz);
+  if (Path.first != -1) {
+    uint64_t Result = ::readlinkat(Path.first, Path.second, buf, bufsiz);
     if (Result != -1)
       return Result;
 
@@ -636,8 +638,8 @@ uint64_t FileManager::Chmod(const char *pathname, mode_t mode) {
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, false, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::fchmodat(Path->first, Path->second, mode, 0);
+  if (Path.first != -1) {
+    uint64_t Result = ::fchmodat(Path.first, Path.second, mode, 0);
     if (Result != -1)
       return Result;
   }
@@ -689,8 +691,8 @@ uint64_t FileManager::Readlinkat(int dirfd, const char *pathname, char *buf, siz
 
   FDPathTmpData TmpFilename;
   auto NewPath = GetEmulatedFDPath(pathname, false, TmpFilename);
-  if (NewPath.has_value()) {
-    uint64_t Result = ::readlinkat(NewPath->first, NewPath->second, buf, bufsiz);
+  if (NewPath.first != -1) {
+    uint64_t Result = ::readlinkat(NewPath.first, NewPath.second, buf, bufsiz);
     if (Result != -1)
       return Result;
 
@@ -715,12 +717,12 @@ uint64_t FileManager::Openat([[maybe_unused]] int dirfs, const char *pathname, i
   if (fd == -1) {
     FDPathTmpData TmpFilename;
     auto Path = GetEmulatedFDPath(SelfPath, true, TmpFilename);
-    if (Path.has_value()) {
-      fd = ::openat(Path->first, Path->second, flags, mode);
+    if (Path.first != -1) {
+      fd = ::syscall(SYSCALL_DEF(openat), Path.first, Path.second, flags, mode);
     }
 
     if (fd == -1)
-      fd = ::openat(dirfs, SelfPath, flags, mode);
+      fd = ::syscall(SYSCALL_DEF(openat), dirfs, SelfPath, flags, mode);
   }
 
   if (fd != -1) {
@@ -741,8 +743,8 @@ uint64_t FileManager::Openat2(int dirfs, const char *pathname, FEX::HLE::open_ho
   if (fd == -1) {
     FDPathTmpData TmpFilename;
     auto Path = GetEmulatedFDPath(SelfPath, true, TmpFilename);
-    if (Path.has_value()) {
-      fd = ::syscall(SYSCALL_DEF(openat2), Path->first, Path->second, how, usize);
+    if (Path.first != -1) {
+      fd = ::syscall(SYSCALL_DEF(openat2), Path.first, Path.second, how, usize);
     }
 
     if (fd == -1)
@@ -764,8 +766,8 @@ uint64_t FileManager::Statx(int dirfd, const char *pathname, int flags, uint32_t
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, (flags & AT_SYMLINK_NOFOLLOW) == 0, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = FHU::Syscalls::statx(Path->first, Path->second, flags, mask, statxbuf);
+  if (Path.first != -1) {
+    uint64_t Result = FHU::Syscalls::statx(Path.first, Path.second, flags, mask, statxbuf);
     if (Result != -1)
       return Result;
   }
@@ -778,8 +780,8 @@ uint64_t FileManager::Mknod(const char *pathname, mode_t mode, dev_t dev) {
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, false, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::mknodat(Path->first, Path->second, mode, dev);
+  if (Path.first != -1) {
+    uint64_t Result = ::mknodat(Path.first, Path.second, mode, dev);
     if (Result != -1)
       return Result;
   }
@@ -802,8 +804,8 @@ uint64_t FileManager::NewFSStatAt(int dirfd, const char *pathname, struct stat *
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, (flag & AT_SYMLINK_NOFOLLOW) == 0, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::fstatat(Path->first, Path->second, buf, flag);
+  if (Path.first != -1) {
+    uint64_t Result = ::fstatat(Path.first, Path.second, buf, flag);
     if (Result != -1) {
       return Result;
     }
@@ -817,8 +819,8 @@ uint64_t FileManager::NewFSStatAt64(int dirfd, const char *pathname, struct stat
 
   FDPathTmpData TmpFilename;
   auto Path = GetEmulatedFDPath(SelfPath, (flag & AT_SYMLINK_NOFOLLOW) == 0, TmpFilename);
-  if (Path.has_value()) {
-    uint64_t Result = ::fstatat64(Path->first, Path->second, buf, flag);
+  if (Path.first != -1) {
+    uint64_t Result = ::fstatat64(Path.first, Path.second, buf, flag);
     if (Result != -1) {
       return Result;
     }

Source/Tests/LinuxSyscalls/FileManagement.h

@@ -81,7 +81,7 @@ public:
 
   std::string GetEmulatedPath(const char *pathname, bool FollowSymlink = false);
   using FDPathTmpData = std::array<char[PATH_MAX], 2>;
-  std::optional<std::pair<int, const char*>> GetEmulatedFDPath(const char *pathname, bool FollowSymlink, FDPathTmpData &TmpFilename);
+  std::pair<int, const char*> GetEmulatedFDPath(const char *pathname, bool FollowSymlink, FDPathTmpData &TmpFilename);
 
   std::mutex *GetFDLock() { return &FDLock; }