Merge pull request #18201 from hrydgard/asserts-and-checks

Asserts and checks
hrydgard · Sep 23, 2023 · 7dc18a9 · 7dc18a9
2 parents c1529b2 + e64d1e9
commit 7dc18a9
Show file tree

Hide file tree

Showing 9 changed files with 19 additions and 28 deletions.
diff --git a/Common/GPU/Vulkan/VulkanMemory.cpp b/Common/GPU/Vulkan/VulkanMemory.cpp
@@ -291,7 +291,7 @@ VulkanPushPool::Block VulkanPushPool::CreateBlock(size_t size) {
 	_assert_(result == VK_SUCCESS);
 
 	result = vmaMapMemory(vulkan_->Allocator(), block.allocation, (void **)(&block.writePtr));
-	_assert_msg_(result == VK_SUCCESS, "VulkanPushPool: Failed to map memory (result = %08x)", result);
+	_assert_msg_(result == VK_SUCCESS, "VulkanPushPool: Failed to map memory (result = %s)", VulkanResultToString(result));
 
 	_assert_msg_(block.writePtr != nullptr, "VulkanPushPool: Failed to map memory on block of size %d", (int)block.size);
 	return block;

diff --git a/Common/Thread/Promise.h b/Common/Thread/Promise.h
@@ -45,6 +45,7 @@ class PromiseTask : public Task {
 template<class T>
 class Promise {
 public:
+	// Never fails.
 	static Promise<T> *Spawn(ThreadManager *threadman, std::function<T()> fun, TaskType taskType, TaskPriority taskPriority = TaskPriority::NORMAL) {
 		Mailbox<T> *mailbox = new Mailbox<T>();
 

diff --git a/Core/HLE/ReplaceTables.cpp b/Core/HLE/ReplaceTables.cpp
@@ -1590,7 +1590,10 @@ std::vector<int> GetReplacementFuncIndexes(u64 hash, int funcSize) {
 	return emptyResult;
 }
 
-const ReplacementTableEntry *GetReplacementFunc(int i) {
+const ReplacementTableEntry *GetReplacementFunc(size_t i) {
+	if (i >= ARRAY_SIZE(entries)) {
+		return nullptr;
+	}
 	return &entries[i];
 }
 

diff --git a/Core/HLE/ReplaceTables.h b/Core/HLE/ReplaceTables.h
@@ -64,7 +64,7 @@ void Replacement_Shutdown();
 
 int GetNumReplacementFuncs();
 std::vector<int> GetReplacementFuncIndexes(u64 hash, int funcSize);
-const ReplacementTableEntry *GetReplacementFunc(int index);
+const ReplacementTableEntry *GetReplacementFunc(size_t index);
 
 void WriteReplaceInstructions(u32 address, u64 hash, int size);
 void RestoreReplacedInstruction(u32 address);

diff --git a/Core/MIPS/ARM/ArmJit.cpp b/Core/MIPS/ARM/ArmJit.cpp
@@ -561,7 +561,7 @@ void ArmJit::Comp_ReplacementFunc(MIPSOpcode op)
 
 	const ReplacementTableEntry *entry = GetReplacementFunc(index);
 	if (!entry) {
-		ERROR_LOG(HLE, "Invalid replacement op %08x", op.encoding);
+		ERROR_LOG_REPORT_ONCE(replFunc, HLE, "Invalid replacement op %08x at %08x", op.encoding, js.compilerPC);
 		return;
 	}
 

diff --git a/Core/MIPS/ARM64/Arm64Jit.cpp b/Core/MIPS/ARM64/Arm64Jit.cpp
@@ -562,7 +562,8 @@ void Arm64Jit::Comp_ReplacementFunc(MIPSOpcode op)
 
 	const ReplacementTableEntry *entry = GetReplacementFunc(index);
 	if (!entry) {
-		ERROR_LOG(HLE, "Invalid replacement op %08x", op.encoding);
+		ERROR_LOG_REPORT_ONCE(replFunc, HLE, "Invalid replacement op %08x at %08x", op.encoding, js.compilerPC);
+		// TODO: What should we do here? We're way off in the weeds probably.
 		return;
 	}
 

diff --git a/Core/MIPS/x86/Jit.cpp b/Core/MIPS/x86/Jit.cpp
@@ -605,7 +605,7 @@ void Jit::Comp_ReplacementFunc(MIPSOpcode op) {
 
 	const ReplacementTableEntry *entry = GetReplacementFunc(index);
 	if (!entry) {
-		ERROR_LOG(HLE, "Invalid replacement op %08x", op.encoding);
+		ERROR_LOG_REPORT_ONCE(replFunc, HLE, "Invalid replacement op %08x at %08x", op.encoding, js.compilerPC);
 		return;
 	}
 

diff --git a/GPU/Vulkan/ShaderManagerVulkan.cpp b/GPU/Vulkan/ShaderManagerVulkan.cpp
@@ -47,6 +47,7 @@
 // Most drivers treat vkCreateShaderModule as pretty much a memcpy. What actually
 // takes time here, and makes this worthy of parallelization, is GLSLtoSPV.
 // Takes ownership over tag.
+// This always returns something, checking the return value for null is not meaningful.
 static Promise<VkShaderModule> *CompileShaderModuleAsync(VulkanContext *vulkan, VkShaderStageFlagBits stage, const char *code, std::string *tag) {
 	auto compile = [=] {
 		PROFILE_THIS_SCOPE("shadercomp");
@@ -112,13 +113,10 @@ static Promise<VkShaderModule> *CompileShaderModuleAsync(VulkanContext *vulkan,
 
 VulkanFragmentShader::VulkanFragmentShader(VulkanContext *vulkan, FShaderID id, FragmentShaderFlags flags, const char *code)
 	: vulkan_(vulkan), id_(id), flags_(flags) {
+	_assert_(!id.is_invalid());
 	source_ = code;
 	module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_FRAGMENT_BIT, source_.c_str(), new std::string(FragmentShaderDesc(id)));
-	if (!module_) {
-		failed_ = true;
-	} else {
-		VERBOSE_LOG(G3D, "Compiled fragment shader:\n%s\n", (const char *)code);
-	}
+	VERBOSE_LOG(G3D, "Compiled fragment shader:\n%s\n", (const char *)code);
 }
 
 VulkanFragmentShader::~VulkanFragmentShader() {
@@ -147,13 +145,10 @@ std::string VulkanFragmentShader::GetShaderString(DebugShaderStringType type) co
 
 VulkanVertexShader::VulkanVertexShader(VulkanContext *vulkan, VShaderID id, VertexShaderFlags flags, const char *code, bool useHWTransform)
 	: vulkan_(vulkan), useHWTransform_(useHWTransform), flags_(flags), id_(id) {
+	_assert_(!id.is_invalid());
 	source_ = code;
 	module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_VERTEX_BIT, source_.c_str(), new std::string(VertexShaderDesc(id)));
-	if (!module_) {
-		failed_ = true;
-	} else {
-		VERBOSE_LOG(G3D, "Compiled vertex shader:\n%s\n", (const char *)code);
-	}
+	VERBOSE_LOG(G3D, "Compiled vertex shader:\n%s\n", (const char *)code);
 }
 
 VulkanVertexShader::~VulkanVertexShader() {
@@ -182,13 +177,10 @@ std::string VulkanVertexShader::GetShaderString(DebugShaderStringType type) cons
 
 VulkanGeometryShader::VulkanGeometryShader(VulkanContext *vulkan, GShaderID id, const char *code)
 	: vulkan_(vulkan), id_(id) {
+	_assert_(!id.is_invalid());
 	source_ = code;
 	module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_GEOMETRY_BIT, source_.c_str(), new std::string(GeometryShaderDesc(id).c_str()));
-	if (!module_) {
-		failed_ = true;
-	} else {
-		VERBOSE_LOG(G3D, "Compiled geometry shader:\n%s\n", (const char *)code);
-	}
+	VERBOSE_LOG(G3D, "Compiled geometry shader:\n%s\n", (const char *)code);
 }
 
 VulkanGeometryShader::~VulkanGeometryShader() {

diff --git a/GPU/Vulkan/ShaderManagerVulkan.h b/GPU/Vulkan/ShaderManagerVulkan.h
@@ -43,8 +43,6 @@ class VulkanFragmentShader {
 
 	const std::string &source() const { return source_; }
 
-	bool Failed() const { return failed_; }
-
 	std::string GetShaderString(DebugShaderStringType type) const;
 	Promise<VkShaderModule> *GetModule() { return module_; }
 	const FShaderID &GetID() const { return id_; }
@@ -68,7 +66,6 @@ class VulkanVertexShader {
 
 	const std::string &source() const { return source_; }
 
-	bool Failed() const { return failed_; }
 	bool UseHWTransform() const { return useHWTransform_; }  // TODO: Roll into flags
 	VertexShaderFlags Flags() const { return flags_; }
 
@@ -81,7 +78,6 @@ class VulkanVertexShader {
 
 	VulkanContext *vulkan_;
 	std::string source_;
-	bool failed_ = false;
 	bool useHWTransform_;
 	VShaderID id_;
 	VertexShaderFlags flags_;
@@ -94,9 +90,8 @@ class VulkanGeometryShader {
 
 	const std::string &source() const { return source_; }
 
-	bool Failed() const { return failed_; }
-
 	std::string GetShaderString(DebugShaderStringType type) const;
+
 	Promise<VkShaderModule> *GetModule() const { return module_; }
 	const GShaderID &GetID() { return id_; }
 
@@ -105,7 +100,6 @@ class VulkanGeometryShader {
 
 	VulkanContext *vulkan_;
 	std::string source_;
-	bool failed_ = false;
 	GShaderID id_;
 };