Memmap: Replace GetPointer with GetSpanForAddress

To ensure memory safety, callers of GetPointer have to perform a bounds
check. But how is this bounds check supposed to be performed?
GetPointerForRange contained one implementation of a bounds check, but
it was cumbersome, and it also isn't obvious why it's correct.

To make doing the right thing easier, this commit changes GetPointer to
return a span that tells the caller how many bytes it's allowed to
access.

Source/Core/Core/HW/Memmap.cpp

@@ -12,6 +12,7 @@
 #include <array>
 #include <cstring>
 #include <memory>
+#include <span>
 #include <tuple>
 
 #include "Common/ChunkFile.h"
@@ -400,22 +401,23 @@ void MemoryManager::Clear()
 
 u8* MemoryManager::GetPointerForRange(u32 address, size_t size) const
 {
-  // Make sure we don't have a range spanning 2 separate banks
-  if (size >= GetExRamSizeReal())
+  std::span<u8> span = GetSpanForAddress(address);
+
+  if (span.data() == nullptr)
   {
+    // The address isn't in a valid memory region.
+    // A panic alert has already been raised by GetPointer, so let's not raise another one.
+    return nullptr;
+  }
+
+  if (span.size() < size)
+  {
+    // The start address is in a valid region, but the end address is beyond the end of that region.
     PanicAlertFmt("Oversized range in GetPointerForRange. {:x} bytes at {:#010x}", size, address);
     return nullptr;
   }
 
-  // Check that the beginning and end of the range are valid
-  u8* pointer = GetPointer(address);
-  if (pointer == nullptr || (size != 0 && GetPointer(address + u32(size) - 1) == nullptr))
-  {
-    // A panic alert has already been raised by GetPointer
-    return nullptr;
-  }
-
-  return pointer;
+  return span.data();
 }
 
 void MemoryManager::CopyFromEmu(void* data, u32 address, size_t size) const
@@ -487,24 +489,27 @@ std::string MemoryManager::GetString(u32 em_address, size_t size)
   }
 }
 
-u8* MemoryManager::GetPointer(u32 address) const
+std::span<u8> MemoryManager::GetSpanForAddress(u32 address) const
 {
   // TODO: Should we be masking off more bits here?  Can all devices access
   // EXRAM?
   address &= 0x3FFFFFFF;
   if (address < GetRamSizeReal())
-    return m_ram + address;
+    return std::span(m_ram + address, GetRamSizeReal() - address);
 
   if (m_exram)
   {
     if ((address >> 28) == 0x1 && (address & 0x0fffffff) < GetExRamSizeReal())
-      return m_exram + (address & GetExRamMask());
+    {
+      return std::span(m_exram + (address & GetExRamMask()),
+                       GetExRamSizeReal() - (address & GetExRamMask()));
+    }
   }
 
   auto& ppc_state = m_system.GetPPCState();
   PanicAlertFmt("Unknown Pointer {:#010x} PC {:#010x} LR {:#010x}", address, ppc_state.pc,
                 LR(ppc_state));
-  return nullptr;
+  return {};
 }
 
 u8 MemoryManager::Read_U8(u32 address) const

Source/Core/Core/HW/Memmap.h

@@ -5,6 +5,7 @@
 
 #include <array>
 #include <memory>
+#include <span>
 #include <string>
 #include <vector>
 
@@ -105,10 +106,16 @@ public:
   // Routines to access physically addressed memory, designed for use by
   // emulated hardware outside the CPU. Use "Device_" prefix.
   std::string GetString(u32 em_address, size_t size = 0);
-  // WARNING: Incrementing the pointer returned by GetPointer is unsafe without additional bounds
-  // checks. New code should use other functions instead, like GetPointerForRange or CopyFromEmu.
-  u8* GetPointer(u32 address) const;
+
+  // If the specified guest address is within a valid memory region, returns a span starting at the
+  // host address corresponding to the specified address and ending where the memory region ends.
+  // Otherwise, returns a 0-length span starting at nullptr.
+  std::span<u8> GetSpanForAddress(u32 address) const;
+
+  // If the specified range is within a single valid memory region, returns a pointer to the start
+  // of the corresponding range in host memory. Otherwise, returns nullptr.
   u8* GetPointerForRange(u32 address, size_t size) const;
