Merge pull request #12872 from LillyJadeKatrin/retroachievements-start-session-bug

Add MemoryVerifier to AchievementManager Startup

Source/Core/Core/AchievementManager.cpp

@@ -13,6 +13,7 @@
 #include <rcheevos/include/rc_api_info.h>
 #include <rcheevos/include/rc_hash.h>
 
+#include "Common/Assert.h"
 #include "Common/CommonPaths.h"
 #include "Common/FileUtil.h"
 #include "Common/Image.h"
@@ -22,6 +23,7 @@
 #include "Common/WorkQueueThread.h"
 #include "Core/Config/AchievementSettings.h"
 #include "Core/Core.h"
+#include "Core/HW/Memmap.h"
 #include "Core/PowerPC/MMU.h"
 #include "Core/System.h"
 #include "DiscIO/Blob.h"
@@ -44,7 +46,7 @@ void AchievementManager::Init()
   LoadDefaultBadges();
   if (!m_client && Config::Get(Config::RA_ENABLED))
   {
-    m_client = rc_client_create(MemoryPeeker, Request);
+    m_client = rc_client_create(MemoryVerifier, Request);
     std::string host_url = Config::Get(Config::RA_HOST_URL);
     if (!host_url.empty())
       rc_client_set_host(m_client, host_url.c_str());
@@ -120,6 +122,7 @@ void AchievementManager::LoadGame(const std::string& file_path, const DiscIO::Vo
   rc_client_set_unofficial_enabled(m_client, Config::Get(Config::RA_UNOFFICIAL_ENABLED));
   rc_client_set_encore_mode_enabled(m_client, Config::Get(Config::RA_ENCORE_ENABLED));
   rc_client_set_spectator_mode_enabled(m_client, Config::Get(Config::RA_SPECTATOR_ENABLED));
+  rc_client_set_read_memory_function(m_client, MemoryVerifier);
   if (volume)
   {
     std::lock_guard lg{m_lock};
@@ -679,13 +682,14 @@ void AchievementManager::LoadGameCallback(int result, const char* error_message,
   }
   INFO_LOG_FMT(ACHIEVEMENTS, "Loaded data for game ID {}.", game->id);
 
-  AchievementManager::GetInstance().m_display_welcome_message = true;
+  auto& instance = AchievementManager::GetInstance();
-  AchievementManager::GetInstance().FetchGameBadges();
+  rc_client_set_read_memory_function(instance.m_client, MemoryPeeker);
-  AchievementManager::GetInstance().m_system = &Core::System::GetInstance();
+  instance.m_display_welcome_message = true;
-  AchievementManager::GetInstance().m_update_callback({.all = true});
+  instance.FetchGameBadges();
+  instance.m_system = &Core::System::GetInstance();
+  instance.m_update_callback({.all = true});
   // Set this to a value that will immediately trigger RP
-  AchievementManager::GetInstance().m_last_rp_time =
+  instance.m_last_rp_time = std::chrono::steady_clock::now() - std::chrono::minutes{2};
-      std::chrono::steady_clock::now() - std::chrono::minutes{2};
 }
 
 void AchievementManager::ChangeMediaCallback(int result, const char* error_message,
@@ -706,6 +710,7 @@ void AchievementManager::ChangeMediaCallback(int result, const char* error_messa
 
     ERROR_LOG_FMT(ACHIEVEMENTS, "RetroAchievements media change failed: {}", error_message);
   }
+  rc_client_set_read_memory_function(AchievementManager::GetInstance().m_client, MemoryPeeker);
 }
 
 void AchievementManager::DisplayWelcomeMessage()
@@ -914,11 +919,36 @@ void AchievementManager::Request(const rc_api_request_t* request,
       });
 }
 
+// Currently, when rc_client calls the memory peek method provided in its constructor (or in
+// rc_client_set_read_memory_function) it will do so on the thread that calls DoFrame, which is
+// currently the host thread, with one exception: an asynchronous callback in the load game process.
+// This is done to validate/invalidate each memory reference in the downloaded assets, mark assets
+// as unsupported, and notify the player upon startup that there are unsupported assets and how
+// many. As such, all that call needs to do is return the number of bytes that can be read with this
+// call. As only the CPU and host threads are allowed to read from memory, I provide a separate
+// method for this verification. In lieu of a more convenient set of steps, I provide MemoryVerifier
+// to rc_client at construction, and in the Load Game callback, after the verification has been
+// complete, I call rc_client_set_read_memory_function to switch to the usual MemoryPeeker for all
+// future synchronous calls.
+u32 AchievementManager::MemoryVerifier(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client)
+{
+  auto& system = Core::System::GetInstance();
+  u32 ram_size = system.GetMemory().GetRamSizeReal();
+  if (address >= ram_size)
+    return 0;
+  return std::min(ram_size - address, num_bytes);
+}
+
 u32 AchievementManager::MemoryPeeker(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client)
 {
   if (buffer == nullptr)
     return 0u;
   auto& system = Core::System::GetInstance();
+  if (!(Core::IsHostThread() || Core::IsCPUThread()))
+  {
+    ASSERT_MSG(ACHIEVEMENTS, false, "MemoryPeeker called from wrong thread");
+    return 0;
+  }
   Core::CPUThreadGuard threadguard(system);
   for (u32 num_read = 0; num_read < num_bytes; num_read++)
   {

Source/Core/Core/AchievementManager.h

@@ -177,6 +177,7 @@ private:
 
   static void Request(const rc_api_request_t* request, rc_client_server_callback_t callback,
                       void* callback_data, rc_client_t* client);
+  static u32 MemoryVerifier(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client);
   static u32 MemoryPeeker(u32 address, u8* buffer, u32 num_bytes, rc_client_t* client);
   void FetchBadge(Badge* badge, u32 badge_type, const BadgeNameFunction function,
                   const UpdatedItems callback_data);