kernel/server_session: Make data members private

Makes it much nicer to locally reason about server session behavior, as
part of its functionality isn't placed around other classes.

src/core/hle/kernel/client_session.cpp

@@ -17,21 +17,11 @@ ClientSession::~ClientSession() {
     // This destructor will be called automatically when the last ClientSession handle is closed by
     // the emulated application.
 
-    // Local references to ServerSession and SessionRequestHandler are necessary to guarantee they
+    // A local reference to the ServerSession is necessary to guarantee it
     // will be kept alive until after ClientDisconnected() returns.
     SharedPtr<ServerSession> server = parent->server;
     if (server) {
-        std::shared_ptr<SessionRequestHandler> hle_handler = server->hle_handler;
-        if (hle_handler)
-            hle_handler->ClientDisconnected(server);
-
-        // TODO(Subv): Force a wake up of all the ServerSession's waiting threads and set
-        // their WaitSynchronization result to 0xC920181A.
-
-        // Clean up the list of client threads with pending requests, they are unneeded now that the
-        // client endpoint is closed.
-        server->pending_requesting_threads.clear();
-        server->currently_handling = nullptr;
+        server->ClientDisconnected();
     }
 
     parent->client = nullptr;

src/core/hle/kernel/client_session.h

@@ -36,14 +36,15 @@ public:
 
     ResultCode SendSyncRequest(SharedPtr<Thread> thread);
 
-    std::string name; ///< Name of client port (optional)
+private:
+    explicit ClientSession(KernelCore& kernel);
+    ~ClientSession() override;
 
     /// The parent session, which links to the server endpoint.
     std::shared_ptr<Session> parent;
 
-private:
-    explicit ClientSession(KernelCore& kernel);
-    ~ClientSession() override;
+    /// Name of the client session (optional)
+    std::string name;
 };
 
 } // namespace Kernel

src/core/hle/kernel/hle_ipc.cpp

