From f4722327059b3a618ea63539a31c4f5a16e14bd4 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Nov 2018 08:29:25 -0500 Subject: [PATCH 1/5] kernel/shared_memory: Make data members private Rather than allow unfettered access to the class internals, we hide all members by default and create and API that other code can operate against. --- src/core/hle/kernel/shared_memory.h | 39 ++++++++++++++++------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h index 2c06bb7ce..9a7c189e8 100644 --- a/src/core/hle/kernel/shared_memory.h +++ b/src/core/hle/kernel/shared_memory.h @@ -81,6 +81,11 @@ public: return HANDLE_TYPE; } + /// Gets the size of the underlying memory block in bytes. + u64 GetSize() const { + return size; + } + /** * Converts the specified MemoryPermission into the equivalent VMAPermission. * @param permission The MemoryPermission to convert. @@ -112,26 +117,26 @@ public: */ u8* GetPointer(u32 offset = 0); - /// Process that created this shared memory block. - SharedPtr owner_process; - /// Address of shared memory block in the owner process if specified. - VAddr base_address; - /// Backing memory for this shared memory block. - std::shared_ptr> backing_block; - /// Offset into the backing block for this shared memory. - std::size_t backing_block_offset; - /// Size of the memory block. Page-aligned. - u64 size; - /// Permission restrictions applied to the process which created the block. - MemoryPermission permissions; - /// Permission restrictions applied to other processes mapping the block. - MemoryPermission other_permissions; - /// Name of shared memory object. - std::string name; - private: explicit SharedMemory(KernelCore& kernel); ~SharedMemory() override; + + /// Backing memory for this shared memory block. + std::shared_ptr> backing_block; + /// Offset into the backing block for this shared memory. + std::size_t backing_block_offset = 0; + /// Size of the memory block. Page-aligned. + u64 size = 0; + /// Permission restrictions applied to the process which created the block. + MemoryPermission permissions{}; + /// Permission restrictions applied to other processes mapping the block. + MemoryPermission other_permissions{}; + /// Process that created this shared memory block. + SharedPtr owner_process; + /// Address of shared memory block in the owner process if specified. + VAddr base_address = 0; + /// Name of shared memory object. + std::string name; }; } // namespace Kernel From 76ac234bf66a5cad7898056dcd33f52d2c38a232 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Nov 2018 08:50:28 -0500 Subject: [PATCH 2/5] kernel/shared_memory: Make GetPointer() take a std::size_t instead of a u32 Makes the interface nicer to use in terms of 64-bit code, as it makes it less likely for one to get truncation warnings (and also makes sense in the context of the rest of the interface where 64-bit types are used for sizes and offsets --- src/core/hle/kernel/shared_memory.cpp | 2 +- src/core/hle/kernel/shared_memory.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp index a016a86b6..214f0c9bf 100644 --- a/src/core/hle/kernel/shared_memory.cpp +++ b/src/core/hle/kernel/shared_memory.cpp @@ -132,7 +132,7 @@ VMAPermission SharedMemory::ConvertPermissions(MemoryPermission permission) { return static_cast(masked_permissions); } -u8* SharedMemory::GetPointer(u32 offset) { +u8* SharedMemory::GetPointer(std::size_t offset) { return backing_block->data() + backing_block_offset + offset; } diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h index 9a7c189e8..7d7d6486d 100644 --- a/src/core/hle/kernel/shared_memory.h +++ b/src/core/hle/kernel/shared_memory.h @@ -115,7 +115,7 @@ public: * @param offset Offset from the start of the shared memory block to get pointer * @return Pointer to the shared memory block from the specified offset */ - u8* GetPointer(u32 offset = 0); + u8* GetPointer(std::size_t offset = 0); private: explicit SharedMemory(KernelCore& kernel); From 2d37ca372615e8e3e2a1feca07c1077dc4a001dd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Nov 2018 08:56:15 -0500 Subject: [PATCH 3/5] kernel/shared_memory: Use 64-bit types for offset and size in CreateForApplet Keeps the interface consistent with the regular Create() function. --- src/core/hle/kernel/shared_memory.cpp | 2 +- src/core/hle/kernel/shared_memory.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp index 214f0c9bf..68ad39fa1 100644 --- a/src/core/hle/kernel/shared_memory.cpp +++ b/src/core/hle/kernel/shared_memory.cpp @@ -61,7 +61,7 @@ SharedPtr SharedMemory::Create(KernelCore& kernel, SharedPtr SharedMemory::CreateForApplet( - KernelCore& kernel, std::shared_ptr> heap_block, u32 offset, u32 size, + KernelCore& kernel, std::shared_ptr> heap_block, std::size_t offset, u64 size, MemoryPermission permissions, MemoryPermission other_permissions, std::string name) { SharedPtr shared_memory(new SharedMemory(kernel)); diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h index 7d7d6486d..60433f09b 100644 --- a/src/core/hle/kernel/shared_memory.h +++ b/src/core/hle/kernel/shared_memory.h @@ -64,7 +64,7 @@ public: */ static SharedPtr CreateForApplet(KernelCore& kernel, std::shared_ptr> heap_block, - u32 offset, u32 size, + std::size_t offset, u64 size, MemoryPermission permissions, MemoryPermission other_permissions, std::string name = "Unknown Applet"); From fb5d4b17de035e3682e64513ae7612f40afd01bd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Nov 2018 09:00:32 -0500 Subject: [PATCH 4/5] kernel/shared_memory: Add a const qualified member function overload for GetPointer() Given this doesn't mutate instance state, we can provide a const-qualified variant as well. --- src/core/hle/kernel/shared_memory.cpp | 4 ++++ src/core/hle/kernel/shared_memory.h | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp index 68ad39fa1..b8bcc16ee 100644 --- a/src/core/hle/kernel/shared_memory.cpp +++ b/src/core/hle/kernel/shared_memory.cpp @@ -136,4 +136,8 @@ u8* SharedMemory::GetPointer(std::size_t offset) { return backing_block->data() + backing_block_offset + offset; } +const u8* SharedMemory::GetPointer(std::size_t offset) const { + return backing_block->data() + backing_block_offset + offset; +} + } // namespace Kernel diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h index 60433f09b..f3b05e48b 100644 --- a/src/core/hle/kernel/shared_memory.h +++ b/src/core/hle/kernel/shared_memory.h @@ -113,10 +113,17 @@ public: /** * Gets a pointer to the shared memory block * @param offset Offset from the start of the shared memory block to get pointer - * @return Pointer to the shared memory block from the specified offset + * @return A pointer to the shared memory block from the specified offset */ u8* GetPointer(std::size_t offset = 0); + /** + * Gets a constant pointer to the shared memory block + * @param offset Offset from the start of the shared memory block to get pointer + * @return A constant pointer to the shared memory block from the specified offset + */ + const u8* GetPointer(std::size_t offset = 0) const; + private: explicit SharedMemory(KernelCore& kernel); ~SharedMemory() override; From 233e495c1411a813460ce71efb7be69ff73649ee Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Nov 2018 09:05:04 -0500 Subject: [PATCH 5/5] kernel/shared_memory: Make Map() and Unmap() take the target process by reference rather than as a pointer Both member functions assume the passed in target process will not be null. Instead of making this assumption implicit, we can change the functions to be references and enforce this at the type-system level. --- src/core/hle/kernel/shared_memory.cpp | 14 +++++++------- src/core/hle/kernel/shared_memory.h | 6 +++--- src/core/hle/kernel/svc.cpp | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp index b8bcc16ee..0494581f5 100644 --- a/src/core/hle/kernel/shared_memory.cpp +++ b/src/core/hle/kernel/shared_memory.cpp @@ -78,10 +78,10 @@ SharedPtr SharedMemory::CreateForApplet( return shared_memory; } -ResultCode SharedMemory::Map(Process* target_process, VAddr address, MemoryPermission permissions, +ResultCode SharedMemory::Map(Process& target_process, VAddr address, MemoryPermission permissions, MemoryPermission other_permissions) { const MemoryPermission own_other_permissions = - target_process == owner_process ? this->permissions : this->other_permissions; + &target_process == owner_process ? this->permissions : this->other_permissions; // Automatically allocated memory blocks can only be mapped with other_permissions = DontCare if (base_address == 0 && other_permissions != MemoryPermission::DontCare) { @@ -106,7 +106,7 @@ ResultCode SharedMemory::Map(Process* target_process, VAddr address, MemoryPermi VAddr target_address = address; // Map the memory block into the target process - auto result = target_process->VMManager().MapMemoryBlock( + auto result = target_process.VMManager().MapMemoryBlock( target_address, backing_block, backing_block_offset, size, MemoryState::Shared); if (result.Failed()) { LOG_ERROR( @@ -116,14 +116,14 @@ ResultCode SharedMemory::Map(Process* target_process, VAddr address, MemoryPermi return result.Code(); } - return target_process->VMManager().ReprotectRange(target_address, size, - ConvertPermissions(permissions)); + return target_process.VMManager().ReprotectRange(target_address, size, + ConvertPermissions(permissions)); } -ResultCode SharedMemory::Unmap(Process* target_process, VAddr address) { +ResultCode SharedMemory::Unmap(Process& target_process, VAddr address) { // TODO(Subv): Verify what happens if the application tries to unmap an address that is not // mapped to a SharedMemory. - return target_process->VMManager().UnmapRange(address, size); + return target_process.VMManager().UnmapRange(address, size); } VMAPermission SharedMemory::ConvertPermissions(MemoryPermission permission) { diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h index f3b05e48b..0b48db699 100644 --- a/src/core/hle/kernel/shared_memory.h +++ b/src/core/hle/kernel/shared_memory.h @@ -99,16 +99,16 @@ public: * @param permissions Memory block map permissions (specified by SVC field) * @param other_permissions Memory block map other permissions (specified by SVC field) */ - ResultCode Map(Process* target_process, VAddr address, MemoryPermission permissions, + ResultCode Map(Process& target_process, VAddr address, MemoryPermission permissions, MemoryPermission other_permissions); /** * Unmaps a shared memory block from the specified address in system memory - * @param target_process Process from which to umap the memory block. + * @param target_process Process from which to unmap the memory block. * @param address Address in system memory where the shared memory block is mapped * @return Result code of the unmap operation */ - ResultCode Unmap(Process* target_process, VAddr address); + ResultCode Unmap(Process& target_process, VAddr address); /** * Gets a pointer to the shared memory block diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 75dbfc31d..487e5f0fb 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -796,7 +796,7 @@ static ResultCode MapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 s return ERR_INVALID_MEMORY_RANGE; } - return shared_memory->Map(current_process, addr, permissions_type, MemoryPermission::DontCare); + return shared_memory->Map(*current_process, addr, permissions_type, MemoryPermission::DontCare); } static ResultCode UnmapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 size) { @@ -826,7 +826,7 @@ static ResultCode UnmapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 return ERR_INVALID_MEMORY_RANGE; } - return shared_memory->Unmap(current_process, addr); + return shared_memory->Unmap(*current_process, addr); } /// Query process memory