Print guest stack trace on invalid memory access (#1407)

* Print guest stack trace on invalid memory access

* Improve XML docs

ARMeilleure/Instructions/InstEmitMemoryHelper.cs

@@ -326,7 +326,7 @@ private static Operand EmitPtPointerLoad(ArmEmitterContext context, Operand addr
             }
             while (bit < context.Memory.AddressSpaceBits);
 
-            context.BranchIfTrue(lblSlowPath, context.ICompareLess(pte, Const(0L)));
+            context.BranchIfTrue(lblSlowPath, context.ICompareLessOrEqual(pte, Const(0L)));
 
             Operand pageOffset = context.BitwiseAnd(address, Const(address.Type, PageMask));
 

Ryujinx.Cpu/InvalidAccessHandler.cs

@@ -0,0 +1,9 @@
+namespace Ryujinx.Cpu
+{
+    /// <summary>
+    /// Function that handles a invalid memory access from the emulated CPU.
+    /// </summary>
+    /// <param name="va">Virtual address of the invalid region that is being accessed</param>
+    /// <returns>True if the invalid access should be ignored, false otherwise</returns>
+    public delegate bool InvalidAccessHandler(ulong va);
+}

Ryujinx.Cpu/MemoryManager.cs

@@ -18,22 +18,33 @@ public sealed class MemoryManager : IMemoryManager, IDisposable
 
         private const int PteSize = 8;
 
+        private readonly InvalidAccessHandler _invalidAccessHandler;
+
+        /// <summary>
+        /// Address space width in bits.
+        /// </summary>
         public int AddressSpaceBits { get; }
 
         private readonly ulong _addressSpaceSize;
 
         private readonly MemoryBlock _backingMemory;
         private readonly MemoryBlock _pageTable;
 
+        /// <summary>
+        /// Page table base pointer.
+        /// </summary>
         public IntPtr PageTablePointer => _pageTable.Pointer;
 
         /// <summary>
         /// Creates a new instance of the memory manager.
         /// </summary>
         /// <param name="backingMemory">Physical backing memory where virtual memory will be mapped to</param>
         /// <param name="addressSpaceSize">Size of the address space</param>
-        public MemoryManager(MemoryBlock backingMemory, ulong addressSpaceSize)
+        /// <param name="invalidAccessHandler">Optional function to handle invalid memory accesses</param>
+        public MemoryManager(MemoryBlock backingMemory, ulong addressSpaceSize, InvalidAccessHandler invalidAccessHandler = null)
         {
+            _invalidAccessHandler = invalidAccessHandler;
+
             ulong asSize = PageSize;
             int asBits = PageBits;
 
@@ -92,6 +103,7 @@ public void Unmap(ulong va, ulong size)
         /// <typeparam name="T">Type of the data being read</typeparam>
         /// <param name="va">Virtual address of the data in memory</param>
         /// <returns>The data</returns>
+        /// <exception cref="InvalidMemoryRegionException">Throw for unhandled invalid or unmapped memory accesses</exception>
         public T Read<T>(ulong va) where T : unmanaged
         {
             return MemoryMarshal.Cast<byte, T>(GetSpan(va, Unsafe.SizeOf<T>()))[0];
@@ -102,6 +114,7 @@ public T Read<T>(ulong va) where T : unmanaged
         /// </summary>
         /// <param name="va">Virtual address of the data in memory</param>
         /// <param name="data">Span to store the data being read into</param>
+        /// <exception cref="InvalidMemoryRegionException">Throw for unhandled invalid or unmapped memory accesses</exception>
         public void Read(ulong va, Span<byte> data)
         {
             ReadImpl(va, data);
@@ -113,6 +126,7 @@ public void Read(ulong va, Span<byte> data)
         /// <typeparam name="T">Type of the data being written</typeparam>
         /// <param name="va">Virtual address to write the data into</param>
         /// <param name="value">Data to be written</param>
+        /// <exception cref="InvalidMemoryRegionException">Throw for unhandled invalid or unmapped memory accesses</exception>
         public void Write<T>(ulong va, T value) where T : unmanaged
         {
             Write(va, MemoryMarshal.Cast<T, byte>(MemoryMarshal.CreateSpan(ref value, 1)));
@@ -123,41 +137,52 @@ public void Write<T>(ulong va, T value) where T : unmanaged
         /// </summary>
         /// <param name="va">Virtual address to write the data into</param>
         /// <param name="data">Data to be written</param>
+        /// <exception cref="InvalidMemoryRegionException">Throw for unhandled invalid or unmapped memory accesses</exception>
         public void Write(ulong va, ReadOnlySpan<byte> data)
         {
             if (data.Length == 0)
             {
                 return;
             }
 
-            MarkRegionAsModified(va, (ulong)data.Length);
-
-            if (IsContiguous(va, data.Length))
+            try
             {
-                data.CopyTo(_backingMemory.GetSpan(GetPhysicalAddressInternal(va), data.Length));
-            }
-            else
-            {
-                int offset = 0, size;
+                MarkRegionAsModified(va, (ulong)data.Length);
 
-                if ((va & PageMask) != 0)
+                if (IsContiguousAndMapped(va, data.Length))
                 {
-                    ulong pa = GetPhysicalAddressInternal(va);
+                    data.CopyTo(_backingMemory.GetSpan(GetPhysicalAddressInternal(va), data.Length));
+                }
+                else
+                {
+                    int offset = 0, size;
 
-                    size = Math.Min(data.Length, PageSize - (int)(va & PageMask));
+                    if ((va & PageMask) != 0)
+                    {
+                        ulong pa = GetPhysicalAddressInternal(va);
 
-                    data.Slice(0, size).CopyTo(_backingMemory.GetSpan(pa, size));
+                        size = Math.Min(data.Length, PageSize - (int)(va & PageMask));
 
-                    offset += size;
-                }
+                        data.Slice(0, size).CopyTo(_backingMemory.GetSpan(pa, size));
 
-                for (; offset < data.Length; offset += size)
-                {
-                    ulong pa = GetPhysicalAddressInternal(va + (ulong)offset);
+                        offset += size;
+                    }
 
-                    size = Math.Min(data.Length - offset, PageSize);
+                    for (; offset < data.Length; offset += size)
+                    {
+                        ulong pa = GetPhysicalAddressInternal(va + (ulong)offset);
+
+                        size = Math.Min(data.Length - offset, PageSize);
 
-                    data.Slice(offset, size).CopyTo(_backingMemory.GetSpan(pa, size));
+                        data.Slice(offset, size).CopyTo(_backingMemory.GetSpan(pa, size));
+                    }
+                }
+            }
+            catch (InvalidMemoryRegionException)
+            {
+                if (_invalidAccessHandler == null || !_invalidAccessHandler(va))
+                {
+                    throw;
                 }
             }
         }
@@ -172,14 +197,15 @@ public void Write(ulong va, ReadOnlySpan<byte> data)
         /// <param name="va">Virtual address of the data</param>
         /// <param name="size">Size of the data</param>
         /// <returns>A read-only span of the data</returns>
+        /// <exception cref="InvalidMemoryRegionException">Throw for unhandled invalid or unmapped memory accesses</exception>
         public ReadOnlySpan<byte> GetSpan(ulong va, int size)
         {
             if (size == 0)
             {
                 return ReadOnlySpan<byte>.Empty;
             }
 
-            if (IsContiguous(va, size))
+            if (IsContiguousAndMapped(va, size))
             {
                 return _backingMemory.GetSpan(GetPhysicalAddressInternal(va), size);
             }
@@ -204,14 +230,15 @@ public ReadOnlySpan<byte> GetSpan(ulong va, int size)
         /// <param name="va">Virtual address of the data</param>
         /// <param name="size">Size of the data</param>
         /// <returns>A writable region of memory containing the data</returns>
+        /// <exception cref="InvalidMemoryRegionException">Throw for unhandled invalid or unmapped memory accesses</exception>
         public WritableRegion GetWritableRegion(ulong va, int size)
         {
             if (size == 0)
             {
                 return new WritableRegion(null, va, Memory<byte>.Empty);
             }
 
-            if (IsContiguous(va, size))
+            if (IsContiguousAndMapped(va, size))
             {
                 return new WritableRegion(null, va, _backingMemory.GetMemory(GetPhysicalAddressInternal(va), size));
             }
@@ -234,6 +261,7 @@ public WritableRegion GetWritableRegion(ulong va, int size)
         /// <typeparam name="T">Type of the data to get the reference</typeparam>
         /// <param name="va">Virtual address of the data</param>
         /// <returns>A reference to the data in memory</returns>
+        /// <exception cref="MemoryNotContiguousException">Throw if the specified memory region is not contiguous in physical memory</exception>
         public ref T GetRef<T>(ulong va) where T : unmanaged
         {
             if (!IsContiguous(va, Unsafe.SizeOf<T>()))
@@ -256,6 +284,9 @@ public ref T GetRefNoChecks<T>(ulong va) where T : unmanaged
             return ref _backingMemory.GetRef<T>(GetPhysicalAddressInternal(va));
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool IsContiguousAndMapped(ulong va, int size) => IsContiguous(va, size) && IsMapped(va);
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private bool IsContiguous(ulong va, int size)
         {
@@ -295,26 +326,36 @@ private void ReadImpl(ulong va, Span<byte> data)
                 return;
             }
 
-            int offset = 0, size;
-
-            if ((va & PageMask) != 0)
+            try
             {
-                ulong pa = GetPhysicalAddressInternal(va);
+                int offset = 0, size;
 
-                size = Math.Min(data.Length, PageSize - (int)(va & PageMask));
+                if ((va & PageMask) != 0)
+                {
+                    ulong pa = GetPhysicalAddressInternal(va);
 
-                _backingMemory.GetSpan(pa, size).CopyTo(data.Slice(0, size));
+                    size = Math.Min(data.Length, PageSize - (int)(va & PageMask));
 
-                offset += size;
-            }
+                    _backingMemory.GetSpan(pa, size).CopyTo(data.Slice(0, size));
 
-            for (; offset < data.Length; offset += size)
-            {
-                ulong pa = GetPhysicalAddressInternal(va + (ulong)offset);
+                    offset += size;
+                }
 
-                size = Math.Min(data.Length - offset, PageSize);
+                for (; offset < data.Length; offset += size)
+                {
+                    ulong pa = GetPhysicalAddressInternal(va + (ulong)offset);
+
+                    size = Math.Min(data.Length - offset, PageSize);
 
-                _backingMemory.GetSpan(pa, size).CopyTo(data.Slice(offset, size));
+                    _backingMemory.GetSpan(pa, size).CopyTo(data.Slice(offset, size));
+                }
+            }
+            catch (InvalidMemoryRegionException)
+            {
+                if (_invalidAccessHandler == null || !_invalidAccessHandler(va))
+                {
+                    throw;
+                }
             }
         }
 
@@ -416,6 +457,7 @@ public int QueryModified(ulong va, ulong size, int id, (ulong, ulong)[] modified
         /// </summary>
         /// <param name="va">Virtual address to check</param>
         /// <returns>True if the address is mapped, false otherwise</returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public bool IsMapped(ulong va)
         {
             if (!ValidateAddress(va))

Ryujinx.HLE/HOS/Kernel/Process/KProcess.cs

@@ -1,5 +1,6 @@
 using ARMeilleure.State;
 using Ryujinx.Common;
+using Ryujinx.Common.Logging;
 using Ryujinx.Cpu;
 using Ryujinx.HLE.Exceptions;
 using Ryujinx.HLE.HOS.Kernel.Common;
@@ -1071,18 +1072,6 @@ public KernelResult ClearIfNotExited()
             return result;
         }
 
-        public void StopAllThreads()
-        {
-            lock (_threadingLock)
-            {
-                foreach (KThread thread in _threads)
-                {
-                    KernelContext.Scheduler.ExitThread(thread);
-                    KernelContext.Scheduler.CoreManager.Set(thread.HostThread);
-                }
-            }
-        }
-
         private void InitializeMemoryManager(AddressSpaceType addrSpaceType, MemoryRegion memRegion)
         {
             int addrSpaceBits = addrSpaceType switch
@@ -1094,7 +1083,7 @@ private void InitializeMemoryManager(AddressSpaceType addrSpaceType, MemoryRegio
                 _ => throw new ArgumentException(nameof(addrSpaceType))
             };
 
-            CpuMemory = new MemoryManager(KernelContext.Memory, 1UL << addrSpaceBits);
+            CpuMemory = new MemoryManager(KernelContext.Memory, 1UL << addrSpaceBits, InvalidAccessHandler);
             CpuContext = new CpuContext(CpuMemory);
 
             // TODO: This should eventually be removed.
@@ -1104,13 +1093,19 @@ private void InitializeMemoryManager(AddressSpaceType addrSpaceType, MemoryRegio
             MemoryManager = new KMemoryManager(KernelContext, CpuMemory);
         }
 
-        public void PrintCurrentThreadStackTrace()
+        private bool InvalidAccessHandler(ulong va)
         {
-            KernelContext.Scheduler.GetCurrentThread().PrintGuestStackTrace();
+            KernelContext.Scheduler.GetCurrentThreadOrNull()?.PrintGuestStackTrace();
+
+            Logger.PrintError(LogClass.Cpu, $"Invalid memory access at virtual address 0x{va:X16}.");
+
+            return false;
         }
 
         private void UndefinedInstructionHandler(object sender, InstUndefinedEventArgs e)
         {
+            KernelContext.Scheduler.GetCurrentThreadOrNull()?.PrintGuestStackTrace();
+
             throw new UndefinedInstructionException(e.Address, e.OpCode);
         }
 

Ryujinx.HLE/HOS/Kernel/Threading/KScheduler.cs

@@ -198,6 +198,11 @@ public void SelectThreads()
         }
 
         public KThread GetCurrentThread()
+        {
+            return GetCurrentThreadOrNull() ?? GetDummyThread();
+        }
+
+        public KThread GetCurrentThreadOrNull()
         {
             lock (CoreContexts)
             {
@@ -210,9 +215,7 @@ public KThread GetCurrentThread()
                 }
             }
 
-            return GetDummyThread();
-
-            throw new InvalidOperationException("Current thread is not scheduled!");
+            return null;
         }
 
         private KThread _dummyThread;

Ryujinx.Memory/InvalidMemoryRegionException.cs

@@ -0,0 +1,19 @@
+using System;
+
+namespace Ryujinx.Memory
+{
+    public class InvalidMemoryRegionException : Exception
+    {
+        public InvalidMemoryRegionException() : base("Attempted to access a invalid memory region.")
+        {
+        }
+
+        public InvalidMemoryRegionException(string message) : base(message)
+        {
+        }
+
+        public InvalidMemoryRegionException(string message, Exception innerException) : base(message, innerException)
+        {
+        }
+    }
+}