@@ -264,11 +264,11 @@ ResultCode HLERequestContext::WriteToOutgoingCommandBuffer(Thread& thread) {
         // Write the domain objects to the command buffer, these go after the raw untranslated data.
         // TODO(Subv): This completely ignores C buffers.
         std::size_t domain_offset = size - domain_message_header->num_objects;
-        auto& request_handlers = server_session->domain_request_handlers;
 
-        for (auto& object : domain_objects) {
-            request_handlers.emplace_back(object);
-            dst_cmdbuf[domain_offset++] = static_cast<u32_le>(request_handlers.size());
+        for (const auto& object : domain_objects) {
+            server_session->AppendDomainRequestHandler(object);
+            dst_cmdbuf[domain_offset++] =
+                static_cast<u32_le>(server_session->NumDomainRequestHandlers());
         }
     }
 

src/core/hle/kernel/server_session.cpp

@@ -63,6 +63,34 @@ void ServerSession::Acquire(Thread* thread) {
     pending_requesting_threads.pop_back();
 }
 
+void ServerSession::ClientDisconnected() {
+    // We keep a shared pointer to the hle handler to keep it alive throughout
+    // the call to ClientDisconnected, as ClientDisconnected invalidates the
+    // hle_handler member itself during the course of the function executing.
+    std::shared_ptr<SessionRequestHandler> handler = hle_handler;
+    if (handler) {
+        // Note that after this returns, this server session's hle_handler is
+        // invalidated (set to null).
+        handler->ClientDisconnected(this);
+    }
+
+    // TODO(Subv): Force a wake up of all the ServerSession's waiting threads and set
+    // their WaitSynchronization result to 0xC920181A.
+
+    // Clean up the list of client threads with pending requests, they are unneeded now that the
+    // client endpoint is closed.
+    pending_requesting_threads.clear();
+    currently_handling = nullptr;
+}
+
+void ServerSession::AppendDomainRequestHandler(std::shared_ptr<SessionRequestHandler> handler) {
+    domain_request_handlers.push_back(std::move(handler));
+}
+
+std::size_t ServerSession::NumDomainRequestHandlers() const {
+    return domain_request_handlers.size();
+}
+
 ResultCode ServerSession::HandleDomainSyncRequest(Kernel::HLERequestContext& context) {
     auto* const domain_message_header = context.GetDomainMessageHeader();
     if (domain_message_header) {

src/core/hle/kernel/server_session.h

@@ -46,6 +46,14 @@ public:
         return HANDLE_TYPE;
     }
 
+    Session* GetParent() {
+        return parent.get();
+    }
+
+    const Session* GetParent() const {
+        return parent.get();
+    }
+
     using SessionPair = std::tuple<SharedPtr<ServerSession>, SharedPtr<ClientSession>>;
 
     /**
@@ -78,23 +86,16 @@ public:
 
     void Acquire(Thread* thread) override;
 
-    std::string name;                ///< The name of this session (optional)
-    std::shared_ptr<Session> parent; ///< The parent session, which links to the client endpoint.
-    std::shared_ptr<SessionRequestHandler>
-        hle_handler; ///< This session's HLE request handler (applicable when not a domain)
+    /// Called when a client disconnection occurs.
+    void ClientDisconnected();
 
-    /// This is the list of domain request handlers (after conversion to a domain)
-    std::vector<std::shared_ptr<SessionRequestHandler>> domain_request_handlers;
+    /// Adds a new domain request handler to the collection of request handlers within
+    /// this ServerSession instance.
+    void AppendDomainRequestHandler(std::shared_ptr<SessionRequestHandler> handler);
 
-    /// List of threads that are pending a response after a sync request. This list is processed in
-    /// a LIFO manner, thus, the last request will be dispatched first.
-    /// TODO(Subv): Verify if this is indeed processed in LIFO using a hardware test.
-    std::vector<SharedPtr<Thread>> pending_requesting_threads;
-
-    /// Thread whose request is currently being handled. A request is considered "handled" when a
-    /// response is sent via svcReplyAndReceive.
-    /// TODO(Subv): Find a better name for this.
-    SharedPtr<Thread> currently_handling;
+    /// Retrieves the total number of domain request handlers that have been
+    /// appended to this ServerSession instance.
+    std::size_t NumDomainRequestHandlers() const;
 
     /// Returns true if the session has been converted to a domain, otherwise False
     bool IsDomain() const {
@@ -129,8 +130,30 @@ private:
     /// object handle.
     ResultCode HandleDomainSyncRequest(Kernel::HLERequestContext& context);
 
+    /// The parent session, which links to the client endpoint.
+    std::shared_ptr<Session> parent;
+
+    /// This session's HLE request handler (applicable when not a domain)
+    std::shared_ptr<SessionRequestHandler> hle_handler;
+
+    /// This is the list of domain request handlers (after conversion to a domain)
+    std::vector<std::shared_ptr<SessionRequestHandler>> domain_request_handlers;
+
+    /// List of threads that are pending a response after a sync request. This list is processed in
+    /// a LIFO manner, thus, the last request will be dispatched first.
+    /// TODO(Subv): Verify if this is indeed processed in LIFO using a hardware test.
+    std::vector<SharedPtr<Thread>> pending_requesting_threads;
+
+    /// Thread whose request is currently being handled. A request is considered "handled" when a
+    /// response is sent via svcReplyAndReceive.
+    /// TODO(Subv): Find a better name for this.
+    SharedPtr<Thread> currently_handling;
+
     /// When set to True, converts the session to a domain at the end of the command
     bool convert_to_domain{};
+
+    /// The name of this session (optional)
+    std::string name;
 };
 
 } // namespace Kernel

src/core/hle/kernel/svc.cpp

@@ -47,6 +47,23 @@ constexpr bool IsValidAddressRange(VAddr address, u64 size) {
     return address + size > address;
 }
 
+// Checks if a given address range lies within a larger address range.
+constexpr bool IsInsideAddressRange(VAddr address, u64 size, VAddr address_range_begin,
+                                    VAddr address_range_end) {
+    const VAddr end_address = address + size - 1;
+    return address_range_begin <= address && end_address <= address_range_end - 1;
+}
+
+bool IsInsideAddressSpace(const VMManager& vm, VAddr address, u64 size) {
+    return IsInsideAddressRange(address, size, vm.GetAddressSpaceBaseAddress(),
+                                vm.GetAddressSpaceEndAddress());
+}
+
+bool IsInsideNewMapRegion(const VMManager& vm, VAddr address, u64 size) {
+    return IsInsideAddressRange(address, size, vm.GetNewMapRegionBaseAddress(),
+                                vm.GetNewMapRegionEndAddress());
+}
+
 // 8 GiB
 constexpr u64 MAIN_MEMORY_SIZE = 0x200000000;
 
@@ -88,14 +105,14 @@ ResultCode MapUnmapMemorySanityChecks(const VMManager& vm_manager, VAddr dst_add
         return ERR_INVALID_ADDRESS_STATE;
     }
 
-    if (!vm_manager.IsWithinAddressSpace(src_addr, size)) {
+    if (!IsInsideAddressSpace(vm_manager, src_addr, size)) {
         LOG_ERROR(Kernel_SVC,
                   "Source is not within the address space, addr=0x{:016X}, size=0x{:016X}",
                   src_addr, size);
         return ERR_INVALID_ADDRESS_STATE;
     }
 
-    if (!vm_manager.IsWithinNewMapRegion(dst_addr, size)) {
+    if (!IsInsideNewMapRegion(vm_manager, dst_addr, size)) {
         LOG_ERROR(Kernel_SVC,
                   "Destination is not within the new map region, addr=0x{:016X}, size=0x{:016X}",
                   dst_addr, size);
@@ -221,7 +238,7 @@ static ResultCode SetMemoryPermission(VAddr addr, u64 size, u32 prot) {
     auto* const current_process = Core::CurrentProcess();
     auto& vm_manager = current_process->VMManager();
 
-    if (!vm_manager.IsWithinAddressSpace(addr, size)) {
+    if (!IsInsideAddressSpace(vm_manager, addr, size)) {
         LOG_ERROR(Kernel_SVC,
                   "Source is not within the address space, addr=0x{:016X}, size=0x{:016X}", addr,
                   size);
@@ -282,7 +299,7 @@ static ResultCode SetMemoryAttribute(VAddr address, u64 size, u32 mask, u32 attr
     }
 
     auto& vm_manager = Core::CurrentProcess()->VMManager();
-    if (!vm_manager.IsWithinAddressSpace(address, size)) {
+    if (!IsInsideAddressSpace(vm_manager, address, size)) {
         LOG_ERROR(Kernel_SVC,
                   "Given address (0x{:016X}) is outside the bounds of the address space.", address);
         return ERR_INVALID_ADDRESS_STATE;

src/core/hle/kernel/vm_manager.cpp

@@ -17,8 +17,8 @@
 #include "core/memory_setup.h"
 
 namespace Kernel {
-namespace {
-const char* GetMemoryStateName(MemoryState state) {
+
+static const char* GetMemoryStateName(MemoryState state) {
     static constexpr const char* names[] = {
         "Unmapped",         "Io",
         "Normal",           "CodeStatic",
@@ -35,14 +35,6 @@ const char* GetMemoryStateName(MemoryState state) {
     return names[ToSvcMemoryState(state)];
 }
 
-// Checks if a given address range lies within a larger address range.
-constexpr bool IsInsideAddressRange(VAddr address, u64 size, VAddr address_range_begin,
-                                    VAddr address_range_end) {
-    const VAddr end_address = address + size - 1;
-    return address_range_begin <= address && end_address <= address_range_end - 1;
-}
-} // Anonymous namespace
-
 bool VirtualMemoryArea::CanBeMergedWith(const VirtualMemoryArea& next) const {
     ASSERT(base + size == next.base);
     if (permissions != next.permissions || state != next.state || attribute != next.attribute ||
@@ -257,7 +249,8 @@ ResultCode VMManager::ReprotectRange(VAddr target, u64 size, VMAPermission new_p
 }
 
 ResultVal<VAddr> VMManager::HeapAllocate(VAddr target, u64 size, VMAPermission perms) {
-    if (!IsWithinHeapRegion(target, size)) {
+    if (target < GetHeapRegionBaseAddress() || target + size > GetHeapRegionEndAddress() ||
+        target + size < target) {
         return ERR_INVALID_ADDRESS;
     }
 
@@ -292,7 +285,8 @@ ResultVal<VAddr> VMManager::HeapAllocate(VAddr target, u64 size, VMAPermission p
 }
 
 ResultCode VMManager::HeapFree(VAddr target, u64 size) {
-    if (!IsWithinHeapRegion(target, size)) {
+    if (target < GetHeapRegionBaseAddress() || target + size > GetHeapRegionEndAddress() ||
+        target + size < target) {
         return ERR_INVALID_ADDRESS;
     }
 
@@ -712,11 +706,6 @@ u64 VMManager::GetAddressSpaceWidth() const {
     return address_space_width;
 }
 
-bool VMManager::IsWithinAddressSpace(VAddr address, u64 size) const {
-    return IsInsideAddressRange(address, size, GetAddressSpaceBaseAddress(),
-                                GetAddressSpaceEndAddress());
-}
-
 VAddr VMManager::GetASLRRegionBaseAddress() const {
     return aslr_region_base;
 }
@@ -761,11 +750,6 @@ u64 VMManager::GetCodeRegionSize() const {
     return code_region_end - code_region_base;
 }
 
-bool VMManager::IsWithinCodeRegion(VAddr address, u64 size) const {
-    return IsInsideAddressRange(address, size, GetCodeRegionBaseAddress(),
-                                GetCodeRegionEndAddress());
-}
-
 VAddr VMManager::GetHeapRegionBaseAddress() const {
     return heap_region_base;
 }
@@ -778,11 +762,6 @@ u64 VMManager::GetHeapRegionSize() const {
     return heap_region_end - heap_region_base;
 }
 
-bool VMManager::IsWithinHeapRegion(VAddr address, u64 size) const {
-    return IsInsideAddressRange(address, size, GetHeapRegionBaseAddress(),
-                                GetHeapRegionEndAddress());
-}
-
 VAddr VMManager::GetMapRegionBaseAddress() const {
     return map_region_base;
 }
@@ -795,10 +774,6 @@ u64 VMManager::GetMapRegionSize() const {
     return map_region_end - map_region_base;
 }
 
-bool VMManager::IsWithinMapRegion(VAddr address, u64 size) const {
-    return IsInsideAddressRange(address, size, GetMapRegionBaseAddress(), GetMapRegionEndAddress());
-}
-
 VAddr VMManager::GetNewMapRegionBaseAddress() const {
     return new_map_region_base;
 }
@@ -811,11 +786,6 @@ u64 VMManager::GetNewMapRegionSize() const {
     return new_map_region_end - new_map_region_base;
 }
 
-bool VMManager::IsWithinNewMapRegion(VAddr address, u64 size) const {
-    return IsInsideAddressRange(address, size, GetNewMapRegionBaseAddress(),
-                                GetNewMapRegionEndAddress());
-}
-
 VAddr VMManager::GetTLSIORegionBaseAddress() const {
     return tls_io_region_base;
 }
@@ -828,9 +798,4 @@ u64 VMManager::GetTLSIORegionSize() const {
     return tls_io_region_end - tls_io_region_base;
 }
 
-bool VMManager::IsWithinTLSIORegion(VAddr address, u64 size) const {
-    return IsInsideAddressRange(address, size, GetTLSIORegionBaseAddress(),
-                                GetTLSIORegionEndAddress());
-}
-
 } // namespace Kernel

src/core/hle/kernel/vm_manager.h

@@ -432,21 +432,18 @@ public:
     /// Gets the address space width in bits.
     u64 GetAddressSpaceWidth() const;
 
-    /// Determines whether or not the given address range lies within the address space.
-    bool IsWithinAddressSpace(VAddr address, u64 size) const;
-
     /// Gets the base address of the ASLR region.
     VAddr GetASLRRegionBaseAddress() const;
 
     /// Gets the end address of the ASLR region.
     VAddr GetASLRRegionEndAddress() const;
 
-    /// Gets the size of the ASLR region
-    u64 GetASLRRegionSize() const;
-
     /// Determines whether or not the specified address range is within the ASLR region.
     bool IsWithinASLRRegion(VAddr address, u64 size) const;
 
+    /// Gets the size of the ASLR region
+    u64 GetASLRRegionSize() const;
+
     /// Gets the base address of the code region.
     VAddr GetCodeRegionBaseAddress() const;
 
@@ -456,9 +453,6 @@ public:
     /// Gets the total size of the code region in bytes.
     u64 GetCodeRegionSize() const;
 
-    /// Determines whether or not the specified range is within the code region.
-    bool IsWithinCodeRegion(VAddr address, u64 size) const;
-
     /// Gets the base address of the heap region.
     VAddr GetHeapRegionBaseAddress() const;
 
@@ -468,9 +462,6 @@ public:
     /// Gets the total size of the heap region in bytes.
     u64 GetHeapRegionSize() const;
 
-    /// Determines whether or not the specified range is within the heap region.
-    bool IsWithinHeapRegion(VAddr address, u64 size) const;
-
     /// Gets the base address of the map region.
     VAddr GetMapRegionBaseAddress() const;
 
@@ -480,9 +471,6 @@ public:
     /// Gets the total size of the map region in bytes.
     u64 GetMapRegionSize() const;
 
-    /// Determines whether or not the specified range is within the map region.
-    bool IsWithinMapRegion(VAddr address, u64 size) const;
-
     /// Gets the base address of the new map region.
     VAddr GetNewMapRegionBaseAddress() const;
 
@@ -492,9 +480,6 @@ public:
     /// Gets the total size of the new map region in bytes.
     u64 GetNewMapRegionSize() const;
 
-    /// Determines whether or not the given address range is within the new map region
-    bool IsWithinNewMapRegion(VAddr address, u64 size) const;
-
     /// Gets the base address of the TLS IO region.
     VAddr GetTLSIORegionBaseAddress() const;
 
@@ -504,9 +489,6 @@ public:
     /// Gets the total size of the TLS IO region in bytes.
     u64 GetTLSIORegionSize() const;
 
-    /// Determines if the given address range is within the TLS IO region.
-    bool IsWithinTLSIORegion(VAddr address, u64 size) const;
-
     /// Each VMManager has its own page table, which is set as the main one when the owning process
     /// is scheduled.
     Memory::PageTable page_table;

src/core/hle/service/sm/controller.cpp

@@ -30,7 +30,7 @@ void Controller::DuplicateSession(Kernel::HLERequestContext& ctx) {
 
     IPC::ResponseBuilder rb{ctx, 2, 0, 1, IPC::ResponseBuilder::Flags::AlwaysMoveHandles};
     rb.Push(RESULT_SUCCESS);
-    Kernel::SharedPtr<Kernel::ClientSession> session{ctx.Session()->parent->client};
+    Kernel::SharedPtr<Kernel::ClientSession> session{ctx.Session()->GetParent()->client};
     rb.PushMoveObjects(session);
 
     LOG_DEBUG(Service, "session={}", session->GetObjectId());

src/core/memory.cpp

@@ -171,9 +171,6 @@ T Read(const VAddr vaddr) {
         return value;
     }
 
-    // The memory access might do an MMIO or cached access, so we have to lock the HLE kernel state
-    std::lock_guard<std::recursive_mutex> lock(HLE::g_hle_lock);
-
     PageType type = current_page_table->attributes[vaddr >> PAGE_BITS];
     switch (type) {
     case PageType::Unmapped:
@@ -204,9 +201,6 @@ void Write(const VAddr vaddr, const T data) {
         return;
     }
 
-    // The memory access might do an MMIO or cached access, so we have to lock the HLE kernel state
-    std::lock_guard<std::recursive_mutex> lock(HLE::g_hle_lock);
-
     PageType type = current_page_table->attributes[vaddr >> PAGE_BITS];
     switch (type) {
     case PageType::Unmapped: