From aa332dcc8ed67e3e258036cc3daaaae3268bb902 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Sat, 21 Nov 2020 22:18:49 -0500 Subject: [PATCH 1/8] threading_posix: Increase stack size in test 16 KB is not enough for the linux thread to be spawned so bump up to 16MB --- src/xenia/base/testing/threading_test.cc | 2 +- src/xenia/base/threading_posix.cc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/xenia/base/testing/threading_test.cc b/src/xenia/base/testing/threading_test.cc index f8fae6339..ba6eda0db 100644 --- a/src/xenia/base/testing/threading_test.cc +++ b/src/xenia/base/testing/threading_test.cc @@ -813,7 +813,7 @@ TEST_CASE("Create and Run Thread", "Thread") { result = Wait(Thread::GetCurrentThread(), false, 50ms); REQUIRE(result == WaitResult::kTimeout); - params.stack_size = 16 * 1024; + params.stack_size = 16 * 1024 * 1024; thread = Thread::Create(params, [] { while (true) { Thread::Exit(-1); diff --git a/src/xenia/base/threading_posix.cc b/src/xenia/base/threading_posix.cc index 9e39b17a5..f6a6db193 100644 --- a/src/xenia/base/threading_posix.cc +++ b/src/xenia/base/threading_posix.cc @@ -505,6 +505,7 @@ class PosixCondition : public PosixConditionBase { } } if (pthread_create(&thread_, &attr, ThreadStartRoutine, start_data) != 0) { + pthread_attr_destroy(&attr); return false; } pthread_attr_destroy(&attr); From 5fa59fc4a9688f6676738966f9b67b928021a5c6 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Sat, 21 Nov 2020 22:36:20 -0500 Subject: [PATCH 2/8] threading_posix: don't delete thread_local thread object Disabling on exit thread delete as it causes an assert fail. There isn't a leak here because current_thread_ is a thread_local static. --- src/xenia/base/threading_posix.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/xenia/base/threading_posix.cc b/src/xenia/base/threading_posix.cc index f6a6db193..7194855f2 100644 --- a/src/xenia/base/threading_posix.cc +++ b/src/xenia/base/threading_posix.cc @@ -1007,7 +1007,10 @@ Thread* Thread::GetCurrentThread() { pthread_t handle = pthread_self(); current_thread_ = new PosixThread(handle); - atexit([] { delete current_thread_; }); + // TODO(bwrsandman): Disabling deleting thread_local current thread to prevent + // assert in destructor. Since this is thread local, the + // "memory leaking" is controlled. + // atexit([] { delete current_thread_; }); return current_thread_; } From 962b90f699f2eb195abf566bdbd93a384f3cec36 Mon Sep 17 00:00:00 2001 From: uytvbn Date: Tue, 24 Oct 2017 23:12:33 +0200 Subject: [PATCH 3/8] [Linux] Implement virtual memory allocation --- src/xenia/base/memory_posix.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/xenia/base/memory_posix.cc b/src/xenia/base/memory_posix.cc index 8010e5f18..8da6d9a1b 100644 --- a/src/xenia/base/memory_posix.cc +++ b/src/xenia/base/memory_posix.cc @@ -40,7 +40,13 @@ void* AllocFixed(void* base_address, size_t length, AllocationType allocation_type, PageAccess access) { // mmap does not support reserve / commit, so ignore allocation_type. uint32_t prot = ToPosixProtectFlags(access); - return mmap(base_address, length, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + void* result = mmap(base_address, length, prot, + MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS, -1, 0); + if (result == MAP_FAILED) { + return nullptr; + } else { + return result; + } } bool DeallocFixed(void* base_address, size_t length, @@ -91,7 +97,7 @@ FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, } void CloseFileMappingHandle(FileMappingHandle handle) { - close((intptr_t)handle); + close(static_cast(reinterpret_cast(handle))); } void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, From 22ef265057be6428baaafd8d40d7ff35b9125524 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 11 Jul 2019 10:26:59 -0400 Subject: [PATCH 4/8] [memory] Add Memory mapping view tests Add test for mapping and for mapping with access. --- src/xenia/base/testing/memory_test.cc | 55 +++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/xenia/base/testing/memory_test.cc b/src/xenia/base/testing/memory_test.cc index 5433dc118..66a365c8f 100644 --- a/src/xenia/base/testing/memory_test.cc +++ b/src/xenia/base/testing/memory_test.cc @@ -10,6 +10,9 @@ #include "xenia/base/memory.h" #include "third_party/catch/include/catch.hpp" +#include "third_party/fmt/include/fmt/format.h" + +#include "xenia/base/clock.h" namespace xe { namespace base { @@ -414,6 +417,58 @@ TEST_CASE("copy_and_swap_16_in_32_unaligned", "Copy and Swap") { REQUIRE(true == true); } +TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") { + auto path = fmt::format("Local\\xenia_test_{}", Clock::QueryHostTickCount()); + auto memory = xe::memory::CreateFileMappingHandle( + path, 0x100, xe::memory::PageAccess::kReadWrite, true); + REQUIRE(memory); + xe::memory::CloseFileMappingHandle(memory); +} + +TEST_CASE("map_view", "Virtual Memory Mapping") { + auto path = fmt::format("Local\\xenia_test_{}", Clock::QueryHostTickCount()); + const size_t length = 0x100; + auto memory = xe::memory::CreateFileMappingHandle( + path, length, xe::memory::PageAccess::kReadWrite, true); + REQUIRE(memory); + + uintptr_t address = 0x100000000; + auto view = + xe::memory::MapFileView(memory, reinterpret_cast(address), length, + xe::memory::PageAccess::kReadWrite, 0); + REQUIRE(reinterpret_cast(view) == address); + + xe::memory::UnmapFileView(memory, reinterpret_cast(address), length); + xe::memory::CloseFileMappingHandle(memory); +} + +TEST_CASE("read_write_view", "Virtual Memory Mapping") { + const size_t length = 0x100; + auto path = fmt::format("Local\\xenia_test_{}", Clock::QueryHostTickCount()); + auto memory = xe::memory::CreateFileMappingHandle( + path, length, xe::memory::PageAccess::kReadWrite, true); + REQUIRE(memory); + + uintptr_t address = 0x100000000; + auto view = + xe::memory::MapFileView(memory, reinterpret_cast(address), length, + xe::memory::PageAccess::kReadWrite, 0); + REQUIRE(reinterpret_cast(view) == address); + + for (uint32_t i = 0; i < length; i += sizeof(uint8_t)) { + auto p_value = reinterpret_cast(address + i); + *p_value = i; + } + for (uint32_t i = 0; i < length; i += sizeof(uint8_t)) { + auto p_value = reinterpret_cast(address + i); + uint8_t value = *p_value; + REQUIRE(value == i); + } + + xe::memory::UnmapFileView(memory, reinterpret_cast(address), length); + xe::memory::CloseFileMappingHandle(memory); +} + } // namespace test } // namespace base } // namespace xe From 2c7009ca803bf6d112a8c1b6ca39d53dac032459 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Wed, 10 Jul 2019 22:31:16 -0400 Subject: [PATCH 5/8] [memory] Move "Local\\" prefix to win impl CreateFileMappingHandle now takes shared memory name without a prefix. The doc of shm_open recommends not using slashes and prefixing with "/". The prefixing has been moved to the os implementation layer. Invocations of CreateFileMappingHandle were all using "Local\\" so these prefixes were removed. --- src/xenia/base/memory_posix.cc | 3 ++- src/xenia/base/memory_win.cc | 3 ++- src/xenia/base/testing/memory_test.cc | 6 +++--- src/xenia/cpu/backend/x64/x64_code_cache.cc | 3 +-- src/xenia/memory.cc | 3 +-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/xenia/base/memory_posix.cc b/src/xenia/base/memory_posix.cc index 8da6d9a1b..81020d094 100644 --- a/src/xenia/base/memory_posix.cc +++ b/src/xenia/base/memory_posix.cc @@ -88,7 +88,8 @@ FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, } oflag |= O_CREAT; - int ret = shm_open(path.c_str(), oflag, 0777); + auto full_path = "/" / path; + int ret = shm_open(full_path.c_str(), oflag, 0777); if (ret > 0) { ftruncate64(ret, length); } diff --git a/src/xenia/base/memory_win.cc b/src/xenia/base/memory_win.cc index 13b78a411..ed39e1028 100644 --- a/src/xenia/base/memory_win.cc +++ b/src/xenia/base/memory_win.cc @@ -147,9 +147,10 @@ FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, bool commit) { DWORD protect = ToWin32ProtectFlags(access) | (commit ? SEC_COMMIT : SEC_RESERVE); + auto full_path = "Local" / path; return CreateFileMappingW(INVALID_HANDLE_VALUE, NULL, protect, static_cast(length >> 32), - static_cast(length), path.c_str()); + static_cast(length), full_path.c_str()); } void CloseFileMappingHandle(FileMappingHandle handle) { CloseHandle(handle); } diff --git a/src/xenia/base/testing/memory_test.cc b/src/xenia/base/testing/memory_test.cc index 66a365c8f..1fe2184fa 100644 --- a/src/xenia/base/testing/memory_test.cc +++ b/src/xenia/base/testing/memory_test.cc @@ -418,7 +418,7 @@ TEST_CASE("copy_and_swap_16_in_32_unaligned", "Copy and Swap") { } TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") { - auto path = fmt::format("Local\\xenia_test_{}", Clock::QueryHostTickCount()); + auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount()); auto memory = xe::memory::CreateFileMappingHandle( path, 0x100, xe::memory::PageAccess::kReadWrite, true); REQUIRE(memory); @@ -426,7 +426,7 @@ TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") { } TEST_CASE("map_view", "Virtual Memory Mapping") { - auto path = fmt::format("Local\\xenia_test_{}", Clock::QueryHostTickCount()); + auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount()); const size_t length = 0x100; auto memory = xe::memory::CreateFileMappingHandle( path, length, xe::memory::PageAccess::kReadWrite, true); @@ -444,7 +444,7 @@ TEST_CASE("map_view", "Virtual Memory Mapping") { TEST_CASE("read_write_view", "Virtual Memory Mapping") { const size_t length = 0x100; - auto path = fmt::format("Local\\xenia_test_{}", Clock::QueryHostTickCount()); + auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount()); auto memory = xe::memory::CreateFileMappingHandle( path, length, xe::memory::PageAccess::kReadWrite, true); REQUIRE(memory); diff --git a/src/xenia/cpu/backend/x64/x64_code_cache.cc b/src/xenia/cpu/backend/x64/x64_code_cache.cc index 8f9b433dd..6ea280908 100644 --- a/src/xenia/cpu/backend/x64/x64_code_cache.cc +++ b/src/xenia/cpu/backend/x64/x64_code_cache.cc @@ -63,8 +63,7 @@ bool X64CodeCache::Initialize() { } // Create mmap file. This allows us to share the code cache with the debugger. - file_name_ = - fmt::format("Local\\xenia_code_cache_{}", Clock::QueryHostTickCount()); + file_name_ = fmt::format("xenia_code_cache_{}", Clock::QueryHostTickCount()); mapping_ = xe::memory::CreateFileMappingHandle( file_name_, kGeneratedCodeSize, xe::memory::PageAccess::kExecuteReadWrite, false); diff --git a/src/xenia/memory.cc b/src/xenia/memory.cc index 7e60797f5..8f20a79af 100644 --- a/src/xenia/memory.cc +++ b/src/xenia/memory.cc @@ -125,8 +125,7 @@ Memory::~Memory() { } bool Memory::Initialize() { - file_name_ = - fmt::format("Local\\xenia_memory_{}", Clock::QueryHostTickCount()); + file_name_ = fmt::format("xenia_memory_{}", Clock::QueryHostTickCount()); // Create main page file-backed mapping. This is all reserved but // uncommitted (so it shouldn't expand page file). From 49e194009bbaaeb6cda5cc7db7fd415fbfead275 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 11 Jul 2019 09:44:42 -0400 Subject: [PATCH 6/8] [memory linux] Properly unlink shared memory shm_unlink(name) is the proper way to close a shared memory in linux. Prior to this, xenia was creating and not cleaning up shared memory handle which would accumulate in /dev/shm. shm_unlink is the proper way of doing this. Add filename to CloseFileMappingHandle signature. Add simple test to open and close. --- src/xenia/base/memory.h | 3 ++- src/xenia/base/memory_posix.cc | 6 ++++-- src/xenia/base/memory_win.cc | 5 ++++- src/xenia/base/testing/memory_test.cc | 6 +++--- src/xenia/cpu/backend/x64/x64_code_cache.cc | 2 +- src/xenia/memory.cc | 2 +- 6 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/xenia/base/memory.h b/src/xenia/base/memory.h index e1125a5e3..c0dfba3c6 100644 --- a/src/xenia/base/memory.h +++ b/src/xenia/base/memory.h @@ -101,7 +101,8 @@ typedef void* FileMappingHandle; FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, size_t length, PageAccess access, bool commit); -void CloseFileMappingHandle(FileMappingHandle handle); +void CloseFileMappingHandle(FileMappingHandle handle, + const std::filesystem::path& path); void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, PageAccess access, size_t file_offset); bool UnmapFileView(FileMappingHandle handle, void* base_address, size_t length); diff --git a/src/xenia/base/memory_posix.cc b/src/xenia/base/memory_posix.cc index 81020d094..975f1eed4 100644 --- a/src/xenia/base/memory_posix.cc +++ b/src/xenia/base/memory_posix.cc @@ -97,8 +97,10 @@ FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, return ret <= 0 ? nullptr : reinterpret_cast(ret); } -void CloseFileMappingHandle(FileMappingHandle handle) { - close(static_cast(reinterpret_cast(handle))); +void CloseFileMappingHandle(FileMappingHandle handle, + const std::filesystem::path& path) { + auto full_path = "/" / path; + shm_unlink(full_path.c_str()); } void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, diff --git a/src/xenia/base/memory_win.cc b/src/xenia/base/memory_win.cc index ed39e1028..343285d94 100644 --- a/src/xenia/base/memory_win.cc +++ b/src/xenia/base/memory_win.cc @@ -153,7 +153,10 @@ FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, static_cast(length), full_path.c_str()); } -void CloseFileMappingHandle(FileMappingHandle handle) { CloseHandle(handle); } +void CloseFileMappingHandle(FileMappingHandle handle, + const std::filesystem::path& path) { + CloseHandle(handle); +} void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, PageAccess access, size_t file_offset) { diff --git a/src/xenia/base/testing/memory_test.cc b/src/xenia/base/testing/memory_test.cc index 1fe2184fa..b267a4f74 100644 --- a/src/xenia/base/testing/memory_test.cc +++ b/src/xenia/base/testing/memory_test.cc @@ -422,7 +422,7 @@ TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") { auto memory = xe::memory::CreateFileMappingHandle( path, 0x100, xe::memory::PageAccess::kReadWrite, true); REQUIRE(memory); - xe::memory::CloseFileMappingHandle(memory); + xe::memory::CloseFileMappingHandle(memory, path); } TEST_CASE("map_view", "Virtual Memory Mapping") { @@ -439,7 +439,7 @@ TEST_CASE("map_view", "Virtual Memory Mapping") { REQUIRE(reinterpret_cast(view) == address); xe::memory::UnmapFileView(memory, reinterpret_cast(address), length); - xe::memory::CloseFileMappingHandle(memory); + xe::memory::CloseFileMappingHandle(memory, path); } TEST_CASE("read_write_view", "Virtual Memory Mapping") { @@ -466,7 +466,7 @@ TEST_CASE("read_write_view", "Virtual Memory Mapping") { } xe::memory::UnmapFileView(memory, reinterpret_cast(address), length); - xe::memory::CloseFileMappingHandle(memory); + xe::memory::CloseFileMappingHandle(memory, path); } } // namespace test diff --git a/src/xenia/cpu/backend/x64/x64_code_cache.cc b/src/xenia/cpu/backend/x64/x64_code_cache.cc index 6ea280908..c59786b5f 100644 --- a/src/xenia/cpu/backend/x64/x64_code_cache.cc +++ b/src/xenia/cpu/backend/x64/x64_code_cache.cc @@ -43,7 +43,7 @@ X64CodeCache::~X64CodeCache() { if (mapping_) { xe::memory::UnmapFileView(mapping_, generated_code_base_, kGeneratedCodeSize); - xe::memory::CloseFileMappingHandle(mapping_); + xe::memory::CloseFileMappingHandle(mapping_, file_name_); mapping_ = nullptr; } } diff --git a/src/xenia/memory.cc b/src/xenia/memory.cc index 8f20a79af..82991a158 100644 --- a/src/xenia/memory.cc +++ b/src/xenia/memory.cc @@ -115,7 +115,7 @@ Memory::~Memory() { // Unmap all views and close mapping. if (mapping_) { UnmapViews(); - xe::memory::CloseFileMappingHandle(mapping_); + xe::memory::CloseFileMappingHandle(mapping_, file_name_); mapping_base_ = nullptr; mapping_ = nullptr; } From 432369ae8476a0ad7dee187be86bc1412afb6a4c Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 11 Jul 2019 22:00:07 -0400 Subject: [PATCH 7/8] [kernel] Define param order between compilers Fix issue in clang where args were inverted last to first due to the way c++ implements function calls. The function make_tuple, being a function, has undefined ordering of resolution of the values of its parameters `Ps(init)` and would vary between compilers. Namely clang would resolve them in order and msvc would resolve them in reverse order. This normally would not matter except for the fact that init maintains a mutable state which is affected my the order of operations: init.ordinal is a counter and also defines where in memory a value is stored. The C++ standard doesn't define an order of resolution of parameters in a function but it will define an order in a brace-initializer. Switching make_tuple for a brace-initializer enforces an order which is the same between all compilers (tested gcc, clang and msvc). Prior code was written to decrement ordinal due to the reverse traversal. This has been switched to incrementing thanks to the in-order traversal. --- src/xenia/kernel/util/shim_utils.h | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/xenia/kernel/util/shim_utils.h b/src/xenia/kernel/util/shim_utils.h index fcfc90d4d..126358b17 100644 --- a/src/xenia/kernel/util/shim_utils.h +++ b/src/xenia/kernel/util/shim_utils.h @@ -148,7 +148,7 @@ class Param { protected: Param() : ordinal_(-1) {} - explicit Param(Init& init) : ordinal_(--init.ordinal) {} + explicit Param(Init& init) : ordinal_(init.ordinal++) {} template void LoadValue(Init& init, V* out_value) { @@ -519,10 +519,13 @@ xe::cpu::Export* RegisterExport(R (*fn)(Ps&...), const char* name, ++export_entry->function_data.call_count; Param::Init init = { ppc_context, - sizeof...(Ps), 0, }; - auto params = std::make_tuple(Ps(init)...); + // Using braces initializer instead of make_tuple because braces + // enforce execution order across compilers. + // The make_tuple order is undefined per the C++ standard and + // cause inconsitencies between msvc and clang. + std::tuple params = {Ps(init)...}; if (export_entry->tags & xe::cpu::ExportTag::kLog && (!(export_entry->tags & xe::cpu::ExportTag::kHighFrequency) || cvars::log_high_frequency_kernel_calls)) { @@ -554,9 +557,13 @@ xe::cpu::Export* RegisterExport(void (*fn)(Ps&...), const char* name, ++export_entry->function_data.call_count; Param::Init init = { ppc_context, - sizeof...(Ps), + 0, }; - auto params = std::make_tuple(Ps(init)...); + // Using braces initializer instead of make_tuple because braces + // enforce execution order across compilers. + // The make_tuple order is undefined per the C++ standard and + // cause inconsitencies between msvc and clang. + std::tuple params = {Ps(init)...}; if (export_entry->tags & xe::cpu::ExportTag::kLog && (!(export_entry->tags & xe::cpu::ExportTag::kHighFrequency) || cvars::log_high_frequency_kernel_calls)) { From 86ae42919d86019168c38a878a1dbe38f9b2c207 Mon Sep 17 00:00:00 2001 From: Triang3l Date: Sun, 22 Nov 2020 14:17:37 +0300 Subject: [PATCH 8/8] [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 {