Merge pull request #19359 from hrydgard/sanitizer-fixes

Fix some minor issues found by --sanitize. Add --sanitizeub.

Common/Buffer.cpp

@@ -22,7 +22,9 @@ char *Buffer::Append(size_t length) {
 
 void Buffer::Append(const std::string &str) {
 	char *ptr = Append(str.size());
-	memcpy(ptr, str.data(), str.size());
+	if (ptr) {
+		memcpy(ptr, str.data(), str.size());
+	}
 }
 
 void Buffer::Append(const char *str) {

Common/Data/Format/RIFF.cpp

@@ -21,11 +21,12 @@ RIFFReader::~RIFFReader() {
 }
 
 int RIFFReader::ReadInt() {
+	int value = 0;
 	if (data_ && pos_ < eof_ - 3) {
 		pos_ += 4;
-		return *(int *)(data_ + pos_ - 4);
+		memcpy(&value, data_ + pos_ - 4, 4);
 	}
-	return 0;
+	return value;
 }
 
 bool RIFFReader::Descend(uint32_t intoId) {
@@ -104,4 +105,3 @@ int RIFFReader::GetCurrentChunkSize() {
 	else
 		return 0;
 }
-

Core/Debugger/MemBlockInfo.cpp

@@ -280,7 +280,7 @@ void MemSlabMap::Clear() {
 			delete s;
 		s = next;
 	}
-	delete bulkStorage_;
+	delete [] bulkStorage_;
 	bulkStorage_ = nullptr;
 	first_ = nullptr;
 	lastFind_ = nullptr;

Core/MIPS/MIPSInt.cpp

@@ -42,6 +42,7 @@
 #define FsI(i) (currentMIPS->fs[i])
 #define PC (currentMIPS->pc)
 
+#define _SIMM16_SHL2 ((u32)(s32)(s16)(op & 0xFFFF) << 2)
 #define _RS   ((op>>21) & 0x1F)
 #define _RT   ((op>>16) & 0x1F)
 #define _RD   ((op>>11) & 0x1F)
@@ -94,7 +95,7 @@ namespace MIPSInt
 {
 	void Int_Cache(MIPSOpcode op)
 	{
-		int imm = SignExtend16ToS32(op & 0xFFFF);
+		int imm = SignExtend16ToS32(op);
 		int rs = _RS;
 		uint32_t addr = R(rs) + imm;
 		int func = (op >> 16) & 0x1F;
@@ -153,7 +154,7 @@ namespace MIPSInt
 	void Int_Syscall(MIPSOpcode op)
 	{
 		// Need to pre-move PC, as CallSyscall may result in a rescheduling!
-		// To do this neater, we'll need a little generated kernel loop that syscall can jump to and then RFI from 
+		// To do this neater, we'll need a little generated kernel loop that syscall can jump to and then RFI from
 		// but I don't see a need to bother.
 		if (mipsr4k.inDelaySlot)
 		{
@@ -182,12 +183,12 @@ namespace MIPSInt
 
 	void Int_RelBranch(MIPSOpcode op)
 	{
-		int imm = (signed short)(op&0xFFFF)<<2;
+		int imm = _SIMM16_SHL2;
 		int rs = _RS;
 		int rt = _RT;
 		u32 addr = PC + imm + 4;
 
-		switch (op >> 26) 
+		switch (op >> 26)
 		{
 		case 4:  if (R(rt) == R(rs))  DelayBranchTo(addr); else PC += 4; break; //beq
 		case 5:  if (R(rt) != R(rs))  DelayBranchTo(addr); else PC += 4; break; //bne
@@ -207,7 +208,7 @@ namespace MIPSInt
 
 	void Int_RelBranchRI(MIPSOpcode op)
 	{
-		int imm = (signed short)(op&0xFFFF)<<2;
+		int imm = _SIMM16_SHL2;
 		int rs = _RS;
 		u32 addr = PC + imm + 4;
 
@@ -230,7 +231,7 @@ namespace MIPSInt
 
 	void Int_VBranch(MIPSOpcode op)
 	{
-		int imm = (signed short)(op&0xFFFF)<<2;
+		int imm = _SIMM16_SHL2;
 		u32 addr = PC + imm + 4;
 
 		// x, y, z, w, any, all, (invalid), (invalid)
@@ -248,7 +249,7 @@ namespace MIPSInt
 
 	void Int_FPUBranch(MIPSOpcode op)
 	{
-		int imm = (signed short)(op&0xFFFF)<<2;
+		int imm = _SIMM16_SHL2;
 		u32 addr = PC + imm + 4;
 		switch((op>>16)&0x1f)
 		{
@@ -261,7 +262,7 @@ namespace MIPSInt
 			break;
 		}
 	}
-	
+
 	void Int_JumpType(MIPSOpcode op)
 	{
 		if (mipsr4k.inDelaySlot)
@@ -270,7 +271,7 @@ namespace MIPSInt
 		u32 off = ((op & 0x03FFFFFF) << 2);
 		u32 addr = (currentMIPS->pc & 0xF0000000) | off;
 
-		switch (op>>26) 
+		switch (op>>26)
 		{
 		case 2: //j
 			if (!mipsr4k.inDelaySlot)
@@ -298,7 +299,7 @@ namespace MIPSInt
 		int rs = _RS;
 		int rd = _RD;
 		u32 addr = R(rs);
-		switch (op & 0x3f) 
+		switch (op & 0x3f)
 		{
 		case 8: //jr
 			if (!mipsr4k.inDelaySlot)
@@ -328,7 +329,7 @@ namespace MIPSInt
 			return; //nop
 		}
 
-		switch (op>>26) 
+		switch (op>>26)
 		{
 		case 8:	R(rt) = R(rs) + simm; break; //addi
 		case 9:	R(rt) = R(rs) + simm; break;	//addiu
@@ -391,7 +392,7 @@ namespace MIPSInt
 			return;
 		}
 
-		switch (op & 63) 
+		switch (op & 63)
 		{
 		case 10: if (R(rt) == 0) R(rd) = R(rs); break; //movz
 		case 11: if (R(rt) != 0) R(rd) = R(rs); break; //movn
@@ -428,7 +429,7 @@ namespace MIPSInt
 			return;
 		}
 
-		switch (op >> 26) 
+		switch (op >> 26)
 		{
 		case 32: R(rt) = SignExtend8ToU32(Memory::Read_U8(addr)); break; //lb
 		case 33: R(rt) = SignExtend16ToU32(Memory::Read_U16(addr)); break; //lh
@@ -549,7 +550,7 @@ namespace MIPSInt
 				DEBUG_LOG(Log::CPU, "FCR%i written to, value %08x", fs, value);
 				break;
 			}
-		
+
 		default:
 			_dbg_assert_msg_(false,"Trying to interpret instruction that can't be interpreted");
 			break;
@@ -590,7 +591,7 @@ namespace MIPSInt
 		int rs = _RS;
 		int rd = _RD;
 
-		switch (op & 63) 
+		switch (op & 63)
 		{
 		case 24: //mult
 			{
@@ -702,16 +703,16 @@ namespace MIPSInt
 			PC += 4;
 			return;
 		}
-		
+
 		switch (op & 0x3f)
 		{
 		case 0: R(rd) = R(rt) << sa;					 break; //sll
-		case 2: 
+		case 2:
 			if (_RS == 0) //srl
 			{
 				R(rd) = R(rt) >> sa;
-				break; 
-			} 
+				break;
+			}
 			else if (_RS == 1) //rotr
 			{
 				R(rd) = __rotr(R(rt), sa);
@@ -726,7 +727,7 @@ namespace MIPSInt
 			if (_FD == 0) //srlv
 			{
 				R(rd) = R(rt) >> (R(rs)&0x1F);
-				break; 
+				break;
 			}
 			else if (_FD == 1) // rotrv
 			{

GPU/Common/VertexDecoderCommon.cpp

@@ -1374,8 +1374,9 @@ void VertexDecoder::DecodeVerts(u8 *decodedptr, const void *verts, const UVScale
 		prescaleUV_ = uvScaleOffset;
 		// Interpret the decode steps
 		for (; count; count--) {
-			for (int i = 0; i < numSteps_; i++) {
-				((*this).*steps_[i])();
+			const int steps = numSteps_;
+			for (int i = 0; i < steps; i++) {
+				(this->*steps_[i])();
 			}
 			ptr_ += size;
 			decoded_ += stride;

GPU/Common/VertexDecoderCommon.h

@@ -186,7 +186,7 @@ public:
 			Uint8x4ToFloat4(color, *(const u32 *)(data_ + decFmt_.c0off));
 			break;
 		case DEC_FLOAT_4:
-			memcpy(color, data_ + decFmt_.c0off, 16); 
+			memcpy(color, data_ + decFmt_.c0off, 16);
 			break;
 		default:
 			memset(color, 0, sizeof(float) * 4);
@@ -225,7 +225,7 @@ public:
 			}
 			break;
 		case DEC_FLOAT_4:
-			memcpy(color, data_ + decFmt_.c1off, 12); 
+			memcpy(color, data_ + decFmt_.c1off, 12);
 			break;
 		default:
 			memset(color, 0, sizeof(float) * 3);
@@ -445,8 +445,8 @@ public:
 
 	// "Immutable" state, set at startup
 
-	// The decoding steps. Never more than 5.
-	StepFunction steps_[5];
+	// The decoding steps. Never more than 5 (weight, texcoord, color, normal, pos)
+	StepFunction steps_[5]{};
 	int numSteps_;
 
 	u32 fmt_;

UI/ControlMappingScreen.cpp

@@ -247,6 +247,7 @@ static const BindingCategory cats[] = {
 	{"Control modifiers", VIRTKEY_ANALOG_ROTATE_CW},
 	{"Emulator controls", VIRTKEY_FASTFORWARD},
 	{"Extended PSP controls", VIRTKEY_AXIS_RIGHT_Y_MAX},
+	{},  // sentinel
 };
 
 void ControlMappingScreen::CreateViews() {
@@ -313,7 +314,7 @@ void ControlMappingScreen::CreateViews() {
 	if (curCat >= 0 && curSection) {
 		curSection->SetOpenPtr(&categoryToggles_[curCat]);
 	}
-	_dbg_assert_(curCat == ARRAY_SIZE(cats) - 1);
+	_dbg_assert_(curCat == ARRAY_SIZE(cats) - 2);  // count the sentinel
 
 	keyMapGeneration_ = KeyMap::g_controllerMapGeneration;
 }

b.sh

@@ -72,6 +72,9 @@ do
 		--sanitize) echo "Enabling address-sanitizer if available"
 			CMAKE_ARGS="-DUSE_ASAN=ON ${CMAKE_ARGS}"
 			;;
+		--sanitizeub) echo "Enabling ub-sanitizer if available"
+			CMAKE_ARGS="-DUSE_UBSAN=ON ${CMAKE_ARGS}"
+			;;
 		--gold) echo "Gold build enabled"
 			CMAKE_ARGS="-DGOLD=ON ${CMAKE_ARGS}"
 			;;