RSX Bugfixes and plugging memory leaks

BUGFIX: Add break after NV4097_SET_TEXTURE_BORDER_COLOR in RSXThread.cpp
BUGFIX: Fix parameters passed to RSXTexture::SetControl3 (they were being
passed in reverse order)
BUGFIX: Remove invalid, non-sensical call to glPixelStorei in GLGSRender.h
BUGFIX: Fix signed/unsigned comparison compiler warnings in GLGSRender.h
CHANGE: Make GLFragmentProgram::Decompiler synchronous by default
CHANGE: Update wxWidgets submodule to latest commit
BUGFIX: Fix several memory leaks
ADDED: Created a new MSVC debug configuration to output locations
  of allocations that end up leaking after the program
  is closed.
BUGFIX: Fix the stupid PadHandler crash due to the lack of a virtual d'tor

Utilities/IdManager.h

@@ -50,6 +50,14 @@ struct ID
 	{
 	}
 
+	ID(ID&& other)
+	{
+		m_name = other.m_name;
+		m_attr = other.m_attr;
+		m_data = other.m_data;
+		other.m_data = nullptr;
+	}
+
 	void Kill()
 	{
 		delete m_data;

Utilities/Thread.cpp

@@ -35,6 +35,8 @@ ThreadBase::~ThreadBase()
 {
 	if(IsAlive())
 		Stop(false);
+
+	safe_delete(m_executor);
 }
 
 void ThreadBase::Start()

rpcs3/Emu/CPU/CPUDecoder.h

@@ -7,15 +7,15 @@ class CPUDecoder
 {
 public:
 	virtual u8 DecodeMemory(const u64 address)=0;
+
+	virtual ~CPUDecoder() = default;
 };
 
 template<typename TO>
 class InstrCaller
 {
 public:
-	virtual ~InstrCaller<TO>()
-	{
-	}
+	virtual ~InstrCaller<TO>() = default;
 
 	virtual void operator ()(TO* op, u32 code) const = 0;
 
@@ -334,6 +334,17 @@ class InstrBase : public InstrCaller<TO>
 				});
 	}
 
+	virtual ~InstrBase()
+	{
+		if (m_args) {
+			// m_args contains pointers to statically allocated CodeFieldBase objects
+			// We shouldn't call delete on these, they aren't allocated with new
+
+			// The m_args array itself, however, should be deleted
+			delete[] m_args;
+		}
+	}
+
 	__forceinline const std::string& GetName() const
 	{
 		return m_name;
@@ -396,12 +407,54 @@ class InstrList : public InstrCaller<TO>
 
 	virtual ~InstrList()
 	{
-		for(int i=0; i<count; ++i)
+		bool deletedErrorFunc = false;
+
+		// Clean up m_instrs
+		for(int i = 0; i < count; ++i)
 		{
-			delete m_instrs[i];
+			InstrCaller<TO>* deleteMe = m_instrs[i];
+
+			if (deleteMe) { // deleteMe will be a nullptr if we've already deleted it through another reference
+				// Remove any instances of pointers to this instruction caller from our m_instrs list
+				m_instrs[i] = nullptr;
+				for (int j = i + 1; j < count; j++) {
+					if (m_instrs[j] == deleteMe) {
+						m_instrs[j] = nullptr;
+					}
+				}
+
+				// If we're deleting the error handler here, remember it so we don't try to delete it again later
+				if (deleteMe == m_error_func) {
+					deletedErrorFunc = true;
+				}
+
+				// Delete the instruction caller
+				delete deleteMe;
+			}
 		}
 
-		delete m_error_func;
+		// Clean up m_instrs_info
+		for (int i = 0; i < count; ++i)
+		{
+			InstrBase<TO>* deleteMe = m_instrs_info[i];
+
+			if (deleteMe) {
+				m_instrs_info[i] = nullptr;
+				for (int j = i + 1; j < count; j++) {
+					if (m_instrs_info[j] == deleteMe) {
+						m_instrs[j] = nullptr;
+					}
+				}
+
+				delete deleteMe;
+			}
+		}
+
+		// If we haven't already deleted our error handler, and we have one, then delete it now
+		if (!deletedErrorFunc && m_error_func)
+		{
+			delete m_error_func;
+		}
 	}
 
 	void set_parent(InstrCaller<TO>* parent, int opcode)

rpcs3/Emu/CPU/CPUThread.cpp

@@ -26,6 +26,7 @@ CPUThread::CPUThread(CPUThreadType type)
 
 CPUThread::~CPUThread()
 {
+	safe_delete(m_dec);
 }
 
 void CPUThread::Close()

rpcs3/Emu/CPU/CPUThreadManager.cpp

@@ -46,31 +46,38 @@ CPUThread& CPUThreadManager::AddThread(CPUThreadType type)
 	return *new_thread;
 }
 
