Merge 04766585f1 into 10798c3464

* fix use-after-free of inContent
* don't try to free the DStream twice

CMakeLists.txt

@@ -8,6 +8,7 @@ endif()
 set(CMAKE_POLICY_DEFAULT_CMP0069 NEW)
 
 set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH})
+set(CMAKE_USER_MAKE_RULES_OVERRIDE "${CMAKE_SOURCE_DIR}/cmake/DefaultBuildFlags.cmake")
 
 option(USE_VCPKG "Use vcpkg for dependency packages" OFF)
 if (USE_VCPKG)
@@ -78,14 +79,6 @@ if (ENABLE_LTO)
     set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
 endif()
 
-if (CMAKE_CXX_COMPILER_ID STREQUAL GNU)
-    set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Og")
-    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -Og")
-endif()
-
-string(REPLACE "-O2" "-O3" CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE}")
-string(REPLACE "-O2" "-O3" CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}")
-
 if (NOT APPLE)
     set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} -s")
 endif()

README.md

@@ -6,10 +6,9 @@
 <a href="https://www.gnu.org/licenses/gpl-3.0" alt="License: GPLv3"><img src="https://img.shields.io/badge/License-GPL%20v3-%23ff554d.svg"></a>
 <a href="https://kiwiirc.com/client/irc.badnik.net/?nick=IRC-Source_?#melonds" alt="IRC channel: #melonds"><img src="https://img.shields.io/badge/IRC%20chat-%23melonds-%23dd2e44.svg"></a>
 <br>
-<a href="https://github.com/melonDS-emu/melonDS/actions?query=workflow%3A%22CMake+Build+%28Windows+x86-64%29%22+event%3Apush"><img src="https://img.shields.io/github/actions/workflow/status/melonDS-emu/melonDS/build-windows.yml?label=Windows%20x86-64&logo=GitHub&branch=master"></img></a>
-<a href="https://github.com/melonDS-emu/melonDS/actions?query=workflow%3A%22CMake+Build+%28Ubuntu+x86-64%29%22+event%3Apush"><img src="https://img.shields.io/github/actions/workflow/status/melonDS-emu/melonDS/build-ubuntu.yml?label=Linux%20x86-64&logo=GitHub"></img></a>
-<a href="https://github.com/melonDS-emu/melonDS/actions?query=workflow%3A%22CMake+Build+%28Ubuntu+aarch64%29%22+event%3Apush"><img src="https://img.shields.io/github/actions/workflow/status/melonDS-emu/melonDS/build-ubuntu-aarch64.yml?label=Linux%20ARM64&logo=GitHub"></img></a>
-<a href="https://github.com/melonDS-emu/melonDS/actions/workflows/build-macos-universal.yml?query=event%3Apush"><img src="https://img.shields.io/github/actions/workflow/status/melonDS-emu/melonDS/build-macos.yml?label=macOS%20Universal&logo=GitHub"></img></a>
+<a href="https://github.com/melonDS-emu/melonDS/actions/workflows/build-windows.yml?query=event%3Apush"><img src="https://github.com/melonDS-emu/melonDS/actions/workflows/build-windows.yml/badge.svg" /></a>
+<a href="https://github.com/melonDS-emu/melonDS/actions/workflows/build-ubuntu.yml?query=event%3Apush"><img src="https://github.com/melonDS-emu/melonDS/actions/workflows/build-ubuntu.yml/badge.svg" /></a>
+<a href="https://github.com/melonDS-emu/melonDS/actions/workflows/build-macos.yml?query=event%3Apush"><img src="https://github.com/melonDS-emu/melonDS/actions/workflows/build-macos.yml/badge.svg" /></a>
 </p>
 DS emulator, sorta
 

cmake/DefaultBuildFlags.cmake

@@ -0,0 +1,9 @@
+if (CMAKE_C_COMPILER_ID STREQUAL GNU)
+	set(CMAKE_C_FLAGS_DEBUG_INIT "-g -Og")
+endif()
+if (CMAKE_CXX_COMPILER_ID STREQUAL GNU)
+	set(CMAKE_CXX_FLAGS_DEBUG_INIT "-g -Og")
+endif()
+
+string(REPLACE "-O2" "-O3" CMAKE_C_FLAGS_RELEASE_INIT "${CMAKE_C_FLAGS_RELEASE_INIT}")
+string(REPLACE "-O2" "-O3" CMAKE_CXX_FLAGS_RELEASE_INIT "${CMAKE_CXX_FLAGS_RELEASE_INIT}")

src/CP15.cpp

@@ -186,10 +186,14 @@ void ARMv5::UpdatePURegion(u32 n)
         return;
     }
 
-    u32 start = rgn >> 12;
-    u32 sz = 2 << ((rgn >> 1) & 0x1F);
-    u32 end = start + (sz >> 12);
-    // TODO: check alignment of start
+    // notes:
+    // * min size of a pu region is 4KiB (12 bits)
+    // * size is calculated as size + 1, but the 12 lsb of address space are ignored, therefore we need it as size + 1 - 12, or size - 11
+    // * pu regions are aligned based on their size
+    u32 size = std::max((int)((rgn>>1) & 0x1F) - 11, 0); // obtain the size, subtract 11 and clamp to a min of 0.
+    u32 start = ((rgn >> 12) >> size) << size; // determine the start offset, and use shifts to force alignment with a multiple of the size.
+    u32 end = start + (1<<size); // add 1 left shifted by size to start to determine end point
+    // dont need to bounds check the end point because the force alignment inherently prevents it from breaking
 
     u8 usermask = 0;
     u8 privmask = 0;