+
   void CopyFromEmu(void* data, u32 address, size_t size) const;
   void CopyToEmu(u32 address, const void* data, size_t size);
   void Memset(u32 address, u8 value, size_t size);

Source/Core/VideoBackends/Software/TextureSampler.cpp

@@ -5,6 +5,7 @@
 
 #include <algorithm>
 #include <cmath>
+#include <span>
 
 #include "Common/CommonTypes.h"
 #include "Common/MsgHandler.h"
@@ -137,7 +138,9 @@ void SampleMip(s32 s, s32 t, s32 mip, bool linear, u8 texmap, u8* sample)
     auto& memory = system.GetMemory();
 
     const u32 imageBase = texUnit.texImage3.image_base << 5;
-    imageSrc = memory.GetPointer(imageBase);
+    // TODO: For memory safety, we need to check the size of this span
+    std::span<const u8> span = memory.GetSpanForAddress(imageBase);
+    imageSrc = span.data();
   }
 
   int image_width_minus_1 = ti0.width;

Source/Core/VideoCommon/TextureInfo.cpp

@@ -3,6 +3,8 @@
 
 #include "VideoCommon/TextureInfo.h"
 
+#include <span>
+
 #include <fmt/format.h>
 #include <xxhash.h>
 
@@ -47,8 +49,10 @@ TextureInfo TextureInfo::FromStage(u32 stage)
 
   auto& system = Core::System::GetInstance();
   auto& memory = system.GetMemory();
-  return TextureInfo(stage, memory.GetPointer(address), tlut_ptr, address, texture_format,
-                     tlut_format, width, height, false, nullptr, nullptr, mip_count);
+  // TODO: For memory safety, we need to check the size of this span
+  std::span<const u8> span = memory.GetSpanForAddress(address);
+  return TextureInfo(stage, span.data(), tlut_ptr, address, texture_format, tlut_format, width,
+                     height, false, nullptr, nullptr, mip_count);
 }
 
 TextureInfo::TextureInfo(u32 stage, const u8* ptr, const u8* tlut_ptr, u32 address,

Source/Core/VideoCommon/VertexLoaderManager.cpp

@@ -91,26 +91,35 @@ void UpdateVertexArrayPointers()
   // Note: Only array bases 0 through 11 are used by the Vertex loaders.
   //       12 through 15 are used for loading data into xfmem.
   // We also only update the array base if the vertex description states we are going to use it.
+  // TODO: For memory safety, we need to check the sizes returned by GetSpanForAddress
   if (IsIndexed(g_main_cp_state.vtx_desc.low.Position))
+  {
     cached_arraybases[CPArray::Position] =
-        memory.GetPointer(g_main_cp_state.array_bases[CPArray::Position]);
+        memory.GetSpanForAddress(g_main_cp_state.array_bases[CPArray::Position]).data();
+  }
 
   if (IsIndexed(g_main_cp_state.vtx_desc.low.Normal))
+  {
     cached_arraybases[CPArray::Normal] =
-        memory.GetPointer(g_main_cp_state.array_bases[CPArray::Normal]);
+        memory.GetSpanForAddress(g_main_cp_state.array_bases[CPArray::Normal]).data();
+  }
 
   for (u8 i = 0; i < g_main_cp_state.vtx_desc.low.Color.Size(); i++)
   {
     if (IsIndexed(g_main_cp_state.vtx_desc.low.Color[i]))
+    {
       cached_arraybases[CPArray::Color0 + i] =
-          memory.GetPointer(g_main_cp_state.array_bases[CPArray::Color0 + i]);
+          memory.GetSpanForAddress(g_main_cp_state.array_bases[CPArray::Color0 + i]).data();
+    }
   }
 
   for (u8 i = 0; i < g_main_cp_state.vtx_desc.high.TexCoord.Size(); i++)
   {
     if (IsIndexed(g_main_cp_state.vtx_desc.high.TexCoord[i]))
+    {
       cached_arraybases[CPArray::TexCoord0 + i] =
-          memory.GetPointer(g_main_cp_state.array_bases[CPArray::TexCoord0 + i]);
+          memory.GetSpanForAddress(g_main_cp_state.array_bases[CPArray::TexCoord0 + i]).data();
+    }
   }
 
   g_bases_dirty = false;