Add range checks in replaced memcpy/memset functions. (#16693)

* Add range checks in replaced memcpy/memset functions.

Keep seeing especially Replace_memcpy as a semi-rare crash in the
reports. Hopefully this will take care of it, though if games hit this,
they're probably on their way to failing somehow anyway.

* Alternate approach, correctly causing memory exceptions if not ignoring
This commit is contained in:
Henrik Rydgård 2023-01-01 19:25:40 +01:00 committed by GitHub
parent 5a71db8808
commit a4d3e0ead8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 10 deletions

View File

@ -138,8 +138,8 @@ static int Replace_memcpy() {
}
}
if (!skip && bytes != 0) {
u8 *dst = Memory::GetPointerWrite(destPtr);
const u8 *src = Memory::GetPointer(srcPtr);
u8 *dst = Memory::GetPointerWriteRange(destPtr, bytes);
const u8 *src = Memory::GetPointerRange(srcPtr, bytes);
if (!dst || !src) {
// Already logged.
@ -190,8 +190,8 @@ static int Replace_memcpy_jak() {
}
}
if (!skip && bytes != 0) {
u8 *dst = Memory::GetPointerWrite(destPtr);
const u8 *src = Memory::GetPointer(srcPtr);
u8 *dst = Memory::GetPointerWriteRange(destPtr, bytes);
const u8 *src = Memory::GetPointerRange(srcPtr, bytes);
if (!dst || !src) {
} else {
@ -241,8 +241,8 @@ static int Replace_memcpy16() {
}
}
if (!skip && bytes != 0) {
u8 *dst = Memory::GetPointerWrite(destPtr);
const u8 *src = Memory::GetPointer(srcPtr);
u8 *dst = Memory::GetPointerWriteRange(destPtr, bytes);
const u8 *src = Memory::GetPointerRange(srcPtr, bytes);
if (dst && src) {
memmove(dst, src, bytes);
}
@ -313,8 +313,8 @@ static int Replace_memmove() {
}
}
if (!skip && bytes != 0) {
u8 *dst = Memory::GetPointerWrite(destPtr);
const u8 *src = Memory::GetPointer(srcPtr);
u8 *dst = Memory::GetPointerWriteRange(destPtr, bytes);
const u8 *src = Memory::GetPointerRange(srcPtr, bytes);
if (dst && src) {
memmove(dst, src, bytes);
}
@ -339,7 +339,7 @@ static int Replace_memset() {
skip = gpu->PerformMemorySet(destPtr, value, bytes);
}
if (!skip && bytes != 0) {
u8 *dst = Memory::GetPointerWrite(destPtr);
u8 *dst = Memory::GetPointerWriteRange(destPtr, bytes);
if (dst) {
memset(dst, value, bytes);
}
@ -366,7 +366,7 @@ static int Replace_memset_jak() {
skip = gpu->PerformMemorySet(destPtr, value, bytes);
}
if (!skip && bytes != 0) {
u8 *dst = Memory::GetPointerWrite(destPtr);
u8 *dst = Memory::GetPointerWriteRange(destPtr, bytes);
if (dst) {
memset(dst, value, bytes);
}

View File

@ -246,6 +246,10 @@ inline void Write_Float(float f, u32 address)
u8* GetPointerWrite(const u32 address);
const u8* GetPointer(const u32 address);
u8 *GetPointerWriteRange(const u32 address, const u32 size);
const u8 *GetPointerRange(const u32 address, const u32 size);
bool IsRAMAddress(const u32 address);
inline bool IsVRAMAddress(const u32 address) {
return ((address & 0x3F800000) == 0x04000000);

View File

@ -62,6 +62,38 @@ const u8 *GetPointer(const u32 address) {
}
}
u8 *GetPointerWriteRange(const u32 address, const u32 size) {
u8 *ptr = GetPointerWrite(address);
if (ptr) {
if (ValidSize(address, size) != size) {
// That's a memory exception! TODO: Adjust reported address to the end of the range?
Core_MemoryException(address, currentMIPS->pc, MemoryExceptionType::WRITE_BLOCK);
return nullptr;
} else {
return ptr;
}
} else {
// Error was reported in GetPointerWrite already, if we're not ignoring errors.
return nullptr;
}
}
const u8 *GetPointerRange(const u32 address, const u32 size) {
const u8 *ptr = GetPointer(address);
if (ptr) {
if (ValidSize(address, size) != size) {
// That's a memory exception! TODO: Adjust reported address to the end of the range?
Core_MemoryException(address, currentMIPS->pc, MemoryExceptionType::READ_BLOCK);
return nullptr;
} else {
return ptr;
}
} else {
// Error was reported in GetPointer already, if we're not ignoring errors.
return nullptr;
}
}
template <typename T>
inline void ReadFromHardware(T &var, const u32 address) {
// TODO: Figure out the fastest order of tests for both read and write (they are probably different).