From 86ae42919d86019168c38a878a1dbe38f9b2c207 Mon Sep 17 00:00:00 2001 From: Triang3l Date: Sun, 22 Nov 2020 14:17:37 +0300 Subject: [PATCH] [Memory] Close shared memory FD and properly handle its invalid value --- src/xenia/base/memory.h | 9 +++++++++ src/xenia/base/memory_posix.cc | 14 +++++++------- src/xenia/base/testing/memory_test.cc | 6 +++--- src/xenia/cpu/backend/x64/x64_code_cache.cc | 6 +++--- src/xenia/cpu/backend/x64/x64_code_cache.h | 3 ++- src/xenia/memory.cc | 8 ++++---- src/xenia/memory.h | 3 ++- 7 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/xenia/base/memory.h b/src/xenia/base/memory.h index c0dfba3c6..45471e50f 100644 --- a/src/xenia/base/memory.h +++ b/src/xenia/base/memory.h @@ -18,6 +18,7 @@ #include "xenia/base/assert.h" #include "xenia/base/byte_order.h" +#include "xenia/base/platform.h" namespace xe { namespace memory { @@ -96,7 +97,15 @@ void AlignedFree(T* ptr) { #endif // XE_COMPILER_MSVC } +#if XE_PLATFORM_WIN32 +// HANDLE. typedef void* FileMappingHandle; +constexpr FileMappingHandle kFileMappingHandleInvalid = nullptr; +#else +// File descriptor. +typedef int FileMappingHandle; +constexpr FileMappingHandle kFileMappingHandleInvalid = -1; +#endif FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, size_t length, PageAccess access, diff --git a/src/xenia/base/memory_posix.cc b/src/xenia/base/memory_posix.cc index 975f1eed4..f2834fb9d 100644 --- a/src/xenia/base/memory_posix.cc +++ b/src/xenia/base/memory_posix.cc @@ -86,19 +86,19 @@ FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, assert_always(); return nullptr; } - oflag |= O_CREAT; auto full_path = "/" / path; int ret = shm_open(full_path.c_str(), oflag, 0777); - if (ret > 0) { - ftruncate64(ret, length); + if (ret < 0) { + return kFileMappingHandleInvalid; } - - return ret <= 0 ? nullptr : reinterpret_cast(ret); + ftruncate64(ret, length); + return ret; } void CloseFileMappingHandle(FileMappingHandle handle, const std::filesystem::path& path) { + close(handle); auto full_path = "/" / path; shm_unlink(full_path.c_str()); } @@ -106,8 +106,8 @@ void CloseFileMappingHandle(FileMappingHandle handle, void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, PageAccess access, size_t file_offset) { uint32_t prot = ToPosixProtectFlags(access); - return mmap64(base_address, length, prot, MAP_PRIVATE | MAP_ANONYMOUS, - reinterpret_cast(handle), file_offset); + return mmap64(base_address, length, prot, MAP_PRIVATE | MAP_ANONYMOUS, handle, + file_offset); } bool UnmapFileView(FileMappingHandle handle, void* base_address, diff --git a/src/xenia/base/testing/memory_test.cc b/src/xenia/base/testing/memory_test.cc index b267a4f74..7a19a55a4 100644 --- a/src/xenia/base/testing/memory_test.cc +++ b/src/xenia/base/testing/memory_test.cc @@ -421,7 +421,7 @@ TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") { auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount()); auto memory = xe::memory::CreateFileMappingHandle( path, 0x100, xe::memory::PageAccess::kReadWrite, true); - REQUIRE(memory); + REQUIRE(memory != xe::memory::FileMappingHandleInvalid); xe::memory::CloseFileMappingHandle(memory, path); } @@ -430,7 +430,7 @@ TEST_CASE("map_view", "Virtual Memory Mapping") { const size_t length = 0x100; auto memory = xe::memory::CreateFileMappingHandle( path, length, xe::memory::PageAccess::kReadWrite, true); - REQUIRE(memory); + REQUIRE(memory != xe::memory::FileMappingHandleInvalid); uintptr_t address = 0x100000000; auto view = @@ -447,7 +447,7 @@ TEST_CASE("read_write_view", "Virtual Memory Mapping") { auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount()); auto memory = xe::memory::CreateFileMappingHandle( path, length, xe::memory::PageAccess::kReadWrite, true); - REQUIRE(memory); + REQUIRE(memory != xe::memory::FileMappingHandleInvalid); uintptr_t address = 0x100000000; auto view = diff --git a/src/xenia/cpu/backend/x64/x64_code_cache.cc b/src/xenia/cpu/backend/x64/x64_code_cache.cc index c59786b5f..d3b22d14c 100644 --- a/src/xenia/cpu/backend/x64/x64_code_cache.cc +++ b/src/xenia/cpu/backend/x64/x64_code_cache.cc @@ -40,11 +40,11 @@ X64CodeCache::~X64CodeCache() { } // Unmap all views and close mapping. - if (mapping_) { + if (mapping_ != xe::memory::kFileMappingHandleInvalid) { xe::memory::UnmapFileView(mapping_, generated_code_base_, kGeneratedCodeSize); xe::memory::CloseFileMappingHandle(mapping_, file_name_); - mapping_ = nullptr; + mapping_ = xe::memory::kFileMappingHandleInvalid; } } @@ -67,7 +67,7 @@ bool X64CodeCache::Initialize() { mapping_ = xe::memory::CreateFileMappingHandle( file_name_, kGeneratedCodeSize, xe::memory::PageAccess::kExecuteReadWrite, false); - if (!mapping_) { + if (mapping_ == xe::memory::kFileMappingHandleInvalid) { XELOGE("Unable to create code cache mmap"); return false; } diff --git a/src/xenia/cpu/backend/x64/x64_code_cache.h b/src/xenia/cpu/backend/x64/x64_code_cache.h index 1f1473094..9f7424ec8 100644 --- a/src/xenia/cpu/backend/x64/x64_code_cache.h +++ b/src/xenia/cpu/backend/x64/x64_code_cache.h @@ -100,7 +100,8 @@ class X64CodeCache : public CodeCache { UnwindReservation unwind_reservation) {} std::filesystem::path file_name_; - xe::memory::FileMappingHandle mapping_ = nullptr; + xe::memory::FileMappingHandle mapping_ = + xe::memory::kFileMappingHandleInvalid; // NOTE: the global critical region must be held when manipulating the offsets // or counts of anything, to keep the tables consistent and ordered. diff --git a/src/xenia/memory.cc b/src/xenia/memory.cc index 82991a158..f70e00f66 100644 --- a/src/xenia/memory.cc +++ b/src/xenia/memory.cc @@ -113,11 +113,11 @@ Memory::~Memory() { heaps_.physical.Dispose(); // Unmap all views and close mapping. - if (mapping_) { + if (mapping_ != xe::memory::kFileMappingHandleInvalid) { UnmapViews(); xe::memory::CloseFileMappingHandle(mapping_, file_name_); mapping_base_ = nullptr; - mapping_ = nullptr; + mapping_ = xe::memory::kFileMappingHandleInvalid; } virtual_membase_ = nullptr; @@ -133,9 +133,9 @@ bool Memory::Initialize() { file_name_, // entire 4gb space + 512mb physical: 0x11FFFFFFF, xe::memory::PageAccess::kReadWrite, false); - if (!mapping_) { + if (mapping_ == xe::memory::kFileMappingHandleInvalid) { XELOGE("Unable to reserve the 4gb guest address space."); - assert_not_null(mapping_); + assert_always(); return false; } diff --git a/src/xenia/memory.h b/src/xenia/memory.h index fa0c62f4c..df76c39bc 100644 --- a/src/xenia/memory.h +++ b/src/xenia/memory.h @@ -491,7 +491,8 @@ class Memory { uint8_t* virtual_membase_ = nullptr; uint8_t* physical_membase_ = nullptr; - xe::memory::FileMappingHandle mapping_ = nullptr; + xe::memory::FileMappingHandle mapping_ = + xe::memory::kFileMappingHandleInvalid; uint8_t* mapping_base_ = nullptr; union { struct {