From 2eefc135e533a3a3fe08e00e76acf13be95e4678 Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Sat, 26 Sep 2015 13:53:20 +0200 Subject: [PATCH 1/2] pcsx2: forbid negative index of array in case of register allocation failure CID 146870 (#1 of 1): Negative array index write (NEGATIVE_RETURNS) 5. negative_returns: Using variable xmmreg as an index to array ... Open discussion: how to handle correctly bad register allocation? Currently negative index is returned and a message printed. It means we need to propagate the index check everywhere in order to not use it. I suspect that Instruction Generation is more or less corrupted so potentially we could just fire an exception. --- pcsx2/x86/iCore.cpp | 16 ++++++++++++++-- pcsx2/x86/ix86-32/iCore-32.cpp | 4 ++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/pcsx2/x86/iCore.cpp b/pcsx2/x86/iCore.cpp index 8b6816026b..eb3b44a1a9 100644 --- a/pcsx2/x86/iCore.cpp +++ b/pcsx2/x86/iCore.cpp @@ -144,6 +144,9 @@ int _allocTempXMMreg(XMMSSEType type, int xmmreg) { else _freeXMMreg(xmmreg); + if (xmmreg == -1) + return -1; + xmmregs[xmmreg].inuse = 1; xmmregs[xmmreg].type = XMMTYPE_TEMP; xmmregs[xmmreg].needed = 1; @@ -191,6 +194,9 @@ int _allocVFtoXMMreg(VURegs *VU, int xmmreg, int vfreg, int mode) { else _freeXMMreg(xmmreg); + if (xmmreg == -1) + return -1; + g_xmmtypes[xmmreg] = XMMT_FPS; xmmregs[xmmreg].inuse = 1; xmmregs[xmmreg].type = XMMTYPE_VFREG; @@ -273,6 +279,9 @@ int _allocACCtoXMMreg(VURegs *VU, int xmmreg, int mode) { else _freeXMMreg(xmmreg); + if (xmmreg == -1) + return -1; + g_xmmtypes[xmmreg] = XMMT_FPS; xmmregs[xmmreg].inuse = 1; xmmregs[xmmreg].type = XMMTYPE_ACC; @@ -314,6 +323,7 @@ int _allocFPtoXMMreg(int xmmreg, int fpreg, int mode) { } if (xmmreg == -1) xmmreg = _getFreeXMMreg(); + if (xmmreg == -1) return -1; g_xmmtypes[xmmreg] = XMMT_FPS; xmmregs[xmmreg].inuse = 1; @@ -379,6 +389,7 @@ int _allocGPRtoXMMreg(int xmmreg, int gprreg, int mode) } if (xmmreg == -1) xmmreg = _getFreeXMMreg(); + if (xmmreg == -1) return -1; g_xmmtypes[xmmreg] = XMMT_INT; xmmregs[xmmreg].inuse = 1; @@ -455,9 +466,10 @@ int _allocFPACCtoXMMreg(int xmmreg, int mode) return i; } - if (xmmreg == -1) { + if (xmmreg == -1) xmmreg = _getFreeXMMreg(); - } + if (xmmreg == -1) + return -1; g_xmmtypes[xmmreg] = XMMT_FPS; xmmregs[xmmreg].inuse = 1; diff --git a/pcsx2/x86/ix86-32/iCore-32.cpp b/pcsx2/x86/ix86-32/iCore-32.cpp index 2170b53324..fbb44d5fa7 100644 --- a/pcsx2/x86/ix86-32/iCore-32.cpp +++ b/pcsx2/x86/ix86-32/iCore-32.cpp @@ -311,6 +311,9 @@ int _allocX86reg(int x86reg, int type, int reg, int mode) else _freeX86reg(x86reg); + if (x86reg == -1) + return -1; + x86regs[x86reg].type = type; x86regs[x86reg].reg = reg; x86regs[x86reg].mode = mode; @@ -584,6 +587,7 @@ int _allocMMXreg(int mmxreg, int reg, int mode) } if (mmxreg == -1) mmxreg = _getFreeMMXreg(); + if (mmxreg == -1) return -1; mmxregs[mmxreg].inuse = 1; mmxregs[mmxreg].reg = reg; From 63889d3bea8c65e4a4fbd63d93ed30514eb18d0f Mon Sep 17 00:00:00 2001 From: Gregory Hainaut Date: Mon, 12 Oct 2015 21:38:27 +0200 Subject: [PATCH 2/2] pcsx2: replace error message with a dev assert Code mustn't be reached. Otherwise registers aren't properly freed --- pcsx2/x86/iCore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pcsx2/x86/iCore.cpp b/pcsx2/x86/iCore.cpp index eb3b44a1a9..7df76a2471 100644 --- a/pcsx2/x86/iCore.cpp +++ b/pcsx2/x86/iCore.cpp @@ -133,7 +133,7 @@ int _getFreeXMMreg() _freeXMMreg(tempi); return tempi; } - Console.Error("*PCSX2*: XMM Reg Allocation Error in _getFreeXMMreg()!"); + pxFailDev("*PCSX2*: XMM Reg Allocation Error in _getFreeXMMreg()!"); return -1; }