Review code

External/FEXCore/Source/Common/Paths.cpp

@@ -71,7 +71,7 @@ namespace FEXCore::Paths {
     EntryCache = CachePath + "/EntryCache/";
 
     // Ensure the folder structure is created for our Data
-    if (!FHU::Filesystem::Exists(EntryCache.c_str()) &&
+    if (!FHU::Filesystem::Exists(EntryCache) &&
         !FHU::Filesystem::CreateDirectories(EntryCache)) {
       LogMan::Msg::DFmt("Couldn't create EntryCache directory: '{}'", EntryCache);
     }

External/FEXCore/Source/Interface/Config/Config.cpp

@@ -146,7 +146,7 @@ namespace JSON {
       }
 
       // Ensure the folder structure is created for our configuration
-      if (!FHU::Filesystem::Exists(ConfigDir.c_str()) &&
+      if (!FHU::Filesystem::Exists(ConfigDir) &&
           !FHU::Filesystem::CreateDirectories(ConfigDir)) {
         // Let's go local in this case
         return "./";
@@ -178,7 +178,7 @@ namespace JSON {
     fextl::string ConfigFile = GetConfigDirectory(Global);
 
     if (!Global &&
-        !FHU::Filesystem::Exists(ConfigFile.c_str()) &&
+        !FHU::Filesystem::Exists(ConfigFile) &&
         !FHU::Filesystem::CreateDirectories(ConfigFile)) {
       LogMan::Msg::DFmt("Couldn't create config directory: '{}'", ConfigFile);
       // Let's go local in this case
@@ -189,7 +189,7 @@ namespace JSON {
 
     // Attempt to create the local folder if it doesn't exist
     if (!Global &&
-        !FHU::Filesystem::Exists(ConfigFile.c_str()) &&
+        !FHU::Filesystem::Exists(ConfigFile) &&
         !FHU::Filesystem::CreateDirectories(ConfigFile)) {
       // Let's go local in this case
       return "./" + Filename + ".json";
@@ -354,7 +354,7 @@ namespace JSON {
       }
 
       // Only return if it exists
-      if (FHU::Filesystem::Exists(PathName.c_str())) {
+      if (FHU::Filesystem::Exists(PathName)) {
         return PathName;
       }
     }
@@ -371,9 +371,9 @@ namespace JSON {
       // HostThunks: $CMAKE_INSTALL_PREFIX/lib/fex-emu/HostThunks/
       // GuestThunks: $CMAKE_INSTALL_PREFIX/share/fex-emu/GuestThunks/
       if (!ContainerPrefix.empty() && !PathName.empty()) {
-        if (!FHU::Filesystem::Exists(PathName.c_str())) {
+        if (!FHU::Filesystem::Exists(PathName)) {
           auto ContainerPath = ContainerPrefix + PathName;
-          if (FHU::Filesystem::Exists(ContainerPath.c_str())) {
+          if (FHU::Filesystem::Exists(ContainerPath)) {
             return ContainerPath;
           }
         }
@@ -725,7 +725,7 @@ namespace JSON {
       EnvMap[Key] = Value;
     }
 
-    std::function GetVar = [](EnvMapType &EnvMap, const std::string_view id)  -> std::optional<std::string_view> {
+    auto GetVar = [](EnvMapType &EnvMap, const std::string_view id)  -> std::optional<std::string_view> {
       if (EnvMap.find(id) != EnvMap.end())
         return EnvMap.at(id);
 

External/FEXCore/Source/Interface/Context/Context.h

@@ -72,14 +72,6 @@ namespace FEXCore::Context {
 
   class ContextImpl final : public FEXCore::Context::Context {
     public:
-      void *operator new(size_t size) {
-        return FEXCore::Allocator::malloc(size);
-      }
-
-      void operator delete(void *ptr) {
-        return FEXCore::Allocator::free(ptr);
-      }
-
       // Context base class implementation.
       bool InitializeContext() override;
 

External/FEXCore/Source/Interface/Core/LookupCache.cpp

@@ -15,11 +15,11 @@ $end_info$
 
 namespace FEXCore {
 LookupCache::LookupCache(FEXCore::Context::ContextImpl *CTX)
-  : ctx {CTX} {
+  : BlockLinks_mbr { fextl::pmr::get_default_resource() }
+  , ctx {CTX} {
 
   TotalCacheSize = ctx->Config.VirtualMemSize / 4096 * 8 + CODE_SIZE + L1_SIZE;
-  BlockLinks_mbr = fextl::make_unique<std::pmr::monotonic_buffer_resource>(fextl::pmr::get_default_resource());
-  BlockLinks_pma = fextl::make_unique<std::pmr::polymorphic_allocator<std::byte>>(BlockLinks_mbr.get());
+  BlockLinks_pma = fextl::make_unique<std::pmr::polymorphic_allocator<std::byte>>(&BlockLinks_mbr);
   // Setup our PMR map.
   BlockLinks = BlockLinks_pma->new_object<BlockLinksMapType>();
 
@@ -73,8 +73,6 @@ void LookupCache::ClearCache() {
 
   // Clear L1 and L2 by clearing the full cache.
   madvise(reinterpret_cast<void*>(PagePointer), TotalCacheSize, MADV_DONTNEED);
-  // Clear the BlockLinks allocator which frees the BlockLinks map implicitly.
-  BlockLinks_mbr->release();
   // Allocate a new pointer from the BlockLinks pma again.
   BlockLinks = BlockLinks_pma->new_object<BlockLinksMapType>();
   // All code is gone, clear the block list

External/FEXCore/Source/Interface/Core/LookupCache.h

@@ -243,7 +243,7 @@ private:
   // walking each block member and destructing objects.
   //
   // This makes `BlockLinks` look like a raw pointer that could memory leak, but since it is backed by the MBR, it won't.
-  fextl::unique_ptr<std::pmr::monotonic_buffer_resource> BlockLinks_mbr;
+  std::pmr::monotonic_buffer_resource BlockLinks_mbr;
   using BlockLinksMapType = std::pmr::map<BlockLinkTag, std::function<void()>>;
   fextl::unique_ptr<std::pmr::polymorphic_allocator<std::byte>> BlockLinks_pma;
   BlockLinksMapType *BlockLinks;

External/FEXCore/Source/Utils/AllocatorOverride.cpp

@@ -94,6 +94,8 @@ namespace FEXCore::Allocator {
     auto Res = fmt::format_to_n(Tmp, 512, "Allocation from 0x{:x}\n", reinterpret_cast<uint64_t>(Return));
     Tmp[Res.size] = 0;
     write(STDERR_FILENO, Tmp, Res.size);
+
+    // Fault the execution to stop it in its tracks.
     FEX_TRAP_EXECUTION;
   }
 }

External/FEXCore/Source/Utils/Telemetry.cpp

@@ -30,7 +30,7 @@ namespace FEXCore::Telemetry {
     DataDirectory += "Telemetry/";
 
     // Ensure the folder structure is created for our configuration
-    if (!FHU::Filesystem::Exists(DataDirectory.c_str()) &&
+    if (!FHU::Filesystem::Exists(DataDirectory) &&
         !FHU::Filesystem::CreateDirectories(DataDirectory)) {
       LogMan::Msg::IFmt("Couldn't create telemetry Folder");
     }
@@ -40,7 +40,7 @@ namespace FEXCore::Telemetry {
     auto DataDirectory = Config::GetDataDirectory();
     DataDirectory += "Telemetry/" + ApplicationName + ".telem";
 
-    if (FHU::Filesystem::Exists(DataDirectory.c_str())) {
+    if (FHU::Filesystem::Exists(DataDirectory)) {
       // If the file exists, retain a single backup
       auto Backup = DataDirectory + ".1";
       FHU::Filesystem::CopyFile(DataDirectory, Backup, FHU::Filesystem::CopyOptions::OVERWRITE_EXISTING);

External/FEXCore/include/FEXCore/Utils/Allocator.h

@@ -27,6 +27,16 @@ namespace FEXCore::Allocator {
       FEX_DEFAULT_VISIBILITY ~YesIKnowImNotSupposedToUseTheGlibcAllocator();
       FEX_DEFAULT_VISIBILITY static void HardDisable();
   };
+
+  class FEX_DEFAULT_VISIBILITY GLIBCScopedFault final {
+    public:
+      GLIBCScopedFault() {
+        FEXCore::Allocator::SetupFaultEvaluate();
+      }
+      ~GLIBCScopedFault() {
+        FEXCore::Allocator::ClearFaultEvaluate();
+      }
+  };
 #else
   FEX_DEFAULT_VISIBILITY inline void SetupFaultEvaluate() {}
   FEX_DEFAULT_VISIBILITY inline void ClearFaultEvaluate() {}
@@ -37,6 +47,16 @@ namespace FEXCore::Allocator {
     FEX_DEFAULT_VISIBILITY ~YesIKnowImNotSupposedToUseTheGlibcAllocator() {}
     FEX_DEFAULT_VISIBILITY static inline void HardDisable() {}
   };
+
+  class FEX_DEFAULT_VISIBILITY GLIBCScopedFault final {
+    public:
+      GLIBCScopedFault() {
+        // nop
+      }
+      ~GLIBCScopedFault() {
+        // nop
+      }
+  };
 #endif
 
   struct MemoryRegion {

External/FEXCore/include/FEXCore/fextl/memory_resource.h

@@ -37,7 +37,7 @@ namespace fextl {
      */
     class fixed_size_monotonic_buffer_resource final : public std::pmr::memory_resource {
     public:
-        fixed_size_monotonic_buffer_resource(void* Base, size_t Size)
+        fixed_size_monotonic_buffer_resource(void* Base, [[maybe_unused]] size_t Size)
             : Ptr {reinterpret_cast<uint64_t>(Base)}
 #if defined(ASSERTIONS_ENABLED) && ASSERTIONS_ENABLED
             , PtrEnd {reinterpret_cast<uint64_t>(Base) + Size}

FEXHeaderUtils/FEXHeaderUtils/Filesystem.h

@@ -23,6 +23,10 @@ namespace FHU::Filesystem {
     return access(Path, F_OK) == 0;
   }
 
+  inline bool Exists(const fextl::string &Path) {
+    return access(Path.c_str(), F_OK) == 0;
+  }
+
   /**
    * @brief Creates a directory at the provided path.
    *

Source/Common/FEXServerClient.cpp

@@ -215,7 +215,7 @@ namespace FEXServerClient {
       fextl::string FEXServerPath = FHU::Filesystem::ParentPath(InterpreterPath) + "/FEXServer";
       // Check if a local FEXServer next to FEXInterpreter exists
       // If it does then it takes priority over the installed one
-      if (!FHU::Filesystem::Exists(FEXServerPath.c_str())) {
+      if (!FHU::Filesystem::Exists(FEXServerPath)) {
         FEXServerPath = "FEXServer";
       }
 

Source/Tests/FEXLoader.cpp

@@ -180,7 +180,7 @@ void InterpreterHandler(fextl::string *Filename, fextl::string const &RootFS, fe
 void RootFSRedirect(fextl::string *Filename, fextl::string const &RootFS) {
   auto RootFSLink = ELFCodeLoader::ResolveRootfsFile(*Filename, RootFS);
 
-  if (FHU::Filesystem::Exists(RootFSLink.c_str())) {
+  if (FHU::Filesystem::Exists(RootFSLink)) {
     *Filename = RootFSLink;
   }
 }
@@ -199,7 +199,7 @@ bool IsInterpreterInstalled() {
 }
 
 int main(int argc, char **argv, char **const envp) {
-  FEXCore::Allocator::SetupFaultEvaluate();
+  FEXCore::Allocator::GLIBCScopedFault GLIBFaultScope;
   const bool IsInterpreter = RanAsInterpreter(argv[0]);
 
   ExecutedWithFD = getauxval(AT_EXECFD) != 0;
@@ -218,7 +218,6 @@ int main(int argc, char **argv, char **const envp) {
 
   if (Program.ProgramPath.empty() && !FEXFD) {
     // Early exit if we weren't passed an argument
-    FEXCore::Allocator::ClearFaultEvaluate();
     return 0;
   }
 
@@ -243,7 +242,6 @@ int main(int argc, char **argv, char **const envp) {
   // Ensure FEXServer is setup before config options try to pull CONFIG_ROOTFS
   if (!FEXServerClient::SetupClient(argv[0])) {
     LogMan::Msg::EFmt("FEXServerClient: Failure to setup client");
-    FEXCore::Allocator::ClearFaultEvaluate();
     return -1;
   }
 
@@ -297,11 +295,10 @@ int main(int argc, char **argv, char **const envp) {
   RootFSRedirect(&Program.ProgramPath, LDPath());
   InterpreterHandler(&Program.ProgramPath, LDPath(), &Args);
 
-  if (!ExecutedWithFD && !FEXFD && !FHU::Filesystem::Exists(Program.ProgramPath.c_str())) {
+  if (!ExecutedWithFD && !FEXFD && !FHU::Filesystem::Exists(Program.ProgramPath)) {
     // Early exit if the program passed in doesn't exist
     // Will prevent a crash later
     fmt::print(stderr, "{}: command not found\n", Program.ProgramPath);
-    FEXCore::Allocator::ClearFaultEvaluate();
     return -ENOEXEC;
   }
 
@@ -333,7 +330,6 @@ int main(int argc, char **argv, char **const envp) {
       fmt::print(stderr, "Use FEXRootFSFetcher to download a RootFS\n");
     }
 #endif
-    FEXCore::Allocator::ClearFaultEvaluate();
     return -ENOEXEC;
   }
 
@@ -423,7 +419,6 @@ int main(int argc, char **argv, char **const envp) {
     if (!Loader.MapMemory(SyscallHandler.get())) {
       // failed to map
       LogMan::Msg::EFmt("Failed to map %d-bit elf file.", Loader.Is64BitMode() ? 64 : 32);
-      FEXCore::Allocator::ClearFaultEvaluate();
       return -ENOEXEC;
     }
   }
@@ -535,7 +530,6 @@ int main(int argc, char **argv, char **const envp) {
   // Allocator is now original system allocator
   FEXCore::Telemetry::Shutdown(Program.ProgramName);
   FEXCore::Profiler::Shutdown();
-  FEXCore::Allocator::ClearFaultEvaluate();
 
   if (ShutdownReason == FEXCore::Context::ExitReason::EXIT_SHUTDOWN) {
     return ProgramStatus;

Source/Tests/LinuxSyscalls/FileManagement.cpp

@@ -295,7 +295,7 @@ FileManager::FileManager(FEXCore::Context::Context *ctx)
 
       void SetupOverlay(const ThunkDBObject& DBDepend) {
           auto ThunkPath = fextl::fmt::format("{}/{}", ThunkGuestPath, DBDepend.LibraryName);
-          if (!FHU::Filesystem::Exists(ThunkPath.c_str())) {
+          if (!FHU::Filesystem::Exists(ThunkPath)) {
             if (!Is64BitMode) {
               // Guest libraries not existing is expected since not all libraries are thunked on 32-bit
               return;
@@ -655,14 +655,11 @@ uint64_t FileManager::Readlinkat(int dirfd, const char *pathname, char *buf, siz
         dirfd != AT_FDCWD) {
     // Passed in a dirfd that isn't magic FDCWD
     // We need to get the path from the fd now
-    char Tmp[PATH_MAX];
+    char Tmp[PATH_MAX] = "";
     auto PathLength = FEX::get_fdpath(dirfd, Tmp);
     if (PathLength != -1) {
       Path = fextl::string(Tmp, PathLength);
     }
-    else {
-      Path = "";
-    }
 
     if (pathname) {
       if (!Path.empty()) {

Source/Tests/LinuxSyscalls/Syscalls.cpp

@@ -257,7 +257,7 @@ uint64_t ExecveHandler(const char *pathname, char* const* argv, char* const* env
     // For absolute paths, check the rootfs first (if available)
     if (pathname[0] == '/') {
       auto Path = FEX::HLE::_SyscallHandler->FM.GetEmulatedPath(pathname, true);
-      if (!Path.empty() && FHU::Filesystem::Exists(Path.c_str())) {
+      if (!Path.empty() && FHU::Filesystem::Exists(Path)) {
         Filename = Path;
       }
       else {
@@ -268,7 +268,7 @@ uint64_t ExecveHandler(const char *pathname, char* const* argv, char* const* env
       Filename = pathname;
     }
 
-    bool exists = FHU::Filesystem::Exists(Filename.c_str());
+    bool exists = FHU::Filesystem::Exists(Filename);
     if (!exists) {
       return -ENOENT;
     }

Source/Tests/TestHarnessRunner.cpp

@@ -115,7 +115,7 @@ private:
 }
 
 int main(int argc, char **argv, char **const envp) {
-  FEXCore::Allocator::SetupFaultEvaluate();
+  FEXCore::Allocator::GLIBCScopedFault GLIBFaultScope;
   LogMan::Throw::InstallHandler(AssertHandler);
   LogMan::Msg::InstallHandler(MsgHandler);
   FEXCore::Config::Initialize();
@@ -127,7 +127,6 @@ int main(int argc, char **argv, char **const envp) {
 
   if (Args.size() < 2) {
     LogMan::Msg::EFmt("Not enough arguments");
-    FEXCore::Allocator::ClearFaultEvaluate();
     return -1;
   }
 
@@ -186,7 +185,6 @@ int main(int argc, char **argv, char **const envp) {
 
   if (TestUnsupported) {
     FEXCore::Context::Context::DestroyContext(CTX);
-    FEXCore::Allocator::ClearFaultEvaluate();
     return 0;
   }
 
@@ -212,7 +210,6 @@ int main(int argc, char **argv, char **const envp) {
     if (!Loader.MapMemory(SyscallHandler.get())) {
       // failed to map
       LogMan::Msg::EFmt("Failed to map %d-bit elf file.", Loader.Is64BitMode() ? 64 : 32);
-      FEXCore::Allocator::ClearFaultEvaluate();
       return -ENOEXEC;
     }
 
@@ -222,7 +219,6 @@ int main(int argc, char **argv, char **const envp) {
     bool Result1 = CTX->InitCore(Loader.DefaultRIP(), Loader.GetStackPointer());
 
     if (!Result1) {
-      FEXCore::Allocator::ClearFaultEvaluate();
       return 1;
     }
 
@@ -242,7 +238,6 @@ int main(int argc, char **argv, char **const envp) {
     if (!Loader.MapMemory()) {
       // failed to map
       LogMan::Msg::EFmt("Failed to map %d-bit elf file.", Loader.Is64BitMode() ? 64 : 32);
-      FEXCore::Allocator::ClearFaultEvaluate();
       return -ENOEXEC;
     }
 
@@ -265,7 +260,6 @@ int main(int argc, char **argv, char **const envp) {
   LogMan::Msg::UnInstallHandlers();
 
   FEXCore::Allocator::ClearHooks();
-  FEXCore::Allocator::ClearFaultEvaluate();
   return Passed ? 0 : -1;
 }
 

unittests/APITests/LexicallyNormal.cpp

@@ -3,28 +3,29 @@
 #include <FEXHeaderUtils/Filesystem.h>
 
 #define TestPath(Path) \
-  REQUIRE(std::string_view(FHU::Filesystem::LexicallyNormal(Path)) == std::string_view(std::filesystem::path(Path).lexically_normal().string()));
 
 TEST_CASE("LexicallyNormal") {
-  TestPath("");
-  TestPath("/");
-  TestPath("/./");
-  TestPath("//.");
-  TestPath("//./");
-  TestPath("//.//");
+  auto Path = GENERATE("",
+  "/",
+  "/./",
+  "//.",
+  "//./",
+  "//.//",
 
-  TestPath(".");
-  TestPath("..");
-  TestPath(".//");
-  TestPath("../../");
-  TestPath("././");
-  TestPath("./../");
-  TestPath("./../");
-  TestPath("./.././.././.");
-  TestPath("./.././.././..");
+  ".",
+  "..",
+  ".//",
+  "../../",
+  "././",
+  "./../",
+  "./../",
+  "./.././.././.",
+  "./.././.././..",
 
-  TestPath("./foo/../");
-  TestPath("foo/./bar/..");
-  TestPath("foo/.///bar/..");
-  TestPath("foo/.///bar/../");
+  "./foo/../",
+  "foo/./bar/..",
+  "foo/.///bar/..",
+  "foo/.///bar/../");
+
+  REQUIRE(std::string_view(FHU::Filesystem::LexicallyNormal(Path)) == std::string_view(std::filesystem::path(Path).lexically_normal().string()));
 }