diff --git a/External/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp b/External/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp index b57cf6119..6c1e1ae21 100644 --- a/External/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp +++ b/External/FEXCore/Source/Interface/IR/Passes/RegisterAllocationPass.cpp @@ -99,6 +99,7 @@ namespace { fextl::unordered_map> BlockPredecessors; fextl::unordered_map> VisitedNodePredecessors; + // Required due to raw new usage. void *operator new(size_t size) { return FEXCore::Allocator::malloc(size); } diff --git a/External/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp b/External/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp index 8dfd07c79..12f38edaa 100644 --- a/External/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp +++ b/External/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp @@ -31,14 +31,6 @@ namespace Alloc::OSAllocator { class OSAllocator_64Bit final : public Alloc::HostAllocator { public: - void *operator new(size_t size) { - return ::malloc(size); - } - - void operator delete(void *ptr) { - return ::free(ptr); - } - OSAllocator_64Bit(); virtual ~OSAllocator_64Bit(); void *AllocateSlab(size_t Size) override { return nullptr; } diff --git a/External/FEXCore/Source/Utils/AllocatorOverride.cpp b/External/FEXCore/Source/Utils/AllocatorOverride.cpp index a7b6faa4a..4c7f4c190 100644 --- a/External/FEXCore/Source/Utils/AllocatorOverride.cpp +++ b/External/FEXCore/Source/Utils/AllocatorOverride.cpp @@ -11,7 +11,6 @@ #include extern "C" { -#ifdef GLIBC_ALLOCATOR_FAULT // The majority of FEX internal code should avoid using the glibc allocator. To ensure glibc allocations don't accidentally slip // in, FEX overrides these glibc functions with faulting variants. // @@ -52,7 +51,7 @@ namespace FEXCore::Allocator { static bool GlobalEvaluate{}; // Enable or disable allocation faulting per-thread. - thread_local uint64_t SkipEvalForThread{}; + static thread_local uint64_t SkipEvalForThread{}; // Internal memory allocation hooks to allow non-faulting allocations through. CALLOC_Hook calloc_ptr = __libc_calloc; @@ -100,7 +99,7 @@ namespace FEXCore::Allocator { } if (SkipEvalForThread) { - // Fault evaluation disabled for this thread. + // Fault evaluation currently disabled for this thread. return; } @@ -155,5 +154,4 @@ extern "C" { FEXCore::Allocator::EvaluateReturnAddress(__builtin_extract_return_addr (__builtin_return_address (0))); return FEXCore::Allocator::aligned_alloc_ptr(a, s); } -#endif } diff --git a/External/FEXCore/include/FEXCore/Debug/InternalThreadState.h b/External/FEXCore/include/FEXCore/Debug/InternalThreadState.h index 98253bdc1..47395bd7c 100644 --- a/External/FEXCore/include/FEXCore/Debug/InternalThreadState.h +++ b/External/FEXCore/include/FEXCore/Debug/InternalThreadState.h @@ -64,6 +64,7 @@ namespace FEXCore::Core { fextl::vector GuestOpcodes; fextl::vector *Relocations; + // Required due to raw new usage. void *operator new(size_t size) { return FEXCore::Allocator::malloc(size); } @@ -127,6 +128,7 @@ namespace FEXCore::Core { std::shared_mutex ObjectCacheRefCounter{}; bool DestroyedByParent{false}; // Should the parent destroy this thread, or it destory itself + // Required due to raw new usage. void *operator new(size_t size) { return FEXCore::Allocator::malloc(size); } diff --git a/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h b/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h index 25241f80a..f9cc621a6 100644 --- a/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h +++ b/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h @@ -170,6 +170,7 @@ public: } } + // Required due to raw new usage. void *operator new(size_t size) { return FEXCore::Allocator::malloc(size); } diff --git a/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h b/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h index c76978aca..e22bc6b18 100644 --- a/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h +++ b/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h @@ -1,4 +1,5 @@ #pragma once +#include #include #include #include @@ -27,17 +28,22 @@ namespace FHU::Filesystem { return access(Path.c_str(), F_OK) == 0; } + enum class CreateDirectoryResult { + CREATED, + EXISTS, + ERROR, + }; /** * @brief Creates a directory at the provided path. * * @param Path The path to create a directory at. * - * @return True if the directory was created or already exists. + * @return Result enum depending. */ - inline bool CreateDirectory(const fextl::string &Path) { + inline CreateDirectoryResult CreateDirectory(const fextl::string &Path) { auto Result = ::mkdir(Path.c_str(), 0777); if (Result == 0) { - return true; + return CreateDirectoryResult::CREATED; } if (Result == -1 && errno == EEXIST) { @@ -45,12 +51,14 @@ namespace FHU::Filesystem { struct stat buf; if (stat(Path.c_str(), &buf) == 0) { // Check to see if the path is a file or folder. Following symlinks. - return S_ISDIR(buf.st_mode); + return S_ISDIR(buf.st_mode) ? + CreateDirectoryResult::EXISTS : + CreateDirectoryResult::ERROR; } } // Couldn't create, or the path that existed wasn't a folder. - return false; + return CreateDirectoryResult::ERROR; } /** @@ -62,14 +70,14 @@ namespace FHU::Filesystem { */ inline bool CreateDirectories(const fextl::string &Path) { // Try to create the directory initially. - if (CreateDirectory(Path)) { + if (CreateDirectory(Path) != CreateDirectoryResult::ERROR) { return true; } // Walk the path in reverse and create paths as we go. fextl::string TmpPath {Path.substr(0, Path.rfind('/', Path.size() - 1))}; if (!TmpPath.empty() && CreateDirectories(TmpPath)) { - return CreateDirectory(Path); + return CreateDirectory(Path) != CreateDirectoryResult::ERROR; } return false; } @@ -93,15 +101,37 @@ namespace FHU::Filesystem { inline fextl::string ParentPath(const fextl::string &Path) { auto LastSeparator = Path.rfind('/'); + if (LastSeparator == fextl::string::npos) { // No separator. Likely relative `.`, `..`, ``, or empty string. + if (Path == "." || Path == "..") { + // In this edge-case, return nothing to match std::filesystem::path::parent_path behaviour. + return {}; + } return Path; } - return Path.substr(0, LastSeparator); + if (LastSeparator == 0) { + // In the case of root, just return. + return "/"; + } + + auto SubString = Path.substr(0, LastSeparator); + + while (SubString.size() > 1 && SubString.ends_with("/")) { + // If the substring still ended with `/` then we need to string that off as well. + --LastSeparator; + SubString = Path.substr(0, LastSeparator); + } + + return SubString; } inline bool IsRelative(const fextl::string &Path) { + return !Path.starts_with('/'); + } + + inline bool IsAbsolute(const fextl::string &Path) { return Path.starts_with('/'); } @@ -111,22 +141,23 @@ namespace FHU::Filesystem { OVERWRITE_EXISTING, }; + /** + * @brief Copy a file from a location to another + * + * Behaves similarly to std::filesystem::copy_file but with less copy options. + * + * @param From Source file location. + * @param To Destination file location. + * @param Options Copy options. + * + * @return True if the copy succeeded, false otherwise. + */ inline bool CopyFile(const fextl::string &From, const fextl::string &To, CopyOptions Options = CopyOptions::NONE) { - bool DestTested = false; - bool DestExists = false; - if (Options == CopyOptions::SKIP_EXISTING && Exists(To.c_str())) { - DestTested = true; - DestExists = Exists(To.c_str()); - if (DestExists) { - // If the destination file exists already and the skip existing flag is set then - // return true without error. - return true; - } - } - - if (!DestTested) { - DestTested = true; - DestExists = Exists(To.c_str()); + const bool DestExists = Exists(To); + if (Options == CopyOptions::SKIP_EXISTING && DestExists) { + // If the destination file exists already and the skip existing flag is set then + // return true without error. + return true; } if (Options == CopyOptions::OVERWRITE_EXISTING && DestExists) { @@ -177,15 +208,16 @@ namespace FHU::Filesystem { return {}; } + const auto IsAbsolutePath = IsAbsolute(Path); + const auto EndsWithSeparator = Path.ends_with('/'); // Count the number of separators up front const auto SeparatorCount = std::count(Path.begin(), Path.end(), '/'); - // Needs to be a list so iterators aren't invalidated while erasing. - // Maximum number of list regions will be the counted separators plus 2. - // Multiplied by two since it allocates * 2 the size (32-bytes per element) - // Use a small stack allocator to be more optimal. - // Needs to be a list so iterators aren't invalidated while erasing. - size_t DataSize = sizeof(std::string_view) * (SeparatorCount + 2) * 2; + // Use std::list to store path elements to avoid iterator invalidation on insert/erase. + // The list is allocated on stack to be more optimal. The size is determined by the + // maximum number of list objects (separator count plus 2) multiplied by the list + // element size (32-bytes per element: the string_view itself and the prev/next pointers). + size_t DataSize = (sizeof(std::string_view) + sizeof(void*) * 2) * (SeparatorCount + 2); void *Data = alloca(DataSize); fextl::pmr::fixed_size_monotonic_buffer_resource mbr(Data, DataSize); std::pmr::polymorphic_allocator pa {&mbr}; @@ -205,9 +237,8 @@ namespace FHU::Filesystem { const auto Size = End - Begin; ExpectedStringSize += Size; - // If Size is zero then only insert if Parts is empty. - // Ensures that we don't remove an initial `/` by itself - if (Parts.empty() || Size != 0) { + // Only insert parts that contain data. + if (Size != 0) { Parts.emplace_back(std::string_view(Begin, End)); } @@ -221,9 +252,10 @@ namespace FHU::Filesystem { size_t CurrentIterDistance{}; for (auto iter = Parts.begin(); iter != Parts.end();) { - if (*iter == ".") { + auto &Part = *iter; + if (Part == ".") { // Erase '.' directory parts if not at root. - if (CurrentIterDistance > 0) { + if (CurrentIterDistance > 0 || IsAbsolutePath) { // Erasing this iterator, don't increase iter distances iter = Parts.erase(iter); --ExpectedStringSize; @@ -231,9 +263,8 @@ namespace FHU::Filesystem { } } - if (*iter == "..") { - auto Distance = std::distance(Parts.begin(), iter); - if (Distance > 0) { + if (Part == "..") { + if (CurrentIterDistance > 0) { // If not at root then remove both this iterator and the previous one. // ONLY if the previous iterator is also not ".." // @@ -257,6 +288,12 @@ namespace FHU::Filesystem { continue; } } + else if (IsAbsolutePath) { + // `..` at the base. Just remove this + ExpectedStringSize -= 2; + iter = Parts.erase(iter); + continue; + } } // Interator distance increased by one. @@ -264,28 +301,12 @@ namespace FHU::Filesystem { ++iter; } - fextl::string OutputPath{}; - OutputPath.reserve(ExpectedStringSize + Parts.size()); - auto iter = Parts.begin(); - for (size_t i = 0; i < Parts.size(); ++i, ++iter) { - auto &Part = *iter; - OutputPath += Part; - - const bool IsFinal = (i + 1) == Parts.size(); - // If the final element is ellipses then don't apply a separator. - // Otherwise if it is anything else, apply a separator. - const bool IsEllipses = Part == "." || Part == ".."; - - const bool NeedsSeparator = - !IsFinal || // Needs a separator if this isn't the final part. - (IsFinal && !IsEllipses); // Needs a separator if the final part doesn't end in `.` or `..` - - if (NeedsSeparator) { - OutputPath += '/'; - } - } - - return OutputPath; + // Add a final separator unless the last element is ellipses. + const bool NeedsFinalSeparator = EndsWithSeparator && (!Parts.empty() && Parts.back() != "." && Parts.back() != ".."); + return fextl::fmt::format("{}{}{}", + IsAbsolutePath ? "/" : "", + fmt::join(Parts, "/"), + NeedsFinalSeparator ? "/" : ""); } } diff --git a/unittests/APITests/CMakeLists.txt b/unittests/APITests/CMakeLists.txt index f6d529915..bfe8fcdf3 100644 --- a/unittests/APITests/CMakeLists.txt +++ b/unittests/APITests/CMakeLists.txt @@ -1,6 +1,6 @@ set (TESTS InterruptableConditionVariable - LexicallyNormal + Filesystem ) list(APPEND LIBS FEXCore) diff --git a/unittests/APITests/Filesystem.cpp b/unittests/APITests/Filesystem.cpp new file mode 100644 index 000000000..6ebadf39d --- /dev/null +++ b/unittests/APITests/Filesystem.cpp @@ -0,0 +1,85 @@ +#include +#include +#include + +TEST_CASE("LexicallyNormal") { + auto Path = GENERATE("", + "/", + "/./", + "//.", + "//./", + "//.//", + + ".", + "..", + ".//", + "../../", + "././", + "./../", + "./../", + "./.././.././.", + "./.././.././..", + + "./foo1/../", + "foo4/.///bar/../", + "foo5/././", + + "foo6/", + "foo7/test", + "foo8/test/", + "foo9/./../test/", + + "/../..", + "...", + "/...", + "foo10/...", + "/..", + "/foo11/../../bar" + ); + + REQUIRE(std::string_view(FHU::Filesystem::LexicallyNormal(Path)) == std::string_view(std::filesystem::path(Path).lexically_normal().string())); +} + +TEST_CASE("LexicallyNormalDifferences", "[!shouldfail]") { + auto Path = GENERATE("", + // std::fs here keeps the `/` after `foo2/` + // FEX algorithm doesn't keep behaviour here. + "foo2/./bar/..", // std::fs -> "foo2/" + "foo2/.///bar/.." // std::fs -> "foo3/" + ); + + REQUIRE(std::string_view(FHU::Filesystem::LexicallyNormal(Path)) == std::string_view(std::filesystem::path(Path).lexically_normal().string())); +} + +TEST_CASE("ParentPath") { + auto Path = GENERATE("", + "/", + "/./", + "//.", + "//./", + "//.//", + + ".", + "..", + ".//", + "../../", + "././", + "./../", + "./../", + "./.././.././.", + "./.././.././..", + + "./foo/../", + "foo/./bar/..", + "foo/.///bar/..", + "foo/.///bar/../", + "foo/././" + "...", + "/...", + "foo/...", + "/..", + "/foo/../../bar" + ); + + REQUIRE(std::string_view(FHU::Filesystem::ParentPath(Path)) == std::string_view(std::filesystem::path(Path).parent_path().string())); +} diff --git a/unittests/APITests/LexicallyNormal.cpp b/unittests/APITests/LexicallyNormal.cpp deleted file mode 100644 index 5ce4ed599..000000000 --- a/unittests/APITests/LexicallyNormal.cpp +++ /dev/null @@ -1,31 +0,0 @@ -#include -#include -#include - -#define TestPath(Path) \ - -TEST_CASE("LexicallyNormal") { - auto Path = GENERATE("", - "/", - "/./", - "//.", - "//./", - "//.//", - - ".", - "..", - ".//", - "../../", - "././", - "./../", - "./../", - "./.././.././.", - "./.././.././..", - - "./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())); -}