-//TODO: find out where the thread is actually deleted because it's sure as shit not here
 void CPUThreadManager::RemoveThread(const u32 id)
 {
 	std::lock_guard<std::mutex> lock(m_mtx_thread);
 
-	for(u32 i=0; i<m_threads.size(); ++i)
+	CPUThread* thr = nullptr;
+	u32 thread_index = 0;
+
+	for (u32 i = 0; i < m_threads.size(); ++i)
 	{
-		if(m_threads[i]->m_wait_thread_id == id)
+		if (m_threads[i]->m_wait_thread_id == id)
 		{
 			m_threads[i]->Wait(false);
 			m_threads[i]->m_wait_thread_id = -1;
 		}
 
-		if(m_threads[i]->GetId() != id) continue;
+		if (m_threads[i]->GetId() != id) continue;
 
-		CPUThread* thr = m_threads[i];
+		thr = m_threads[i];
+		thread_index = i;
+	}
+
+	if (thr)
+	{
 #ifndef QT_UI
 		wxGetApp().SendDbgCommand(DID_REMOVE_THREAD, thr);
 #endif
 		thr->Close();
 
-		m_threads.erase(m_threads.begin()+ i);
-		break;
+		m_threads.erase(m_threads.begin() + thread_index);
 	}
 
+	// Removing the ID should trigger the actual deletion of the thread
 	Emu.GetIdManager().RemoveID(id);
 	Emu.CheckStatus();
 }

rpcs3/Emu/Cell/PPCDecoder.h

@@ -8,6 +8,8 @@ class PPCDecoder : public CPUDecoder
 	virtual void Decode(const u32 code)=0;
 
 	virtual u8 DecodeMemory(const u64 address);
+
+	virtual ~PPCDecoder() = default;
 };
 
 

rpcs3/Emu/Cell/PPUDecoder.h

@@ -9,11 +9,11 @@ class PPUDecoder : public PPCDecoder
 	PPUOpcodes* m_op;
 
 public:
-	PPUDecoder(PPUOpcodes& op) : m_op(&op)
+	PPUDecoder(PPUOpcodes* op) : m_op(op)
 	{
 	}
 
-	~PPUDecoder()
+	virtual ~PPUDecoder()
 	{
 		delete m_op;
 	}

rpcs3/Emu/Cell/PPUInstrTable.h

@@ -203,7 +203,9 @@ namespace PPU_instr
 
 	static CodeField<9, 10> STRM;
 
-	static auto main_list = new_list(OPCD, instr_bind(&PPUOpcodes::UNK, GetCode, OPCD, OPCD));
+	//static auto main_list = new_list(OPCD, instr_bind(&PPUOpcodes::UNK, GetCode, OPCD, OPCD));
+	static InstrList<1 << CodeField<0, 5>::size, ::PPUOpcodes> main_list_obj(OPCD, instr_bind(&PPUOpcodes::UNK, GetCode, OPCD, OPCD));
+	static auto main_list = &main_list_obj;
 	static auto g04_list = new_list(main_list, PPU_opcodes::G_04, GD_04);
 	static auto g04_0_list = new_list(g04_list, GD_04_0, instr_bind(&PPUOpcodes::UNK, GetCode, OPCD, GD_04_0));
 	static auto g13_list = new_list(main_list, PPU_opcodes::G_13, GD_13, instr_bind(&PPUOpcodes::UNK, GetCode, OPCD, GD_13));

