Merge pull request #21096 from hrydgard/more-assorted-fixes

More assorted fixes

Common/GPU/OpenGL/thin3d_gl.cpp

@@ -1628,8 +1628,8 @@ bool OpenGLContext::BlitFramebuffer(Framebuffer *fbsrc, int srcX1, int srcY1, in
 void OpenGLContext::BindFramebufferAsTexture(Framebuffer *fbo, int binding, Aspect aspects, int layer) {
 	OpenGLFramebuffer *fb = (OpenGLFramebuffer *)fbo;
 	_assert_(binding < MAX_TEXTURE_SLOTS);
-	_dbg_assert_(fb);
-	_dbg_assert_(fb->framebuffer_);
+	_assert_(fb);
+	_assert_(fb->framebuffer_);
 
 	GLuint glAspect = 0;
 	if (aspects & Aspect::COLOR_BIT) {

Common/GPU/Vulkan/VulkanQueueRunner.cpp

@@ -322,7 +322,9 @@ void VulkanQueueRunner::RunSteps(std::vector<VKRStep *> &steps, int curFrame, Fr
 					// So only acquire once.
 					if (!frameData.hasAcquired) {
 						frameData.AcquireNextImage(vulkan_);
-						SetBackbuffer(framebuffers_[frameData.curSwapchainImage], frameDataShared.swapchainImages_[frameData.curSwapchainImage].image);
+						if (frameData.hasAcquired && frameData.curSwapchainImage != (uint32_t)-1) {
+							SetBackbuffer(framebuffers_[frameData.curSwapchainImage], frameDataShared.swapchainImages_[frameData.curSwapchainImage].image);
+						}
 					}
 
 					if (!frameData.hasPresentCommands) {

Common/Render/Text/draw_text.cpp

@@ -126,6 +126,11 @@ void TextDrawer::MeasureString(std::string_view str, float *w, float *h) {
 		return;
 	}
 
+	// Clamp the size to something sane.
+	if (str.size() > MAX_TEXT_LENGTH) {
+		str = str.substr(0, MAX_TEXT_LENGTH);
+	}
+
 	const CacheKeyType key{std::string(str), fontStyle_};
 
 	TextMeasureEntry *entry;
@@ -151,6 +156,11 @@ void TextDrawer::MeasureString(std::string_view str, float *w, float *h) {
 void TextDrawer::MeasureStringRect(std::string_view str, float maxWidth, float *w, float *h, int align) {
 	const int wrap = align & (FLAG_WRAP_TEXT | FLAG_ELLIPSIZE_TEXT);
 
+	// Clamp the size to something sane.
+	if (str.size() > MAX_TEXT_LENGTH) {
+		str = str.substr(0, MAX_TEXT_LENGTH);
+	}
+
 	float plainW, plainH;
 	MeasureString(str, &plainW, &plainH);
 
@@ -181,16 +191,25 @@ void TextDrawer::DrawStringRect(DrawBuffer &target, std::string_view str, const
 		y = bounds.y2();
 	}
 
+	// Clamp the size to something sane.
+	if (str.size() > MAX_TEXT_LENGTH) {
+		str = str.substr(0, MAX_TEXT_LENGTH);
+	}
+
 	std::string toDraw(str);
 	int wrap = align & (FLAG_WRAP_TEXT | FLAG_ELLIPSIZE_TEXT);
 	if (wrap) {
 		WrapString(toDraw, str, bounds.w, wrap);
 	}
-
 	DrawString(target, toDraw, x, y, color, align);
 }
 
 bool TextDrawer::DrawStringBitmapRect(std::vector<uint8_t> &bitmapData, TextStringEntry &entry, Draw::DataFormat texFormat, std::string_view str, const Bounds &bounds, int align, bool fullColor) {
+	// Clamp the size to something sane.
+	if (str.size() > MAX_TEXT_LENGTH) {
+		str = str.substr(0, MAX_TEXT_LENGTH);
+	}
+
 	std::string toDraw(str);
 	int wrap = align & (FLAG_WRAP_TEXT | FLAG_ELLIPSIZE_TEXT);
 	if (wrap) {

Common/Render/Text/draw_text.h

@@ -124,6 +124,9 @@ protected:
 	bool ignoreGlobalDpi_ = false;
 	FontStyle fontStyle_{};
 
+	// We will clamp strings to this length to avoid various degenenerate behaviors.k
+	static constexpr size_t MAX_TEXT_LENGTH = 16384;
+
 	std::map<CacheKeyType, std::unique_ptr<TextStringEntry>> cache_;
 	std::map<CacheKeyType, std::unique_ptr<TextMeasureEntry>> sizeCache_;
 };

Core/Config.cpp

@@ -309,7 +309,7 @@ static const ConfigSetting generalSettings[] = {
 	ConfigSetting("RemoteTab", SETTING(g_Config, bRemoteTab), false, CfgFlag::DEFAULT),
 	ConfigSetting("RemoteISOSharedDir", SETTING(g_Config, sRemoteISOSharedDir), "", CfgFlag::DEFAULT),
 	ConfigSetting("RemoteISOShareType", SETTING(g_Config, iRemoteISOShareType), (int)RemoteISOShareType::RECENT, CfgFlag::DEFAULT),
-	ConfigSetting("AskForExitConfirmationAfterSeconds", SETTING(g_Config, iAskForExitConfirmationAfterSeconds), 60, CfgFlag::PER_GAME),
+	ConfigSetting("AskForExitConfirmationAfterSeconds", SETTING(g_Config, iAskForExitConfirmationAfterSeconds), 300, CfgFlag::PER_GAME),
 
 #if PPSSPP_PLATFORM(ANDROID) || PPSSPP_PLATFORM(IOS)
 	ConfigSetting("ScreenRotation", SETTING(g_Config, iScreenRotation), &DefaultScreenRotation, CfgFlag::DEFAULT),

Core/Dialog/SavedataParam.cpp

@@ -892,7 +892,7 @@ std::set<std::string> SavedataParam::GetSecureFileNames(const std::string &dirPa
 bool SavedataParam::GetExpectedHash(const std::string &dirPath, const std::string &filename, u8 hash[16]) {
 	auto entries = GetSFOEntries(dirPath);
 
-	for (auto entry : entries) {
+	for (const auto &entry : entries) {
 		if (strncmp(entry.filename, filename.c_str(), sizeof(entry.filename)) == 0) {
 			memcpy(hash, entry.hash, sizeof(entry.hash));
 			return true;
@@ -920,13 +920,7 @@ void SavedataParam::LoadFile(const std::string& dirPath, const std::string& file
 }
 
 // Note: The work is done in-place, hence the memmove etc.
-int SavedataParam::EncryptData(unsigned int mode,
-		 unsigned char *data,
-		 int *dataLen,
-		 int *alignedLen,
-		 unsigned char *hash,
-		 unsigned char *cryptkey)
-{
+int SavedataParam::EncryptData(unsigned int mode, unsigned char *data, int *dataLen, int *alignedLen, unsigned char *hash, const u8 *cryptkey) {
 	pspChnnlsvContext1 ctx1{};
 	pspChnnlsvContext2 ctx2{};
 
@@ -975,7 +969,7 @@ int SavedataParam::EncryptData(unsigned int mode,
 }
 
 // Note: The work is done in-place, hence the memmove etc.
-int SavedataParam::DecryptData(unsigned int mode, unsigned char *data, int *dataLen, int *alignedLen, unsigned char *cryptkey, const u8 *expectedHash) {
+int SavedataParam::DecryptData(unsigned int mode, unsigned char *data, int *dataLen, int *alignedLen, const u8 *cryptkey, const u8 *expectedHash) {
 	pspChnnlsvContext1 ctx1{};
 	pspChnnlsvContext2 ctx2{};
 
@@ -1015,8 +1009,7 @@ int SavedataParam::DecryptData(unsigned int mode, unsigned char *data, int *data
 }
 
 // Requires sfoData to be padded with zeroes to the next 16-byte boundary (due to BuildHash)
-int SavedataParam::UpdateHash(u8* sfoData, int sfoSize, int sfoDataParamsOffset, int encryptmode)
-{
+int SavedataParam::UpdateHash(u8 *sfoData, int sfoSize, int sfoDataParamsOffset, int encryptmode) {
 	int alignedLen = align16(sfoSize);
 	memset(sfoData + sfoDataParamsOffset, 0, 128);
 	u8 filehash[16];

Core/Dialog/SavedataParam.h

@@ -374,8 +374,8 @@ private:
 	bool LoadSFO(SceUtilitySavedataParam *param, const std::string& dirPath);
 	void LoadFile(const std::string& dirPath, const std::string& filename, PspUtilitySavedataFileData *fileData);
 
-	int DecryptData(unsigned int mode, unsigned char *data, int *dataLen, int *alignedLen, unsigned char *cryptkey, const u8 *expectedHash);
-	int EncryptData(unsigned int mode, unsigned char *data, int *dataLen, int *alignedLen, unsigned char *hash, unsigned char *cryptkey);
+	int DecryptData(unsigned int mode, unsigned char *data, int *dataLen, int *alignedLen, const u8 *cryptkey, const u8 *expectedHash);
+	int EncryptData(unsigned int mode, unsigned char *data, int *dataLen, int *alignedLen, u8 *hash, const u8 *cryptkey);
 	int UpdateHash(u8* sfoData, int sfoSize, int sfoDataParamsOffset, int encryptmode);
 
 	// data must be zero-padded from len to alignedLen (which should be the next multiply of 16)!

Core/HW/MediaEngine.cpp

@@ -332,6 +332,8 @@ bool MediaEngine::openContext(bool keepReadPos) {
 			return false;
 	}
 
+	// Here it shouldn't be possible for m_videoStream to be invalid.
+
 	if (!setVideoStream(m_videoStream, true))
 		return false;
 

Core/MIPS/ARM/ArmJit.cpp

@@ -497,7 +497,6 @@ void ArmJit::UnlinkBlock(u8 *checkedEntry, u32 originalAddress) {
 }
 
 bool ArmJit::ReplaceJalTo(u32 dest) {
-#if PPSSPP_ARCH(ARM)
 	const ReplacementTableEntry *entry = nullptr;
 	u32 funcSize = 0;
 	if (!CanReplaceJalTo(dest, &entry, &funcSize)) {
@@ -546,7 +545,6 @@ bool ArmJit::ReplaceJalTo(u32 dest) {
 
 	// Add a trigger so that if the inlined code changes, we invalidate this block.
 	blocks.ProxyBlock(js.blockStart, dest, funcSize / sizeof(u32), GetCodePtr());
-#endif
 	return true;
 }
 

Core/MIPS/JitCommon/JitBlockCache.cpp

@@ -40,27 +40,7 @@
 #include "Core/MIPS/JitCommon/JitBlockCache.h"
 #include "Core/MIPS/JitCommon/JitCommon.h"
 
-// #include "JitBase.h"
-
-// Enable define below to enable oprofile integration. For this to work,
-// it requires at least oprofile version 0.9.4, and changing the build
-// system to link the Dolphin executable against libopagent.	Since the
-// dependency is a little inconvenient and this is possibly a slight
-// performance hit, it's not enabled by default, but it's useful for
-// locating performance issues.
-#if defined USE_OPROFILE && USE_OPROFILE
-#include <opagent.h>
-
-op_agent_t agent;
-#endif
-
-#if defined USE_VTUNE
-#include <jitprofiling.h>
-#pragma comment(lib, "libittnotify.lib")
-#pragma comment(lib, "jitprofiling.lib")
-#endif
-
-const u32 INVALID_EXIT = 0xFFFFFFFF;
+constexpr u32 INVALID_EXIT = 0xFFFFFFFF;
 
 static uint64_t HashJitBlock(const JitBlock &b) {
 	PROFILE_THIS_SCOPE("jithash");
@@ -96,9 +76,6 @@ bool JitBlockCache::IsFull() const {
 }
 
 void JitBlockCache::Init() {
-#if defined USE_OPROFILE && USE_OPROFILE
-	agent = op_open_agent();
-#endif
 	blocks_ = new JitBlock[MAX_NUM_BLOCKS];
 	Clear();
 }
@@ -108,13 +85,6 @@ void JitBlockCache::Shutdown() {
 	delete [] blocks_;
 	blocks_ = 0;
 	num_blocks_ = 0;
-#if defined USE_OPROFILE && USE_OPROFILE
-	op_close_agent(agent);
-#endif
-
-#ifdef USE_VTUNE
-	iJIT_NotifyEvent(iJVM_EVENT_TYPE_SHUTDOWN, NULL);
-#endif
 }
 
 // This clears the JIT cache. It's called from JitCache.cpp when the JIT cache
@@ -287,27 +257,6 @@ void JitBlockCache::FinalizeBlock(int block_num, bool block_link) {
 	if (blockEnd > PSP_GetUserMemoryBase() + halfUserMemory) {
 		ExpandRange(blockMemRanges_[JITBLOCK_RANGE_RAMTOP], b.originalAddress, blockEnd);
 	}
-
-#if defined USE_OPROFILE && USE_OPROFILE
-	char buf[100];
-	snprintf(buf, sizeof(buf), "EmuCode%x", b.originalAddress);
-	const u8* blockStart = blocks_[block_num].checkedEntry;
-	op_write_native_code(agent, buf, (uint64_t)blockStart, blockStart, b.normalEntry + b.codeSize - b.checkedEntry);
-#endif
-
-#ifdef USE_VTUNE
-	snprintf(b.blockName, sizeof(b.blockName), "EmuCode_0x%08x", b.originalAddress);
-
-	iJIT_Method_Load jmethod = {0};
-	jmethod.method_id = iJIT_GetNewMethodID();
-	jmethod.class_file_name = "";
-	jmethod.source_file_name = __FILE__;
-	jmethod.method_load_address = (void*)blocks_[block_num].checkedEntry;
-	jmethod.method_size = b.normalEntry + b.codeSize - b.checkedEntry;
-	jmethod.line_number_size = 0;
-	jmethod.method_name = b.blockName;
-	iJIT_NotifyEvent(iJVM_EVENT_TYPE_METHOD_LOAD_FINISHED, (void*)&jmethod);
-#endif
 }
 
 bool JitBlockCache::RangeMayHaveEmuHacks(u32 start, u32 end) const {

Core/MIPS/JitCommon/JitBlockCache.h

@@ -51,10 +51,6 @@ enum class DestroyType {
 	CLEAR,
 };
 
-// Define this in order to get VTune profile support for the Jit generated code.
-// Add the VTune include/lib directories to the project directories to get this to build.
-// #define USE_VTUNE
-
 // We should be careful not to access these block structures during runtime as they are large.
 // Fine to mess with them at block compile time though.
 struct JitBlock {
@@ -76,10 +72,6 @@ struct JitBlock {
 	bool invalid;
 	bool linkStatus[MAX_JIT_BLOCK_EXITS];
 
-#ifdef USE_VTUNE
-	char blockName[32];
-#endif
-
 	// By having a pointer, we avoid a constructor/destructor being generated and dog slow
 	// performance in debug.
 	std::vector<u32> *proxyFor;

GPU/Common/FramebufferManagerCommon.cpp

@@ -970,7 +970,6 @@ void FramebufferManagerCommon::DestroyFramebuf(VirtualFramebuffer *v) {
 
 void FramebufferManagerCommon::BlitFramebufferDepth(VirtualFramebuffer *src, VirtualFramebuffer *dst, bool allowSizeMismatch) {
 	_dbg_assert_(src && dst);
-
 	_dbg_assert_(src != dst);
 
 	// Check that the depth address is even the same before actually blitting.
@@ -3536,8 +3535,9 @@ void FramebufferManagerCommon::BlitUsingRaster(
 	_dbg_assert_(dest);
 	_dbg_assert_(pipeline);
 
-	if (!src || !dest) {
+	if (!src || !dest || !pipeline) {
 		// Nothing we can do, other than trying to catch it in debug with the asserts above.
+		return;
 	}
 
 	if (pipeline->info.writeChannel == RASTER_DEPTH) {

GPU/GPUCommon.cpp

@@ -1908,16 +1908,21 @@ void GPUCommon::DoBlockTransfer(u32 skipDrawReason) {
 }
 
 bool GPUCommon::PerformMemoryCopy(u32 dest, u32 src, int size, GPUCopyFlag flags) {
-	/*
-	// TODO: Should add this. But let's do it after the 1.18 release.
-	if (dest == 0 || src == 0) {
-		_dbg_assert_msg_(false, "Bad PerformMemoryCopy: %08x -> %08x, size %d (flag: %d)", src, dest, size, (int)flags);
-		return false;
-	}
-	*/
 	if (size == 0) {
 		_dbg_assert_msg_(false, "Zero-sized PerformMemoryCopy: %08x -> %08x, size %d (flag: %d)", src, dest, size, (int)flags);
-		// Let's not ignore this yet but if we hit this, we should investigate.
+		return false;
+	}
+
+	// If dest is not a valid address, just ignore.
+	if (!Memory::IsValidAddress(dest)) {
+		_dbg_assert_msg_(false, "Invalid address for PerformMemorySet: %08x, size %d", dest, size);
+		return false;
+	}
+
+	// Check for invalid memory range. Should we reject? For now, let's clamp it.
+	if (Memory::ClampValidSizeAt(dest, size) < (u32)size) {
+		ERROR_LOG_REPORT_ONCE(invalidmemset, Log::G3D, "PerformMemorySet with invalid range: %08x, size %d", dest, size);
+		size = Memory::ClampValidSizeAt(dest, size);
 	}
 
 	// Track stray copies of a framebuffer in RAM. MotoGP does this.
@@ -1948,6 +1953,23 @@ bool GPUCommon::PerformMemoryCopy(u32 dest, u32 src, int size, GPUCopyFlag flags
 }
 
 bool GPUCommon::PerformMemorySet(u32 dest, u8 v, int size) {
+	if (size == 0) {
+		_dbg_assert_msg_(false, "Zero-sized PerformMemorySet: %08x, value %02x, size %d", dest, v, size);
+		return false;
+	}
+
+	// If dest is not a valid address, just ignore.
+	if (!Memory::IsValidAddress(dest)) {
+		_dbg_assert_msg_(false, "Invalid address for PerformMemorySet: %08x, size %d", dest, size);
+		return false;
+	}
+
+	// Check for invalid memory range. Should we reject? For now, let's clamp it.
+	if (Memory::ClampValidSizeAt(dest, size) < (u32)size) {
+		ERROR_LOG_REPORT_ONCE(invalidmemset, Log::G3D, "PerformMemorySet with invalid range: %08x, size %d", dest, size);
+		size = Memory::ClampValidSizeAt(dest, size);
+	}
+
 	// This may indicate a memset, usually to 0, of a framebuffer.
 	if (framebufferManager_->MayIntersectFramebufferColor(dest)) {
 		Memory::Memset(dest, v, size, "GPUMemset");

GPU/GPUCommon.h

@@ -131,8 +131,10 @@ public:
 	// If size = -1, invalidate everything.
 	virtual void InvalidateCache(u32 addr, int size, GPUInvalidationType type) = 0;
 
+	// These return true if they handled the operation enough that the actual memory operation should be skipped. Not always a clear-cut case...
 	virtual bool PerformMemoryCopy(u32 dest, u32 src, int size, GPUCopyFlag flags = GPUCopyFlag::NONE);
 	virtual bool PerformMemorySet(u32 dest, u8 v, int size);
+
 	virtual bool PerformReadbackToMemory(u32 dest, int size);
 	virtual bool PerformWriteColorFromMemory(u32 dest, int size);
 

UI/GameInfoCache.cpp

@@ -879,22 +879,27 @@ handleELF:
 		}
 
 		if (flags_ & GameInfoFlags::SIZE) {
-			std::lock_guard<std::mutex> lock(info_->lock);
-			info_->gameSizeOnDisk = info_->GetSizeOnDiskInBytes();
+			const u64 gameSizeOnDisk = info_->GetSizeOnDiskInBytes();
+			u64 saveDataSize = 0;
+			u64 installDataSize = 0;
+
 			switch (info_->fileType) {
 			case IdentifiedFileType::PSP_ISO:
 			case IdentifiedFileType::PSP_ISO_NP:
 			case IdentifiedFileType::PSP_DISC_DIRECTORY:
 			case IdentifiedFileType::PSP_PBP:
 			case IdentifiedFileType::PSP_PBP_DIRECTORY:
-				info_->saveDataSize = info_->GetGameSavedataSizeInBytes();
-				info_->installDataSize = info_->GetInstallDataSizeInBytes();
+				saveDataSize = info_->GetGameSavedataSizeInBytes();
+				installDataSize = info_->GetInstallDataSizeInBytes();
 				break;
 			default:
-				info_->saveDataSize = 0;
-				info_->installDataSize = 0;
 				break;
 			}
+
+			std::lock_guard<std::mutex> lock(info_->lock);
+			info_->gameSizeOnDisk = gameSizeOnDisk;
+			info_->saveDataSize = saveDataSize;
+			info_->installDataSize = installDataSize;
 		}
 		if (flags_ & GameInfoFlags::UNCOMPRESSED_SIZE) {
 			info_->gameSizeUncompressed = info_->GetSizeUncompressedInBytes();

UI/GameSettingsScreen.cpp

@@ -1353,7 +1353,7 @@ void GameSettingsScreen::CreateSystemSettings(UI::ViewGroup *systemSettings) {
 
 	systemSettings->Add(new ItemHeader(sy->T("General")));
 
-	PopupSliderChoice *exitConfirmation = systemSettings->Add(new PopupSliderChoice(&g_Config.iAskForExitConfirmationAfterSeconds, 0, 1200, 60, sy->T("Ask for exit confirmation after seconds"), screenManager(), "s"));
+	PopupSliderChoice *exitConfirmation = systemSettings->Add(new PopupSliderChoice(&g_Config.iAskForExitConfirmationAfterSeconds, 0, 1200, 300, sy->T("Ask for exit confirmation after seconds"), screenManager(), "s"));
 	exitConfirmation->SetZeroLabel(sy->T("Off"));
 
 	if (System_GetPropertyInt(SYSPROP_DEVICE_TYPE) == DEVICE_TYPE_MOBILE) {