diff --git a/External/FEXCore/Source/Interface/Context/Context.h b/External/FEXCore/Source/Interface/Context/Context.h index 739a7d395..5c815e5f5 100644 --- a/External/FEXCore/Source/Interface/Context/Context.h +++ b/External/FEXCore/Source/Interface/Context/Context.h @@ -168,7 +168,7 @@ namespace FEXCore::Context { void SetAOTIRLoader(std::function CacheReader) override { IRCaptureCache.SetAOTIRLoader(CacheReader); } - void SetAOTIRWriter(std::function(const fextl::string&)> CacheWriter) override { + void SetAOTIRWriter(std::function(const fextl::string&)> CacheWriter) override { IRCaptureCache.SetAOTIRWriter(CacheWriter); } void SetAOTIRRenamer(std::function CacheRenamer) override { diff --git a/External/FEXCore/Source/Interface/Core/GdbServer.cpp b/External/FEXCore/Source/Interface/Core/GdbServer.cpp index 9d6c22b51..52f578753 100644 --- a/External/FEXCore/Source/Interface/Core/GdbServer.cpp +++ b/External/FEXCore/Source/Interface/Core/GdbServer.cpp @@ -1279,6 +1279,7 @@ void GdbServer::StartThread() { } void GdbServer::OpenListenSocket() { + // getaddrinfo allocates memory that can't be removed. FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator glibc; struct addrinfo hints, *res; diff --git a/External/FEXCore/Source/Interface/IR/AOTIR.cpp b/External/FEXCore/Source/Interface/IR/AOTIR.cpp index a537501d9..ab3bee5ce 100644 --- a/External/FEXCore/Source/Interface/IR/AOTIR.cpp +++ b/External/FEXCore/Source/Interface/IR/AOTIR.cpp @@ -199,6 +199,7 @@ namespace FEXCore::IR { for (;;) { // This code is tricky to refactor so it doesn't allocate memory through glibc. + // The moved std::function object deallocates memory at the end of scope. FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator glibc; AOTIRCaptureCacheWriteoutLock.lock(); @@ -329,7 +330,7 @@ namespace FEXCore::IR { auto RADataCopyDeleter = RADataCopy.get_deleter(); auto IRListCopy = IRList->CreateCopy(); - // This code is tricky to refactor so it doesn't allocate memory through glibc. + // The lambda is converted to std::function. This is tricky to refactor so it doesn't allocate memory through glibc. FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator glibc; AOTIRCaptureCacheWriteoutQueue_Append([this, LocalRIP, LocalStartAddr, Length, hash, IRListCopy, RADataCopy=RADataCopy.release(), RADataCopyDeleter, FileId]() { diff --git a/External/FEXCore/Source/Interface/IR/AOTIR.h b/External/FEXCore/Source/Interface/IR/AOTIR.h index 4ab171437..334a4eaf9 100644 --- a/External/FEXCore/Source/Interface/IR/AOTIR.h +++ b/External/FEXCore/Source/Interface/IR/AOTIR.h @@ -69,7 +69,7 @@ namespace FEXCore::IR { }; struct AOTIRCaptureCacheEntry { - fextl::unique_ptr Stream; + fextl::unique_ptr Stream; fextl::map Index; void AppendAOTIRCaptureCache(uint64_t GuestRIP, uint64_t Start, uint64_t Length, uint64_t Hash, FEXCore::IR::IRListView *IRList, FEXCore::IR::RegisterAllocationData *RAData); @@ -125,7 +125,7 @@ namespace FEXCore::IR { AOTIRLoader = CacheReader; } - void SetAOTIRWriter(std::function(const fextl::string&)> CacheWriter) { + void SetAOTIRWriter(std::function(const fextl::string&)> CacheWriter) { AOTIRWriter = CacheWriter; } @@ -145,7 +145,7 @@ namespace FEXCore::IR { FEXCore::IR::AOTCacheType AOTIRCache; std::function AOTIRLoader; - std::function(const fextl::string&)> AOTIRWriter; + std::function(const fextl::string&)> AOTIRWriter; std::function AOTIRRenamer; fextl::unordered_map AOTIRCaptureCacheMap; }; diff --git a/External/FEXCore/Source/Utils/FileLoading.cpp b/External/FEXCore/Source/Utils/FileLoading.cpp index 31d6dabe5..7a956affc 100644 --- a/External/FEXCore/Source/Utils/FileLoading.cpp +++ b/External/FEXCore/Source/Utils/FileLoading.cpp @@ -10,7 +10,7 @@ namespace FEXCore::FileLoading { template -bool LoadFileImpl(T &Data, const fextl::string &Filepath, size_t FixedSize) { +static bool LoadFileImpl(T &Data, const fextl::string &Filepath, size_t FixedSize) { int FD = open(Filepath.c_str(), O_RDONLY); if (FD == -1) { diff --git a/External/FEXCore/include/FEXCore/Core/Context.h b/External/FEXCore/include/FEXCore/Core/Context.h index 6626f3274..13e35daac 100644 --- a/External/FEXCore/include/FEXCore/Core/Context.h +++ b/External/FEXCore/include/FEXCore/Core/Context.h @@ -70,14 +70,15 @@ namespace FEXCore::Context { std::unique_lock lock; }; - class AOTIRWriterFD { + /** + * @brief IR Serialization handler class. + */ + class AOTIRWriter { public: - virtual ~AOTIRWriterFD() { - } + virtual ~AOTIRWriter() = default; virtual void Write(const void* Data, size_t Size) = 0; virtual size_t Offset() = 0; virtual void Close() = 0; - private: }; struct VDSOSigReturn { @@ -265,7 +266,7 @@ namespace FEXCore::Context { FEX_DEFAULT_VISIBILITY virtual void UnloadAOTIRCacheEntry(FEXCore::IR::AOTIRCacheEntry *Entry) = 0; FEX_DEFAULT_VISIBILITY virtual void SetAOTIRLoader(std::function CacheReader) = 0; - FEX_DEFAULT_VISIBILITY virtual void SetAOTIRWriter(std::function(const fextl::string&)> CacheWriter) = 0; + FEX_DEFAULT_VISIBILITY virtual void SetAOTIRWriter(std::function(const fextl::string&)> CacheWriter) = 0; FEX_DEFAULT_VISIBILITY virtual void SetAOTIRRenamer(std::function CacheRenamer) = 0; FEX_DEFAULT_VISIBILITY virtual void FinalizeAOTIRCache() = 0; diff --git a/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h b/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h index 786fb31e6..94b22961e 100644 --- a/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h +++ b/External/FEXCore/include/FEXCore/IR/IntrusiveIRList.h @@ -171,7 +171,7 @@ public: } } - void Serialize(FEXCore::Context::AOTIRWriterFD& stream) const { + void Serialize(FEXCore::Context::AOTIRWriter& stream) const { void *nul = nullptr; //void *IRDataInternal; stream.Write((const char*)&nul, sizeof(nul)); diff --git a/External/FEXCore/include/FEXCore/IR/RegisterAllocationData.h b/External/FEXCore/include/FEXCore/IR/RegisterAllocationData.h index 60dadf72d..b98e56a7f 100644 --- a/External/FEXCore/include/FEXCore/IR/RegisterAllocationData.h +++ b/External/FEXCore/include/FEXCore/IR/RegisterAllocationData.h @@ -58,7 +58,7 @@ class FEX_PACKED RegisterAllocationData { UniquePtr CreateCopy() const; - void Serialize(FEXCore::Context::AOTIRWriterFD& stream) const { + void Serialize(FEXCore::Context::AOTIRWriter& stream) const { stream.Write((const char*)&SpillSlotCount, sizeof(SpillSlotCount)); stream.Write((const char*)&MapCount, sizeof(MapCount)); // RAData (inline) diff --git a/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h b/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h index 999c9e394..757a7a4d2 100644 --- a/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h +++ b/FEXHeaderUtils/FEXHeaderUtils/Filesystem.h @@ -203,13 +203,10 @@ namespace FHU::Filesystem { /** * @brief Renames a file and overwrites if it already exists. * - * @param From - * @param To - * - * @return True if the rename occured, False otherwise. + * @return No error on rename. */ - inline bool RenameFile(const fextl::string &From, const fextl::string &To) { - return rename(From.c_str(), To.c_str()) == 0; + [[nodiscard]] inline std::error_code RenameFile(const fextl::string &From, const fextl::string &To) { + return rename(From.c_str(), To.c_str()) == 0 ? std::error_code{} : std::make_error_code(std::errc::io_error); } inline fextl::string LexicallyNormal(const fextl::string &Path) { diff --git a/Source/Tests/AOT/AOTGenerator.cpp b/Source/Tests/AOT/AOTGenerator.cpp index 4ed7acdfa..fbdc436a7 100644 --- a/Source/Tests/AOT/AOTGenerator.cpp +++ b/Source/Tests/AOT/AOTGenerator.cpp @@ -147,6 +147,7 @@ void AOTGenSection(FEXCore::Context::Context *CTX, ELFCodeLoader::LoadedSection // All entryproints processed, cleanup this thread CTX->DestroyThread(Thread); + // This thread is now getting abandoned. Disable glibc allocator checking so glibc can safely cleanup its internal allocations. FEXCore::Allocator::YesIKnowImNotSupposedToUseTheGlibcAllocator::HardDisable(); }); diff --git a/Source/Tests/FEXLoader.cpp b/Source/Tests/FEXLoader.cpp index 320875fca..ada704cee 100644 --- a/Source/Tests/FEXLoader.cpp +++ b/Source/Tests/FEXLoader.cpp @@ -121,6 +121,42 @@ namespace FEXServerLogging { } } +namespace AOTIR { + class AOTIRWriterFD final : public FEXCore::Context::AOTIRWriter { + public: + AOTIRWriterFD(const fextl::string &Path) { + // Create and truncate if exists. + constexpr int USER_PERMS = S_IRWXU | S_IRWXG | S_IRWXO; + FD = open(Path.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, USER_PERMS); + } + + operator bool() const { + return FD != -1; + } + + void Write(const void* Data, size_t Size) override { + write(FD, Data, Size); + } + + size_t Offset() override { + return lseek(FD, 0, SEEK_CUR); + } + + void Close() override { + if (FD != -1) { + close(FD); + FD = -1; + } + } + + virtual ~AOTIRWriterFD() { + Close(); + } + private: + int FD{-1}; + }; +} + void InterpreterHandler(fextl::string *Filename, fextl::string const &RootFS, fextl::vector *args) { // Open the Filename to determine if it is a shebang file. int FD = open(Filename->c_str(), O_RDONLY | O_CLOEXEC); @@ -457,43 +493,9 @@ int main(int argc, char **argv, char **const envp) { return open(filepath.c_str(), O_RDONLY); }); - class AOTIRWriterFD final : public FEXCore::Context::AOTIRWriterFD { - public: - AOTIRWriterFD(const fextl::string &Path) { - // Create and truncate if exists. - constexpr int USER_PERMS = S_IRWXU | S_IRWXG | S_IRWXO; - FD = open(Path.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, USER_PERMS); - } - - operator bool() const { - return FD != -1; - } - - void Write(const void* Data, size_t Size) override { - write(FD, Data, Size); - } - - size_t Offset() override { - return lseek(FD, 0, SEEK_CUR); - } - - void Close() override { - if (FD != -1) { - close(FD); - FD = -1; - } - } - - virtual ~AOTIRWriterFD() { - Close(); - } - private: - int FD{-1}; - }; - - CTX->SetAOTIRWriter([](const fextl::string& fileid) -> fextl::unique_ptr { + CTX->SetAOTIRWriter([](const fextl::string& fileid) -> fextl::unique_ptr { const auto filepath = fextl::fmt::format("{}/aotir/{}.aotir.tmp", FEXCore::Config::GetDataDirectory(), fileid); - auto AOTWrite = fextl::make_unique(filepath); + auto AOTWrite = fextl::make_unique(filepath); if (*AOTWrite) { LogMan::Msg::IFmt("AOTIR: Storing {}", fileid); } else { @@ -507,7 +509,9 @@ int main(int argc, char **argv, char **const envp) { const auto NewFilepath = fextl::fmt::format("{}/aotir/{}.aotir", FEXCore::Config::GetDataDirectory(), fileid); // Rename the temporary file to atomically update the file - FHU::Filesystem::RenameFile(TmpFilepath, NewFilepath); + if (!FHU::Filesystem::RenameFile(TmpFilepath, NewFilepath)) { + LogMan::Msg::IFmt("Couldn't rename aotir"); + } }); }