rpcs3/Emu/Cell/PPUThread.cpp

@@ -150,7 +150,8 @@ void PPUThread::DoRun()
 
 	case 1:
 	case 2:
-		m_dec = new PPUDecoder(*new PPUInterpreter(*this));
+		auto ppui = new PPUInterpreter(*this);
+		m_dec = new PPUDecoder(ppui);
 	break;
 	}
 }

rpcs3/Emu/Cell/SPUInstrTable.h

@@ -39,7 +39,9 @@ namespace SPU_instr
 	static CodeField<18, 31> L_18_31;
 	static CodeField<11> L_11;
 
-	static auto rrr_list = new_list<SPUOpcodes>(RRR);
+	// static auto rrr_list = new_list<SPUOpcodes>(RRR);
+	static InstrList<1 << CodeField<0, 3>::size, SPUOpcodes> rrr_list_obj(RRR, nullptr);
+	static auto rrr_list = &rrr_list_obj;
 	static auto ri18_list = new_list(rrr_list, RI18);
 	static auto ri10_list = new_list(ri18_list, RI10);
 	static auto ri16_list = new_list(ri10_list, RI16);

rpcs3/Emu/Cell/SPUThread.h

@@ -1180,7 +1180,7 @@ class SPUThread : public PPCThread
 					reg_value0 = std::stoull(value.substr(16, 31), 0, 16);
 					reg_value1 = std::stoull(value.substr(0, 15), 0, 16);
 				}
-				catch (std::invalid_argument& e)
+				catch (std::invalid_argument& /*e*/)
 				{
 					return false;
 				}

rpcs3/Emu/DbgConsole.cpp

@@ -26,6 +26,11 @@ DbgConsole::~DbgConsole()
 {
 	ThreadBase::Stop();
 	m_dbg_buffer.Flush();
+
+	safe_delete(m_console);
+	safe_delete(m_color_white);
+	safe_delete(m_color_red);
+	safe_delete(m_output);
 }
 
 void DbgConsole::Write(int ch, const std::string& text)

rpcs3/Emu/FS/VFS.cpp

@@ -14,6 +14,11 @@ int sort_devices(const void* _a, const void* _b)
 	return 0;
 }
 
+VFS::~VFS()
+{
+	UnMountAll();
+}
+
 void VFS::Mount(const std::string& ps3_path, const std::string& local_path, vfsDevice* device)
 {
 	UnMount(ps3_path);
@@ -46,8 +51,9 @@ void VFS::UnMountAll()
 	for(u32 i=0; i<m_devices.size(); ++i)
 	{
 		delete m_devices[i];
-		m_devices.erase(m_devices.begin() +i);
 	}
+
+	m_devices.clear();
 }
 
 vfsFileBase* VFS::OpenFile(const std::string& ps3_path, vfsOpenMode mode) const

rpcs3/Emu/FS/VFS.h

@@ -41,8 +41,13 @@ struct VFSManagerEntry
 
 struct VFS
 {
+	~VFS();
+
 	//TODO: find out where these are supposed to be deleted or just make it shared_ptr
 	//and also make GetDevice and GetDeviceLocal return shared_ptr then.
+	// A vfsDevice will be deleted when they're unmounted or the VFS struct is destroyed.
+	// This will cause problems if other code stores the pointer returned by GetDevice/GetDeviceLocal
+	// and tries to use it after the device is unmounted.
 	std::vector<vfsDevice *> m_devices;
 	void Mount(const std::string& ps3_path, const std::string& local_path, vfsDevice* device);
 	void UnMount(const std::string& ps3_path);