A bit more GetPointer cleanup.

Probably not worth it for performance reasons, but some semantic cleanup
is good, especially the accidental GetPointer -> writable casts without
using GetPointerWrite.

Using Unchecked on already checked pointers, or when we'd crash anyway
if it returned nullptr, is good for clarity.
This commit is contained in:
Henrik Rydgård 2023-01-01 21:12:04 +01:00
parent 70e149e3ca
commit e1a48d74c4
11 changed files with 42 additions and 26 deletions

View File

@ -728,7 +728,7 @@ static u32 sysclib_strcpy(u32 dst, u32 src) {
static u32 sysclib_strlen(u32 src) {
ERROR_LOG(SCEKERNEL, "Untested sysclib_strlen(src=%08x)", src);
if (Memory::IsValidAddress(src)) {
return (u32)strlen(Memory::GetCharPointer(src));
return (u32)strlen(Memory::GetCharPointerUnchecked(src));
} else {
// What to do? Crash, probably.
return 0;
@ -738,7 +738,7 @@ static u32 sysclib_strlen(u32 src) {
static int sysclib_memcmp(u32 dst, u32 src, u32 size) {
ERROR_LOG(SCEKERNEL, "Untested sysclib_memcmp(dest=%08x, src=%08x, size=%i)", dst, src, size);
if (Memory::IsValidRange(dst, size) && Memory::IsValidRange(src, size)) {
return memcmp(Memory::GetCharPointer(dst), Memory::GetCharPointer(src), size);
return memcmp(Memory::GetCharPointerUnchecked(dst), Memory::GetCharPointerUnchecked(src), size);
} else {
// What to do? Crash, probably.
return 0;
@ -749,7 +749,7 @@ static int sysclib_sprintf(u32 dst, u32 fmt) {
ERROR_LOG(SCEKERNEL, "Unimpl sysclib_sprintf(dest=%08x, src=%08x)", dst, fmt);
if (Memory::IsValidAddress(dst) && Memory::IsValidAddress(fmt)) {
// TODO: Properly use the format string with more parameters.
return sprintf((char *)Memory::GetPointer(dst), "%s", Memory::GetCharPointer(fmt));
return sprintf((char *)Memory::GetPointerUnchecked(dst), "%s", Memory::GetCharPointerUnchecked(fmt));
} else {
// What to do? Crash, probably.
return 0;
@ -768,8 +768,8 @@ static u32 sysclib_memset(u32 destAddr, int data, int size) {
static int sysclib_strstr(u32 s1, u32 s2) {
ERROR_LOG(SCEKERNEL, "Untested sysclib_strstr(%08x, %08x)", s1, s2);
if (Memory::IsValidAddress(s1) && Memory::IsValidAddress(s2)) {
std::string str1 = Memory::GetCharPointer(s1);
std::string str2 = Memory::GetCharPointer(s2);
std::string str1 = Memory::GetCharPointerUnchecked(s1);
std::string str2 = Memory::GetCharPointerUnchecked(s2);
size_t index = str1.find(str2);
if (index == str1.npos) {
return 0;
@ -782,8 +782,8 @@ static int sysclib_strstr(u32 s1, u32 s2) {
static int sysclib_strncmp(u32 s1, u32 s2, u32 size) {
ERROR_LOG(SCEKERNEL, "Untested sysclib_strncmp(%08x, %08x, %08x)", s1, s2, size);
if (Memory::IsValidAddress(s1) && Memory::IsValidAddress(s2)) {
const char * str1 = Memory::GetCharPointer(s1);
const char * str2 = Memory::GetCharPointer(s2);
const char * str1 = Memory::GetCharPointerUnchecked(s1);
const char * str2 = Memory::GetCharPointerUnchecked(s2);
return strncmp(str1, str2, size);
}
return 0;
@ -810,8 +810,8 @@ static u32 sysclib_strncpy(u32 dest, u32 src, u32 size) {
// This is just regular strncpy, but being explicit to avoid warnings/safety fixes on missing null.
u32 i = 0;
u32 srcSize = Memory::ValidSize(src, size);
const u8 *srcp = Memory::GetPointer(src);
u8 *destp = Memory::GetPointerWrite(dest);
const u8 *srcp = Memory::GetPointerUnchecked(src);
u8 *destp = Memory::GetPointerWriteUnchecked(dest);
for (i = 0; i < srcSize; ++i) {
u8 c = *srcp++;
if (c == 0)

View File

@ -1042,7 +1042,7 @@ static bool KernelImportModuleFuncs(PSPModule *module, u32 *firstImportStubAddr,
strncpy(func.moduleName, modulename, KERNELOBJECT_MAX_NAME_LENGTH);
func.moduleName[KERNELOBJECT_MAX_NAME_LENGTH] = '\0';
u32_le *nidDataPtr = (u32_le *)Memory::GetPointer(entry->nidData);
u32_le *nidDataPtr = (u32_le *)Memory::GetPointerUnchecked(entry->nidData);
for (int i = 0; i < entry->numFuncs; ++i) {
// This is the id of the import.
func.nid = nidDataPtr[i];
@ -1080,7 +1080,7 @@ static bool KernelImportModuleFuncs(PSPModule *module, u32 *firstImportStubAddr,
}
WriteVarSymbolState state;
u32_le *varRef = (u32_le *)Memory::GetPointer(varRefsPtr);
u32_le *varRef = (u32_le *)Memory::GetPointerUnchecked(varRefsPtr);
for (; *varRef != 0; ++varRef) {
var.nid = nid;
var.stubAddr = (*varRef & 0x03FFFFFF) << 2;
@ -1106,7 +1106,7 @@ static bool KernelImportModuleFuncs(PSPModule *module, u32 *firstImportStubAddr,
char temp[512];
const char *modulename;
if (Memory::IsValidAddress(entry->name)) {
modulename = Memory::GetCharPointer(entry->name);
modulename = Memory::GetCharPointerUnchecked(entry->name);
} else {
modulename = "(invalidname)";
}
@ -1316,7 +1316,7 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load
return nullptr;
}
modinfo = (PspModuleInfo *)Memory::GetPointer(modinfoaddr);
modinfo = (PspModuleInfo *)Memory::GetPointerUnchecked(modinfoaddr);
module->nm.nsegment = reader.GetNumSegments();
module->nm.attribute = modinfo->moduleAttrs;
@ -1498,7 +1498,7 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load
continue;
}
u32_le *residentPtr = (u32_le *)Memory::GetPointer(ent->resident);
u32_le *residentPtr = (u32_le *)Memory::GetPointerUnchecked(ent->resident);
u32_le *exportPtr = residentPtr + ent->fcount + variableCount;
if (ent->size != 4 && ent->unknown1 != 0 && ent->unknown2 != 0) {

View File

@ -2175,6 +2175,7 @@ static u32 convertABGRToYCbCr(u32 abgr) {
return (y << 16) | (cb << 8) | cr;
}
// bufferOutputAddr is checked in the caller.
static int __MpegAvcConvertToYuv420(const void *data, u32 bufferOutputAddr, int width, int height) {
u32 *imageBuffer = (u32*)data;
int sizeY = width * height;

View File

@ -48,11 +48,11 @@ static u32 sceP3daBridgeCore(u32 p3daCoreAddr, u32 channelsNum, u32 samplesNum,
DEBUG_LOG(SCEAUDIO, "sceP3daBridgeCore(%08x, %08x, %08x, %08x, %08x)", p3daCoreAddr, channelsNum, samplesNum, inputAddr, outputAddr);
if (Memory::IsValidAddress(inputAddr) && Memory::IsValidAddress(outputAddr)) {
int scaleval = getScaleValue(channelsNum);
s16_le *outbuf = (s16_le *)Memory::GetPointer(outputAddr);
s16_le *outbuf = (s16_le *)Memory::GetPointerWriteUnchecked(outputAddr);
memset(outbuf, 0, samplesNum * sizeof(s16) * 2);
for (u32 k = 0; k < channelsNum; k++) {
u32 inaddr = Memory::Read_U32(inputAddr + k * 4);
const s16 *inbuf = (const s16 *)Memory::GetPointer(inaddr);
const s16 *inbuf = (const s16 *)Memory::GetPointerUnchecked(inaddr);
if (!inbuf)
continue;
for (u32 i = 0; i < samplesNum; i++) {

View File

@ -879,6 +879,7 @@ static int sceRtcSetAlarmTick(u32 unknown1, u32 unknown2)
return 0;
}
// Caller must check outPtr and srcTickPtr.
static int __RtcFormatRFC2822(u32 outPtr, u32 srcTickPtr, int tz)
{
u64 srcTick = Memory::Read_U64(srcTickPtr);
@ -897,7 +898,7 @@ static int __RtcFormatRFC2822(u32 outPtr, u32 srcTickPtr, int tz)
local.tm_min += tz;
rtc_timegm(&local);
char *out = (char *)Memory::GetPointer(outPtr);
char *out = (char *)Memory::GetPointerWriteUnchecked(outPtr);
char *end = out + 32;
out += strftime(out, end - out, "%a, %d %b ", &local);
out += snprintf(out, end - out, "%04d", pt.year);
@ -928,7 +929,7 @@ static int __RtcFormatRFC3339(u32 outPtr, u32 srcTickPtr, int tz)
local.tm_min += tz;
rtc_timegm(&local);
char *out = (char *)Memory::GetPointer(outPtr);
char *out = (char *)Memory::GetPointerWriteUnchecked(outPtr);
char *end = out + 32;
out += snprintf(out, end - out, "%04d", pt.year);
out += strftime(out, end - out, "-%m-%dT%H:%M:%S.00", &local);

View File

@ -816,15 +816,20 @@ static u32 sceUtilitySetSystemParamString(u32 id, u32 strPtr)
return 0;
}
static u32 sceUtilityGetSystemParamString(u32 id, u32 destaddr, int destSize)
static u32 sceUtilityGetSystemParamString(u32 id, u32 destAddr, int destSize)
{
DEBUG_LOG(SCEUTILITY, "sceUtilityGetSystemParamString(%i, %08x, %i)", id, destaddr, destSize);
char *buf = (char *)Memory::GetPointer(destaddr);
if (!Memory::IsValidRange(destAddr, destSize)) {
// TODO: What error code?
return -1;
}
DEBUG_LOG(SCEUTILITY, "sceUtilityGetSystemParamString(%i, %08x, %i)", id, destAddr, destSize);
char *buf = (char *)Memory::GetPointerWriteUnchecked(destAddr);
switch (id) {
case PSP_SYSTEMPARAM_ID_STRING_NICKNAME:
// If there's not enough space for the string and null terminator, fail.
if (destSize <= (int)g_Config.sNickName.length())
return PSP_SYSTEMPARAM_RETVAL_STRING_TOO_LONG;
// TODO: should we zero-pad the output as strncpy does? And what are the semantics for the terminating null if destSize == length?
strncpy(buf, g_Config.sNickName.c_str(), destSize);
break;

View File

@ -65,7 +65,12 @@ static uint64_t HashJitBlock(const JitBlock &b) {
PROFILE_THIS_SCOPE("jithash");
if (JIT_USE_COMPILEDHASH) {
// Includes the emuhack (or emuhacks) in memory.
return XXH3_64bits(Memory::GetPointerRange(b.originalAddress, b.originalSize * 4), b.originalSize * 4);
if (Memory::IsValidRange(b.originalAddress, b.originalSize * 4)) {
return XXH3_64bits(Memory::GetPointerUnchecked(b.originalAddress), b.originalSize * 4);
} else {
// Hm, this would be bad.
return 0;
}
}
return 0;
}

View File

@ -280,6 +280,10 @@ inline const char* GetCharPointer(const u32 address) {
}
}
inline const char *GetCharPointerUnchecked(const u32 address) {
return (const char *)GetPointerUnchecked(address);
}
inline void MemcpyUnchecked(void *to_data, const u32 from_address, const u32 len) {
memcpy(to_data, GetPointerUnchecked(from_address), len);
}

View File

@ -402,7 +402,7 @@ u32 TextureReplacer::ComputeHash(u32 addr, int bufw, int w, int h, GETextureForm
}
}
const u8 *checkp = Memory::GetPointer(addr);
const u8 *checkp = Memory::GetPointerUnchecked(addr);
if (reduceHash_) {
reduceHashSize = LookupReduceHashRange(w, h);
// default to reduceHashGlobalValue which default is 0.5

View File

@ -422,7 +422,7 @@ bool DrawEngineCommon::GetCurrentSimpleVertices(int count, std::vector<GPUDebugV
static std::vector<SimpleVertex> simpleVertices;
temp_buffer.resize(std::max((int)indexUpperBound, 8192) * 128 / sizeof(u32));
simpleVertices.resize(indexUpperBound + 1);
NormalizeVertices((u8 *)(&simpleVertices[0]), (u8 *)(&temp_buffer[0]), Memory::GetPointer(gstate_c.vertexAddr), indexLowerBound, indexUpperBound, gstate.vertType);
NormalizeVertices((u8 *)(&simpleVertices[0]), (u8 *)(&temp_buffer[0]), Memory::GetPointerUnchecked(gstate_c.vertexAddr), indexLowerBound, indexUpperBound, gstate.vertType);
float world[16];
float view[16];

View File

@ -1103,7 +1103,7 @@ void FramebufferManagerCommon::UpdateFromMemory(u32 addr, int size) {
// TODO: This doesn't seem quite right anymore.
fmt = displayFormat_;
}
DrawPixels(vfb, 0, 0, Memory::GetPointer(addr), fmt, vfb->fb_stride, vfb->width, vfb->height, RASTER_COLOR, "UpdateFromMemory_DrawPixels");
DrawPixels(vfb, 0, 0, Memory::GetPointerUnchecked(addr), fmt, vfb->fb_stride, vfb->width, vfb->height, RASTER_COLOR, "UpdateFromMemory_DrawPixels");
SetColorUpdated(vfb, gstate_c.skipDrawReason);
} else {
INFO_LOG(FRAMEBUF, "Invalidating FBO for %08x (%dx%d %s)", vfb->fb_address, vfb->width, vfb->height, GeBufferFormatToString(vfb->fb_format));
@ -1474,7 +1474,7 @@ void FramebufferManagerCommon::CopyDisplayToOutput(bool reallyDirty) {
if (Memory::IsValidAddress(fbaddr)) {
// The game is displaying something directly from RAM. In GTA, it's decoded video.
if (!vfb) {
DrawFramebufferToOutput(Memory::GetPointer(fbaddr), displayStride_, displayFormat_);
DrawFramebufferToOutput(Memory::GetPointerUnchecked(fbaddr), displayStride_, displayFormat_);
return;
}
} else {