@@ -239,7 +243,7 @@ void ARMv5::UpdatePURegion(u32 n)
         "PU region %d: %08X-%08X, user=%02X priv=%02X, %08X/%08X\n",
         n,
         start << 12,
-        end << 12,
+        (end << 12) - 1,
         usermask,
         privmask,
         PU_DataRW,
@@ -579,12 +583,12 @@ void ARMv5::CP15Write(u32 id, u32 val)
 
         std::snprintf(log_output,
                  sizeof(log_output),
-                 "PU: region %d = %08X : %s, %08X-%08X\n",
+                 "PU: region %d = %08X : %s, start: %08X size: %02X\n",
                  (id >> 4) & 0xF,
                  val,
                  val & 1 ? "enabled" : "disabled",
                  val & 0xFFFFF000,
-                 (val & 0xFFFFF000) + (2 << ((val & 0x3E) >> 1))
+                 (val & 0x3E) >> 1
         );
         Log(LogLevel::Debug, "%s", log_output);
         // Some implementations of Log imply a newline, so we build up the line before printing it

src/GPU3D_Soft.h

@@ -282,7 +282,6 @@ private:
             // slope increment has a 18-bit fractional part
             // note: for some reason, x/y isn't calculated directly,
             // instead, 1/y is calculated and then multiplied by x
-            // TODO: this is still not perfect (see for example x=169 y=33)
             if (ylen == 0)
                 Increment = 0;
             else if (ylen == xlen && xlen != 1)
@@ -315,21 +314,42 @@ private:
 
             dx += (y - y0) * Increment;
 
+            if (XMajor)
+            {
+                // used for calculating AA coverage
+                xcov_incr = (ylen << 10) / xlen;
+
+                if (side ^ Negative)
+                {
+                    dxold = dx;
+                    // I dont think the first span can have a gap but
+                    // I think its technically correct to do this calc anyway?
+                    // could probably be removed as a minor optimization
+                    dx &= ~0x1FF;
+                }
+            }
+
             s32 x = XVal();
 
             int interpoffset = (Increment >= 0x40000) && (side ^ Negative);
             Interp.Setup(y0-interpoffset, y1-interpoffset, w0, w1);
             Interp.SetX(y);
 
-            // used for calculating AA coverage
-            if (XMajor) xcov_incr = (ylen << 10) / xlen;
-
             return x;
         }
 
         constexpr s32 Step()
-        {
-            dx += Increment;
+        {   
+            if (XMajor && (side ^ Negative)) // tl, br = \ (right side end) & tr, bl = / (left side start)
+            {
+                // round dx; required to create gaps in lines like on hw
+                // increment using dxold to make the line not completely borked after rendering a gap
+                dx = (dxold & ~0x1FF) + Increment;
+                dxold += Increment;
+            }
+            else
+                dx += Increment;
+
             y++;
 
             s32 x = XVal();
@@ -355,15 +375,18 @@ private:
             // only needed for aa calcs, as actual line spans are broken
             if constexpr (!swapped || side)
             {
-                if (side ^ Negative)
-                    *length = (dx >> 18) - ((dx-Increment) >> 18);
-                else
-                    *length = ((dx+Increment) >> 18) - (dx >> 18);
+                if (side ^ Negative) // tr, bl = / (left side end) & tl br = \ (right side start)
+                // dxold used here to avoid rounding the endpoint
+                *length = (dx >> 18) - (dxold - Increment >> 18);
+                else // tl, br = \ (left side end) & tr, bl = / (right side start)
+                // dx rounded down to create gaps in lines like on hw
+                *length = ((dx & ~0x1FF) + Increment >> 18) - (dx >> 18);
             }
 
             // for X-major edges, we return the coverage
             // for the first pixel, and the increment for
             // further pixels on the same scanline
+            // TODO: check how coverage interacts with line gaps, I think it's correct though?
             s32 startx = dx >> 18;
             if (Negative) startx = xlen - startx;
             if (side)     startx = startx - *length + 1;
@@ -423,7 +446,7 @@ private:
     private:
         s32 x0, xmin, xmax;
         s32 xlen, ylen;
-        s32 dx;
+        s32 dx, dxold;
         s32 y;
 
         s32 xcov_incr;

src/frontend/qt_sdl/ROMManager.cpp

@@ -948,8 +948,8 @@ u32 DecompressROM(const u8* inContent, const u32 inSize, unique_ptr<u8[]>& outCo
 
     if (realSize != ZSTD_CONTENTSIZE_UNKNOWN)
     {
-        outContent = make_unique<u8[]>(realSize);
-        u64 decompressed = ZSTD_decompress(outContent.get(), realSize, inContent, inSize);
+        auto newOutContent = make_unique<u8[]>(realSize);
+        u64 decompressed = ZSTD_decompress(newOutContent.get(), realSize, inContent, inSize);
 
         if (ZSTD_isError(decompressed))
         {
@@ -957,6 +957,7 @@ u32 DecompressROM(const u8* inContent, const u32 inSize, unique_ptr<u8[]>& outCo
             return 0;
         }
 
+        outContent = std::move(newOutContent);
         return realSize;
     }
     else
@@ -1011,7 +1012,6 @@ u32 DecompressROM(const u8* inContent, const u32 inSize, unique_ptr<u8[]>& outCo
             }
         } while (inBuf.pos < inBuf.size);
 
-        ZSTD_freeDStream(dStream);
         outContent = make_unique<u8[]>(outBuf.pos);
         memcpy(outContent.get(), outBuf.dst, outBuf.pos);