From 39e8e70caf9971b82b181bb55a4a3de4fdce92ee Mon Sep 17 00:00:00 2001 From: Nikhil Panchal <37629345+nikhilpanchal1@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:42:11 +0000 Subject: [PATCH] fix(recompiler): guard narrow-type scanning bounds Check module headers, instruction lengths and required operands before scanning narrow types. Cover malformed and byte-swapped modules while retaining the native SPIR-V 1.3 conversion matrix. Tested: Linux/llvmpipe CTest three times with SPIRV-Tools on and off, cold-cache Vulkan regressions, and ASan/UBSan in all optimization modes. --- .../libSceAgcDriver/tests/SpirvOptimizer.cpp | 91 ++++++++++++++----- .../SpirvBackend/src/SpirvOptimizer.cpp | 18 +++- 2 files changed, 81 insertions(+), 28 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/tests/SpirvOptimizer.cpp b/core/libs/prx/libSceAgcDriver/tests/SpirvOptimizer.cpp index 25c39567c..c9f445bbd 100644 --- a/core/libs/prx/libSceAgcDriver/tests/SpirvOptimizer.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/SpirvOptimizer.cpp @@ -7,12 +7,14 @@ #include #include #include +#include #include namespace { int failures = 0; constexpr std::uint32_t Vulkan11 = 0x00401000u; +constexpr std::uint32_t Spirv11 = 0x00010100u; constexpr std::uint32_t Spirv13 = 0x00010300u; void check(bool condition, const std::string& message) { @@ -55,7 +57,8 @@ std::string moduleSource(const NarrowType& type, bool vector, ArithmeticSupport text += "OpCapability DotProductInput4x8BitKHR\nOpExtension \"SPV_KHR_integer_dot_product\"\n"; } text += type.width == 8 ? "OpExtension \"SPV_KHR_8bit_storage\"\n" : "OpExtension \"SPV_KHR_16bit_storage\"\n"; - text += R"(OpMemoryModel Logical GLSL450 + text += R"(OpExtension "SPV_KHR_storage_buffer_storage_class" +OpMemoryModel Logical GLSL450 OpEntryPoint GLCompute %main "main" OpExecutionMode %main LocalSize 1 1 1 OpDecorate %block Block @@ -123,6 +126,14 @@ std::set capabilities(const std::vector& words) { return result; } +std::vector swapByteOrder(std::vector words) { + for (auto& word : words) { + word = (word >> 24u) | ((word & 0x00ff0000u) >> 8u) | + ((word & 0x0000ff00u) << 8u) | (word << 24u); + } + return words; +} + void testNarrowConversions(bool optimizationEnabled) { spvtools::SpirvTools validator(SPV_ENV_VULKAN_1_1); for (const auto& type : Types) { @@ -130,43 +141,50 @@ void testNarrowConversions(bool optimizationEnabled) { for (const auto support : {ArithmeticSupport::StorageOnly, ArithmeticSupport::Explicit, ArithmeticSupport::ImpliedInt8}) { if (support == ArithmeticSupport::ImpliedInt8 && (type.width != 8 || !vector)) continue; const bool arithmetic = support != ArithmeticSupport::StorageOnly; - const std::string label = std::string(type.name) + (vector ? " vector" : " scalar") + + const std::string name = std::string(type.name) + (vector ? " vector" : " scalar") + (support == ArithmeticSupport::ImpliedInt8 ? " implied Int8" : (arithmetic ? " arithmetic" : " storage only")); - const auto input = assemble(moduleSource(type, vector, support)); - if (!validator.Validate(input)) { - check(false, label + ": invalid test input"); - continue; - } - try { - const auto output = ShaderRecompiler::ValidateAndOptimizeSpirv(input, Vulkan11, Spirv13, false); - if (!validator.Validate(output)) { - check(false, label + ": optimizer returned invalid SPIR-V"); + auto native = assemble(moduleSource(type, vector, support)); + for (const auto& [version, swapped] : {std::pair{Spirv13, false}, std::pair{Spirv11, false}, std::pair{Spirv11, true}}) { + native[1] = version; + const auto input = swapped ? swapByteOrder(native) : native; + const auto label = name + (version == Spirv13 ? " SPIR-V 1.3" : " SPIR-V 1.1") + (swapped ? " swapped" : " native"); + if (!validator.Validate(input)) { + check(false, label + ": invalid test input"); continue; } - check(capabilities(output) == capabilities(input), label + ": optimizer changed device capabilities"); - check(opcodeCount(output, spv::OpStore) == (vector ? 1u : 2u), label + ": optimizer lost buffer stores"); - if (!optimizationEnabled) { - check(output == input, label + ": none mode changed the shader"); - } else { - check(opcodeCount(output, spv::OpIAdd) == 0, label + ": optimizer did not remove dead arithmetic"); - if (arithmetic && !type.floating) { - const auto conversion = type.signedInteger ? spv::OpSConvert : spv::OpUConvert; - check(opcodeCount(output, conversion) == 0, label + ": safe constant folding was disabled"); + try { + const auto output = ShaderRecompiler::ValidateAndOptimizeSpirv(input, Vulkan11, Spirv13, false); + if (!validator.Validate(output)) { + check(false, label + ": optimizer returned invalid SPIR-V"); + continue; } + const auto normalized = output.front() == spv::MagicNumber ? output : swapByteOrder(output); + check(capabilities(normalized) == capabilities(native), label + ": optimizer changed device capabilities"); + check(opcodeCount(normalized, spv::OpStore) == (vector ? 1u : 2u), label + ": optimizer lost buffer stores"); + if (!optimizationEnabled) { + check(output == input, label + ": none mode changed the shader"); + } else { + check(opcodeCount(normalized, spv::OpIAdd) == 0, label + ": optimizer did not remove dead arithmetic"); + if (arithmetic && !type.floating) { + const auto conversion = type.signedInteger ? spv::OpSConvert : spv::OpUConvert; + check(opcodeCount(normalized, conversion) == 0, label + ": safe constant folding was disabled"); + } + } + } catch (const std::exception& error) { + check(false, label + ": " + error.what()); } - } catch (const std::exception& error) { - check(false, label + ": " + error.what()); } } } } } -void expectRejected(const std::vector& words, std::uint32_t vulkanVersion, std::uint32_t spirvVersion, const char* reason) { +void expectRejected(const std::vector& words, std::uint32_t vulkanVersion, std::uint32_t spirvVersion, const char* reason, const char* diagnostic = nullptr) { try { static_cast(ShaderRecompiler::ValidateAndOptimizeSpirv(words, vulkanVersion, spirvVersion, false)); check(false, std::string("accepted ") + reason); - } catch (const std::exception&) { + } catch (const std::exception& error) { + if (diagnostic != nullptr) check(std::string(error.what()).starts_with(diagnostic), std::string(reason) + ": unexpected rejection: " + error.what()); } } @@ -178,9 +196,32 @@ void testInvalidInputIsRejected() { auto invalid = moduleSource(Types.front(), false, ArithmeticSupport::StorageOnly); invalid.insert(invalid.find("%main = OpFunction"), "%illegal = OpConstantNull %small\n"); expectRejected(assemble(invalid), Vulkan11, Spirv13, "an illegal storage-only narrow constant even when unused"); + const auto rejectMalformed = [](const std::vector& words, const char* reason) { + auto native = words; + if (native.size() > 1) native[1] = Spirv11; + constexpr auto diagnostic = "SPIR-V validation before optimization failed:"; + expectRejected(native, Vulkan11, Spirv13, reason, diagnostic); + expectRejected(swapByteOrder(native), Vulkan11, Spirv13, (std::string(reason) + " (swapped)").c_str(), diagnostic); + }; + for (std::size_t length = 0; length < 5; ++length) { + rejectMalformed(std::vector(valid.begin(), valid.begin() + length), "an incomplete module header"); + } auto malformed = valid; + malformed[0] = 0; + rejectMalformed(malformed, "an invalid module magic"); + malformed = valid; malformed[5] = spv::OpCapability; - expectRejected(malformed, Vulkan11, Spirv13, "a zero-length SPIR-V instruction"); + rejectMalformed(malformed, "a zero-length SPIR-V instruction"); + malformed[5] = (0xffffu << spv::WordCountShift) | spv::OpCapability; + rejectMalformed(malformed, "an instruction extending beyond the module"); + for (const auto& instruction : { + std::vector{(1u << spv::WordCountShift) | spv::OpCapability}, + std::vector{(3u << spv::WordCountShift) | spv::OpTypeInt, 1u, 8u}, + std::vector{(2u << spv::WordCountShift) | spv::OpTypeFloat, 1u}}) { + malformed.assign(valid.begin(), valid.begin() + 5); + malformed.insert(malformed.end(), instruction.begin(), instruction.end()); + rejectMalformed(malformed, "truncated capability or type operands"); + } } } diff --git a/core/shader/recompiler/SpirvBackend/src/SpirvOptimizer.cpp b/core/shader/recompiler/SpirvBackend/src/SpirvOptimizer.cpp index aebbb0715..f03c6d3a5 100644 --- a/core/shader/recompiler/SpirvBackend/src/SpirvOptimizer.cpp +++ b/core/shader/recompiler/SpirvBackend/src/SpirvOptimizer.cpp @@ -11,6 +11,7 @@ namespace ShaderRecompiler { namespace { bool HasLimitedUseTypes(std::span spirv) { + if (spirv.size() < 5) throw std::runtime_error("SPIRV-Tools: incomplete module header"); bool int8 = false, int16 = false, float16 = false; bool int8Capability = false, int16Capability = false, float16Capability = false; const bool swapped = spirv[0] != spv::MagicNumber; @@ -20,8 +21,18 @@ bool HasLimitedUseTypes(std::span spirv) { ((word << 8u) & 0x00ff0000u) | (word << 24u) : word; }; - for (std::size_t offset = 5; offset < spirv.size(); offset += wordAt(offset) >> 16u) { - switch (wordAt(offset) & 0xffffu) { + if (wordAt(0) != spv::MagicNumber) throw std::runtime_error("SPIRV-Tools: invalid module magic"); + for (std::size_t offset = 5; offset < spirv.size();) { + const auto instruction = wordAt(offset); + const auto wordCount = instruction >> spv::WordCountShift; + const auto opcode = instruction & spv::OpCodeMask; + if (wordCount == 0 || wordCount > spirv.size() - offset || + (opcode == spv::OpCapability && wordCount < 2) || + (opcode == spv::OpTypeInt && wordCount < 4) || + (opcode == spv::OpTypeFloat && wordCount < 3)) { + throw std::runtime_error("SPIRV-Tools: malformed instruction while scanning narrow types"); + } + switch (opcode) { case spv::OpCapability: int8Capability |= wordAt(offset + 1) == spv::CapabilityInt8 || wordAt(offset + 1) == spv::CapabilityDotProductInput4x8Bit; @@ -33,11 +44,12 @@ bool HasLimitedUseTypes(std::span spirv) { int16 |= wordAt(offset + 2) == 16; break; case spv::OpTypeFloat: - float16 |= (wordAt(offset) >> 16u) == 3 && wordAt(offset + 2) == 16; + float16 |= wordCount == 3 && wordAt(offset + 2) == 16; break; case spv::OpFunction: return (int8 && !int8Capability) || (int16 && !int16Capability) || (float16 && !float16Capability); } + offset += wordCount; } return (int8 && !int8Capability) || (int16 && !int16Capability) || (float16 && !float16Capability); }