From 9b2514bb83b3fb3cf3a01b4f8ffd47c43bd71399 Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Thu, 13 Aug 2026 06:20:33 +1200 Subject: [PATCH 1/3] [HLSL] Add out-of-bounds store coverage for LinAlg descriptor stores Proposal 0035 requires MatrixStoreToDescriptor to bounds check its writes, permitting either of two behaviours: drop the whole store if any element falls outside the descriptor view, or drop only the elements that fall outside. The suite already covers the load side of that rule. Nothing covered the store side, so an implementation that wrote past its descriptor view passed. Adds two tests that store a matrix through a destination view shorter than its buffer, mirroring the two existing load cases. One uses a packed 16x16 F16 matrix with a 260 byte view, admitting 130 of 256 elements. The other uses a 4x8 F16 matrix at a 128 byte offset with a 172 byte view, admitting 14 of 32. Both boundaries fall inside a row rather than on a row or padding edge. The comparison is byte level rather than matrix level. Both permitted behaviours leave the bytes past the view unwritten, holding the poison the destination was seeded with, so there is no element value to compare against; decoding those bytes as F16 can also produce NaN, which does not compare equal to itself. Two host helpers support this: storeBufferBoundedByView derives the expected bytes for a given view, and verifyStoreBuffer reports the first differing byte for each candidate. The source is viewed in full so only the destination is bounds checked, leaving the result attributable to the store alone. The runner also asserts that the chosen view both admits and excludes at least one whole element, since a view that did neither would accept any result. These cases cannot by themselves fail an implementation that stores nothing at all, because dropping the whole store is one of the two permitted behaviours. The existing LoadStoreDescriptor cases, which view the destination in full, are what require the store to happen. The 260 byte boundary is deliberate. An earlier draft used 264, which admits 132 elements. That is exactly 33 times 4, so on a device with a 64 lane wave and four elements per lane, an implementation that bounds checked once per lane rather than once per element would see every lane as wholly inside or wholly outside and produce output identical to per-element checking. Per-lane checking is not one of the two permitted behaviours, so that would have been a false pass. 130 is not a multiple of the per-lane element count at any wave size the tile supports below 128 lanes. Validated on WARP: 36 total, 31 passed, 4 failed, 1 skipped, with the non-passing set identical to the branch baseline of 33/28/4/1. Two controls confirm the tests detect what they claim. Widening the destination view to the full buffer fails both, reporting the first differing byte at 260, exactly the view boundary. Making the host oracle write zeroes instead of restoring poison fails the oracle self-test, which the previous value-only check would have passed silently. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b --- .../clang/unittests/HLSLExec/LinAlgTests.cpp | 319 ++++++++++++++++++ 1 file changed, 319 insertions(+) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index baf56d2c21..e1ade5c486 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -1276,6 +1276,97 @@ static void fillPoison(void *Buffer, size_t BufferSize) { Bytes[I] = poisonByteAt(I); } +// The store-side counterpart to zeroElementsOutsideView, expressed as bytes +// rather than values: a store the bounds check rejects never touches memory, +// so the elements the view does not admit whole are left holding the poison +// the destination was seeded with. Draws the boundary with the same inclusive +// end the load side uses. +static std::optional> +storeBufferBoundedByView(const TypedMatrix &Source, + const MatrixBufferLayout &Layout, size_t ViewBytes) { + if (!isMatrixValid(Source)) { + hlsl_test::LogErrorFmt(L"Cannot bound an invalid typed matrix to a view"); + return std::nullopt; + } + + std::optional BufferSize = getMatrixBufferSize(Source, Layout); + if (!BufferSize) + return std::nullopt; + + std::vector Buffer(*BufferSize); + fillPoison(Buffer.data(), Buffer.size()); + if (!writeMatrixBuffer(Source, Layout, Buffer)) + return std::nullopt; + + const size_t ElementBytes = elementSize(Source.compType()); + for (MatrixDim Row = 0; Row < Source.M; ++Row) { + for (MatrixDim Column = 0; Column < Source.N; ++Column) { + std::optional ByteOffset = getElementByteOffset( + Source.compType(), Source.M, Source.N, Row, Column, Layout); + if (!ByteOffset) + return std::nullopt; + size_t ElementEnd; + if (!checkedAdd(*ByteOffset, ElementBytes, ElementEnd)) + return std::nullopt; + if (ElementEnd <= ViewBytes) + continue; + for (size_t I = *ByteOffset; I < ElementEnd; ++I) + Buffer[I] = poisonByteAt(I); + } + } + return Buffer; +} + +// Byte-level counterpart to verifyMatrixBuffer. The permitted store outcomes +// differ in which bytes they leave alone rather than in the values they +// produce, and the poison pattern does not always decode to a comparable +// element, so they are compared as byte images. +static bool verifyStoreBuffer(const void *ActualBuffer, size_t ActualBufferSize, + const std::vector> &Candidates, + const std::wstring &PublicRule, bool Verbose) { + if (Candidates.size() < 2 || PublicRule.empty()) { + hlsl_test::LogErrorFmt(L"Invalid store buffer oracle"); + return false; + } + + const BYTE *Actual = static_cast(ActualBuffer); + std::vector FirstMismatches; + for (const std::vector &Candidate : Candidates) { + if (Candidate.size() != ActualBufferSize) { + hlsl_test::LogErrorFmt( + L"Store candidate is %zu bytes but the buffer read back is %zu", + Candidate.size(), ActualBufferSize); + return false; + } + + size_t Mismatch = ActualBufferSize; + for (size_t I = 0; I < ActualBufferSize; ++I) { + if (Actual[I] != Candidate[I]) { + Mismatch = I; + break; + } + } + if (Mismatch == ActualBufferSize) { + if (Verbose) + hlsl_test::LogCommentFmt( + L"Store buffer matched a permitted outcome: %s", + PublicRule.c_str()); + return true; + } + FirstMismatches.push_back(Mismatch); + } + + hlsl_test::LogErrorFmt(L"No permitted store outcome matched: %s", + PublicRule.c_str()); + for (size_t I = 0; I < FirstMismatches.size(); ++I) { + const size_t Offset = FirstMismatches[I]; + hlsl_test::LogErrorFmt(L"Candidate %zu first differs at byte %zu: " + L"actual=0x%02x, expected=0x%02x", + I, Offset, Actual[Offset], Candidates[I][Offset]); + } + return false; +} + // Returns the number of offending bytes, or nullopt if the buffer cannot hold // the described matrix at all. FirstOffsets, when supplied, collects the // leading offenders for diagnostics. @@ -1585,6 +1676,7 @@ class LinAlgCPUOracleTests { TEST_METHOD(TypedMatrixBufferRoundTrip); TEST_METHOD(UntouchedByteVerification); TEST_METHOD(ViewBoundedElements); + TEST_METHOD(ViewBoundedStoreBytes); }; void LinAlgCPUOracleTests::TypedMatrixBufferRoundTrip() { @@ -1904,6 +1996,79 @@ void LinAlgCPUOracleTests::ViewBoundedElements() { VERIFY_IS_TRUE(BoundedEquals(1024, {1, 2, 3, 4, 5, 6})); } +// The store side draws the same boundary but leaves the excluded elements +// holding poison rather than zero, so it is checked here as bytes. +void LinAlgCPUOracleTests::ViewBoundedStoreBytes() { + using namespace cpu_oracle; + + // The layout ViewBoundedElements uses: elements at bytes 4, 8, 12, 20, 24 + // and 28, each 4 bytes wide, in a 32 byte buffer. + std::optional Matrix = + makeTypedMatrix(2, 3, {1, 2, 3, 4, 5, 6}); + VERIFY_IS_TRUE(Matrix.has_value()); + + const MatrixBufferLayout Layout = { + MatrixLayout::RowMajor, + /*OffsetBytes=*/4, + /*StrideBytes=*/16, + }; + static constexpr size_t ElementOffsets[] = {4, 8, 12, 20, 24, 28}; + + // Bit I is set when element I holds its value. Every element must hold + // either that or the poison a rejected store leaves behind, so an oracle + // that zeroed the rejected elements instead fails here rather than + // reporting them as merely unwritten. + auto WrittenMask = [&](size_t ViewBytes) { + std::optional> Buffer = + storeBufferBoundedByView(*Matrix, Layout, ViewBytes); + VERIFY_IS_TRUE(Buffer.has_value()); + VERIFY_ARE_EQUAL(Buffer->size(), static_cast(32)); + unsigned Mask = 0; + for (unsigned I = 0; I < 6; ++I) { + const size_t Offset = ElementOffsets[I]; + const uint32_t Value = I + 1; + BYTE Written[sizeof(Value)]; + memcpy(Written, &Value, sizeof(Value)); + bool HoldsValue = true; + bool HoldsPoison = true; + for (size_t B = 0; B < sizeof(Value); ++B) { + if ((*Buffer)[Offset + B] != Written[B]) + HoldsValue = false; + if ((*Buffer)[Offset + B] != poisonByteAt(Offset + B)) + HoldsPoison = false; + } + VERIFY_IS_TRUE(HoldsValue || HoldsPoison, + "A view bounded store element held neither its value nor " + "the poison it was seeded with"); + if (HoldsValue) + Mask |= 1u << I; + } + return Mask; + }; + + // A view covering the whole buffer writes everything, and an empty view + // drops the whole store, so it writes nothing. + VERIFY_ARE_EQUAL(WrittenMask(32), 0x3fu); + VERIFY_ARE_EQUAL(WrittenMask(0), 0x00u); + + // A view ending at 24 admits the element that ends exactly there. + VERIFY_ARE_EQUAL(WrittenMask(24), 0x0fu); + + // One byte short of that boundary drops the straddling element whole, + // including the part of it the view does reach. + VERIFY_ARE_EQUAL(WrittenMask(23), 0x07u); + + // The prologue before the offset and the padding between rows belong to no + // element, so a full store must leave both holding poison. + std::optional> Full = + storeBufferBoundedByView(*Matrix, Layout, 32); + VERIFY_IS_TRUE(Full.has_value()); + std::optional Corrupted = countTouchedBytesOutsideElements( + ComponentType::U32, 2, 3, Layout, Full->data(), Full->size()); + VERIFY_IS_TRUE(Corrupted.has_value()); + VERIFY_ARE_EQUAL(*Corrupted, static_cast(0)); +} + class LinAlgCapabilityTests { public: BEGIN_TEST_CLASS(LinAlgCapabilityTests) @@ -2120,6 +2285,8 @@ class DxilConf_SM610_LinAlg { TEST_METHOD(LoadStoreDescriptor_Wave_4x8_F32_RowMajorToColumnMajor); TEST_METHOD(LoadDescriptorOOB_Wave_16x16_F16_PartialView); TEST_METHOD(LoadDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView); + TEST_METHOD(StoreDescriptorOOB_Wave_16x16_F16_PartialView); + TEST_METHOD(StoreDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView); TEST_METHOD(SplatStore_Wave_16x16_F16); TEST_METHOD(AccumulateDescriptor_Wave_16x16_F16); @@ -2447,6 +2614,96 @@ static void runLoadDescriptorOutOfBounds( OutData.size(), Verbose)); } +// Stores through a destination view shorter than its buffer. Both permitted +// outcomes leave the bytes past the view holding poison, so the comparison is +// byte level rather than matrix level. This cannot by itself fail an +// implementation that stores nothing, since dropping the whole store is one of +// those outcomes; the LoadStoreDescriptor cases require the store to happen. +static void runStoreDescriptorOutOfBounds( + ID3D12Device *Device, dxc::SpecificDllLoader &DxcSupport, + const MatrixParams &Params, const cpu_oracle::MatrixBufferLayout &Layout, + size_t OutputViewBytes, bool Verbose, UINT ForcedWaveSize = 0) { + std::optional Input = + cpu_oracle::makeSequentialMatrix(Params.CompType, Params.M, Params.N); + VERIFY_IS_TRUE(Input.has_value(), + "Unable to construct typed StoreDescriptorOOB input"); + + std::optional BufferSize = + cpu_oracle::getMatrixBufferSize(*Input, Layout); + VERIFY_IS_TRUE(BufferSize.has_value(), + "Unable to size the StoreDescriptorOOB buffers"); + VERIFY_IS_TRUE(OutputViewBytes < *BufferSize, + "The destination view must be shorter than its buffer"); + + std::optional> PerElement = + cpu_oracle::storeBufferBoundedByView(*Input, Layout, OutputViewBytes); + std::optional> WholeStore = + cpu_oracle::storeBufferBoundedByView(*Input, Layout, 0); + std::optional> Unbounded = + cpu_oracle::storeBufferBoundedByView(*Input, Layout, *BufferSize); + VERIFY_IS_TRUE(PerElement.has_value() && WholeStore.has_value() && + Unbounded.has_value(), + "Unable to derive the StoreDescriptorOOB candidates"); + + VERIFY_IS_TRUE(*PerElement != *WholeStore, + "The destination view must admit at least one whole element"); + VERIFY_IS_TRUE(*PerElement != *Unbounded, + "The destination view must exclude at least one element"); + + std::stringstream ExtraDefs; + ExtraDefs << " -DLOAD_OFFSET=" << Layout.OffsetBytes; + ExtraDefs << " -DLOAD_STRIDE=" << Layout.StrideBytes; + ExtraDefs << " -DLOAD_LAYOUT=" << static_cast(Layout.Layout); + ExtraDefs << " -DSTORE_OFFSET=" << Layout.OffsetBytes; + ExtraDefs << " -DSTORE_STRIDE=" << Layout.StrideBytes; + ExtraDefs << " -DSTORE_LAYOUT=" << static_cast(Layout.Layout); + ExtraDefs << " -DDECLARED_ALIGN=" << DescriptorDeclaredAlignment; + + if (ForcedWaveSize != 0) + ExtraDefs << " -DFORCED_WAVE_SIZE=" << ForcedWaveSize; + + std::string Args = buildCompilerArgs(Params, ExtraDefs.str().c_str()); + + compileShader(DxcSupport, LoadStoreDescriptorShader, "cs_6_10", Args, + Verbose); + + // Only the destination view is short. The source is viewed in full so the + // load cannot be bounds checked as well, which would leave the observed + // result attributable to either operation. + const cpu_oracle::TypedMatrix InputMatrix = *Input; + auto Op = createComputeOp(LoadStoreDescriptorShader, "cs_6_10", + "DescriptorTable(UAV(u0), UAV(u1))", Args.c_str()); + addUAVBuffer(Op.get(), "Input", *BufferSize, false, "byname"); + addUAVBuffer(Op.get(), "Output", *BufferSize, true, "byname"); + addHeapRawUAV(Op.get(), "ResHeap", "Input", *BufferSize); + addHeapRawUAV(Op.get(), "ResHeap", "Output", OutputViewBytes); + addRootTable(Op.get(), 0, "ResHeap"); + + auto Result = runShaderOp( + Device, DxcSupport, std::move(Op), + [InputMatrix, Layout](LPCSTR Name, std::vector &Data, + st::ShaderOp *) { + cpu_oracle::fillPoison(Data.data(), Data.size()); + if (_stricmp(Name, "Input") != 0) + return; + VERIFY_IS_TRUE(cpu_oracle::writeMatrixBuffer(InputMatrix, Layout, Data), + "Unable to encode typed StoreDescriptorOOB input"); + }, + [Layout](ID3D12GraphicsCommandList *, st::ShaderOpTest *Test) { + verifyDescriptorBaseAlignment(Test, "Input", Layout.OffsetBytes); + verifyDescriptorBaseAlignment(Test, "Output", Layout.OffsetBytes); + }); + + MappedData OutData; + Result->Test->GetReadBackData("Output", &OutData); + + VERIFY_IS_TRUE(cpu_oracle::verifyStoreBuffer( + OutData.data(), OutData.size(), {*PerElement, *WholeStore}, + L"HLSL proposal 0035 bounds checking on MatrixStoreToDescriptor: either " + L"the whole store or only the out-of-view element stores become a no-op", + Verbose)); +} + // No offset and a tightly packed stride: a matrix occupying the whole buffer. static cpu_oracle::MatrixBufferLayout packedLayout(const MatrixParams &Params) { return cpu_oracle::MatrixBufferLayout{ @@ -2627,6 +2884,68 @@ void DxilConf_SM610_LinAlg:: SelectedWaveSize); } +// The same two views on the destination instead of the source, so the rule +// being exercised is bounds checking on the store rather than on the load. +void DxilConf_SM610_LinAlg::StoreDescriptorOOB_Wave_16x16_F16_PartialView() { + MatrixParams Params = {}; + Params.CompType = ComponentType::F16; + Params.M = 16; + Params.N = 16; + Params.Use = MatrixUse::A; + Params.Scope = MatrixScope::Wave; + Params.Layout = MatrixLayout::RowMajor; + Params.NumThreads = 128; + Params.Enable16Bit = true; + + UINT SelectedWaveSize = 0; + if (!matrixConstructionApplicable(D3DDevice, Params, {Params.Use}, + L"StoreDescriptorOOB_Wave_16x16_F16_" + L"PartialView", + SelectedWaveSize)) + return; + + // Packed, so the buffer is 16 rows of 32 bytes. A 260 byte view admits the + // first 130 elements: rows 0 to 7 whole, then two of row 8. Ending two + // elements into the row keeps the boundary off the round multiples a + // coarser-than-per-element bounds check would land on. + runStoreDescriptorOutOfBounds(D3DDevice, DxcSupport, Params, + packedLayout(Params), /*OutputViewBytes=*/260, + VerboseLogging, SelectedWaveSize); +} + +void DxilConf_SM610_LinAlg:: + StoreDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView() { + MatrixParams Params = {}; + Params.CompType = ComponentType::F16; + Params.M = 4; + Params.N = 8; + Params.Use = MatrixUse::A; + Params.Scope = MatrixScope::Wave; + Params.Layout = MatrixLayout::RowMajor; + Params.NumThreads = 128; + Params.Enable16Bit = true; + + UINT SelectedWaveSize = 0; + if (!matrixConstructionApplicable(D3DDevice, Params, {Params.Use}, + L"StoreDescriptorOOB_Wave_4x8_F16_" + L"OffsetPaddedPartialView", + SelectedWaveSize)) + return; + + const cpu_oracle::MatrixBufferLayout Layout = { + MatrixLayout::RowMajor, + /*OffsetBytes=*/DescriptorAlignedOffset, + /*StrideBytes=*/32, + }; + + // Elements sit at 128 + 32*Row + 2*Column. A 172 byte view holds row 0 + // whole and columns 0 to 5 of row 1, so it cuts within a row and stops + // short of the padding rather than on it. + runStoreDescriptorOutOfBounds(D3DDevice, DxcSupport, Params, Layout, + /*OutputViewBytes=*/172, VerboseLogging, + SelectedWaveSize); +} + static const char SplatStoreShader[] = R"( RWByteAddressBuffer Output : register(u0); From c9f2f5ea189174eaf1cd1bd13fa02ca6cdaaadc9 Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Thu, 13 Aug 2026 13:40:11 +1200 Subject: [PATCH 2/3] Apply suggestion from @alsepkow Co-authored-by: Alex Sepkowski --- tools/clang/unittests/HLSLExec/LinAlgTests.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index e1ade5c486..c6750a62b4 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -1325,7 +1325,9 @@ static bool verifyStoreBuffer(const void *ActualBuffer, size_t ActualBufferSize, const std::vector> &Candidates, const std::wstring &PublicRule, bool Verbose) { if (Candidates.size() < 2 || PublicRule.empty()) { - hlsl_test::LogErrorFmt(L"Invalid store buffer oracle"); + hlsl_test::LogErrorFmt( + L"Invalid store buffer oracle: candidates=%zu, public rule is %s", + Candidates.size(), PublicRule.empty() ? L"empty" : L"present"); return false; } From e7dad2312db62539bd3e777a002ffbe89d15d413 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 13 Aug 2026 01:47:48 +0000 Subject: [PATCH 3/3] chore: autopublish 2026-08-13T01:47:48Z --- tools/clang/unittests/HLSLExec/LinAlgTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index c6750a62b4..c077552a83 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -1326,8 +1326,8 @@ static bool verifyStoreBuffer(const void *ActualBuffer, size_t ActualBufferSize, const std::wstring &PublicRule, bool Verbose) { if (Candidates.size() < 2 || PublicRule.empty()) { hlsl_test::LogErrorFmt( - L"Invalid store buffer oracle: candidates=%zu, public rule is %s", - Candidates.size(), PublicRule.empty() ? L"empty" : L"present"); + L"Invalid store buffer oracle: candidates=%zu, public rule is %s", + Candidates.size(), PublicRule.empty() ? L"empty" : L"present"); return false; }