diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index f8ecf9e201..58cef231d7 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -1186,6 +1186,118 @@ static bool verifyMatrixBuffer(const void *ActualBuffer, return false; } +// The bytes a matrix does not occupy -- the prologue before the offset, and +// the padding between rows when the stride exceeds a packed row -- must +// survive a store untouched. +// +// Comparing elements alone would not catch a store that damages the bytes +// around them. A store that ignored the stride entirely writes its elements to +// the wrong addresses and fails the element comparison anyway, but a store +// that places every element correctly and also widens its writes over the +// padding produces a correct matrix while silently corrupting whatever else +// shared the buffer. +// Seeding the destination with a poison pattern and checking that the +// non-element bytes still hold it separates those two cases. +// +// The pattern varies with the byte offset rather than repeating a single +// value. A constant would be indistinguishable from a store that happened to +// write that same value, and also from memory nobody wrote at all -- 0xcd, the +// obvious choice, is what the MSVC debug allocator fills fresh heap with. +// Multiplying the offset by an odd number keeps consecutive bytes distinct, so +// a store writing any constant over two or more adjacent bytes is always +// caught, and a single overwritten byte survives only if it happens to match +// the pattern at exactly that offset. +// +// Counting is kept separate from reporting so the check can be unit tested in +// both directions. verifyUntouchedBytes reports through Log::Error, which +// marks the calling test failed, so a test that deliberately supplies a +// corrupted buffer cannot call it. +static BYTE poisonByteAt(BYTE Seed, size_t Offset) { + return static_cast(Seed ^ static_cast(Offset * 31u)); +} + +static void fillPoison(void *Buffer, size_t BufferSize, BYTE Seed) { + BYTE *Bytes = static_cast(Buffer); + for (size_t I = 0; I < BufferSize; ++I) + Bytes[I] = poisonByteAt(Seed, I); +} + +// 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. +static std::optional countTouchedBytesOutsideElements( + ComponentType CompType, MatrixDim M, MatrixDim N, + const MatrixBufferLayout &Layout, const void *Buffer, size_t BufferSize, + BYTE PoisonSeed, std::vector *FirstOffsets = nullptr) { + static constexpr size_t MaxReportedOffsets = 8; + + std::optional RequiredBytes = + getMatrixBufferSize(CompType, M, N, Layout); + if (!RequiredBytes || BufferSize < *RequiredBytes) + return std::nullopt; + + const size_t ElementBytes = elementSize(CompType); + std::vector Owned(BufferSize, false); + for (MatrixDim Row = 0; Row < M; ++Row) { + for (MatrixDim Column = 0; Column < N; ++Column) { + std::optional ByteOffset = + getElementByteOffset(CompType, M, N, Row, Column, Layout); + if (!ByteOffset || *ByteOffset + ElementBytes > BufferSize) + return std::nullopt; + for (size_t I = 0; I < ElementBytes; ++I) + Owned[*ByteOffset + I] = true; + } + } + + const BYTE *Bytes = static_cast(Buffer); + size_t Corrupted = 0; + for (size_t I = 0; I < BufferSize; ++I) { + if (Owned[I] || Bytes[I] == poisonByteAt(PoisonSeed, I)) + continue; + if (FirstOffsets && FirstOffsets->size() < MaxReportedOffsets) + FirstOffsets->push_back(I); + ++Corrupted; + } + return Corrupted; +} + +// Reporting wrapper around countTouchedBytesOutsideElements for the execution +// tests. +static bool verifyUntouchedBytes(ComponentType CompType, MatrixDim M, + MatrixDim N, const MatrixBufferLayout &Layout, + const void *Buffer, size_t BufferSize, + BYTE PoisonSeed, bool Verbose) { + std::vector FirstOffsets; + std::optional Corrupted = countTouchedBytesOutsideElements( + CompType, M, N, Layout, Buffer, BufferSize, PoisonSeed, &FirstOffsets); + + if (!Corrupted) { + hlsl_test::LogErrorFmt( + L"Buffer of %zu bytes cannot hold the requested matrix layout", + BufferSize); + return false; + } + + if (*Corrupted == 0) { + if (Verbose) + hlsl_test::LogCommentFmt(L"Every byte outside the stored elements still " + L"holds the poison pattern for seed 0x%02x", + PoisonSeed); + return true; + } + + for (size_t Offset : FirstOffsets) + hlsl_test::LogErrorFmt( + L"Byte %zu is outside every element but was overwritten: " + L"actual=0x%02x, expected poison=0x%02x", + Offset, static_cast(Buffer)[Offset], + poisonByteAt(PoisonSeed, Offset)); + hlsl_test::LogErrorFmt(L"%zu bytes outside the stored elements were " + L"overwritten", + *Corrupted); + return false; +} + } // namespace cpu_oracle static std::string buildCompilerArgs(const MatrixParams &Params, @@ -1416,6 +1528,7 @@ class LinAlgCPUOracleTests { END_TEST_CLASS() TEST_METHOD(TypedMatrixBufferRoundTrip); + TEST_METHOD(UntouchedByteVerification); }; void LinAlgCPUOracleTests::TypedMatrixBufferRoundTrip() { @@ -1542,6 +1655,150 @@ void LinAlgCPUOracleTests::TypedMatrixBufferRoundTrip() { std::string::npos); } +// The padding check is verified here rather than only through the execution +// tests because a GPU round trip cannot easily produce a store that places +// every element correctly and still damages the bytes around them, which is +// the single case this check exists to catch. +void LinAlgCPUOracleTests::UntouchedByteVerification() { + using namespace cpu_oracle; + + // A 2x3 uint32 matrix at a 4 byte offset with a 16 byte stride occupies + // bytes 4..15 and 20..31, leaving a 4 byte prologue at 0..3 and 4 bytes of + // padding at 16..19. + 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, + }; + std::optional Size = getMatrixBufferSize(*Matrix, Layout); + VERIFY_IS_TRUE(Size.has_value()); + VERIFY_ARE_EQUAL(size_t(32), *Size); + + constexpr BYTE PoisonSeed = 0xa5; + std::vector Buffer(*Size); + fillPoison(Buffer.data(), Buffer.size(), PoisonSeed); + VERIFY_IS_TRUE(writeMatrixBuffer(*Matrix, Layout, Buffer)); + + auto CountTouched = [&Layout, PoisonSeed](const std::vector &Bytes) { + return countTouchedBytesOutsideElements(ComponentType::U32, 2, 3, Layout, + Bytes.data(), Bytes.size(), + PoisonSeed); + }; + + // A correctly encoded buffer leaves every non-element byte poisoned. + std::optional Clean = CountTouched(Buffer); + VERIFY_IS_TRUE(Clean.has_value()); + VERIFY_ARE_EQUAL(size_t(0), *Clean); + VERIFY_IS_TRUE(verifyUntouchedBytes(ComponentType::U32, 2, 3, Layout, + Buffer.data(), Buffer.size(), PoisonSeed, + /*Verbose=*/false)); + + // Damaging an element is the element comparison's job, not this check's, so + // the count must stay at zero. + std::vector ElementTouched = Buffer; + ElementTouched[4] ^= 0xff; + std::optional AfterElement = CountTouched(ElementTouched); + VERIFY_IS_TRUE(AfterElement.has_value()); + VERIFY_ARE_EQUAL(size_t(0), *AfterElement); + + // Damaging the prologue or the inter-row padding is what this check exists + // to catch, so each one must be counted. + for (size_t Offset : {size_t(0), size_t(16)}) { + std::vector PaddingTouched = Buffer; + PaddingTouched[Offset] ^= 0xff; + std::optional AfterPadding = CountTouched(PaddingTouched); + VERIFY_IS_TRUE(AfterPadding.has_value()); + VERIFY_ARE_EQUAL(size_t(1), *AfterPadding); + } + + // Every non-element byte damaged at once is still counted exactly. + std::vector AllTouched(*Size); + fillPoison(AllTouched.data(), AllTouched.size(), PoisonSeed); + for (BYTE &Byte : AllTouched) + Byte = static_cast(~Byte); + VERIFY_IS_TRUE(writeMatrixBuffer(*Matrix, Layout, AllTouched)); + std::optional AfterAll = CountTouched(AllTouched); + VERIFY_IS_TRUE(AfterAll.has_value()); + VERIFY_ARE_EQUAL(size_t(8), *AfterAll); + + // A buffer filled with one repeated value is fully detected, which is the + // reason the pattern varies with the offset. A constant poison would score + // zero here whenever the store happened to pick that same value, and 0xcd in + // particular is what the MSVC debug allocator leaves in memory nobody wrote. + std::vector ConstantFill(*Size, BYTE(0xcd)); + std::optional AfterConstant = CountTouched(ConstantFill); + VERIFY_IS_TRUE(AfterConstant.has_value()); + VERIFY_ARE_EQUAL(size_t(8), *AfterConstant); + + // The case a constant poison cannot survive: a store that writes a value the + // poison pattern itself uses. Because the pattern varies, that value matches + // at exactly one offset, so seven of the eight non-element bytes are still + // caught. A constant poison would match everywhere and report nothing. + std::vector PoisonValuedFill(*Size, poisonByteAt(PoisonSeed, 0)); + std::optional AfterPoisonValued = CountTouched(PoisonValuedFill); + VERIFY_IS_TRUE(AfterPoisonValued.has_value()); + VERIFY_ARE_EQUAL(size_t(7), *AfterPoisonValued); + + // No two adjacent bytes share a poison value, so a constant written over any + // two neighbours cannot hide in both. + for (size_t Offset = 1; Offset < *Size; ++Offset) + VERIFY_ARE_NOT_EQUAL(poisonByteAt(PoisonSeed, Offset - 1), + poisonByteAt(PoisonSeed, Offset)); + + // The diagnostic list is capped but the count is not, so the two have to be + // checked against a buffer with more offenders than the cap. A 2x3 uint32 + // matrix at a 16 byte offset with a 16 byte stride occupies bytes 16..27 and + // 32..43, leaving twenty bytes outside the elements. + const MatrixBufferLayout PaddedLayout = { + MatrixLayout::RowMajor, + /*OffsetBytes=*/16, + /*StrideBytes=*/16, + }; + std::optional PaddedSize = + getMatrixBufferSize(ComponentType::U32, 2, 3, PaddedLayout); + VERIFY_IS_TRUE(PaddedSize.has_value()); + VERIFY_ARE_EQUAL(size_t(44), *PaddedSize); + + std::vector AllPaddingTouched(*PaddedSize); + fillPoison(AllPaddingTouched.data(), AllPaddingTouched.size(), PoisonSeed); + for (BYTE &Byte : AllPaddingTouched) + Byte = static_cast(~Byte); + std::vector ReportedOffsets; + std::optional AfterPadded = countTouchedBytesOutsideElements( + ComponentType::U32, 2, 3, PaddedLayout, AllPaddingTouched.data(), + AllPaddingTouched.size(), PoisonSeed, &ReportedOffsets); + VERIFY_IS_TRUE(AfterPadded.has_value()); + VERIFY_ARE_EQUAL(size_t(20), *AfterPadded); + VERIFY_ARE_EQUAL(size_t(8), ReportedOffsets.size()); + for (size_t I = 0; I < ReportedOffsets.size(); ++I) + VERIFY_ARE_EQUAL(I, ReportedOffsets[I]); + + // Below the cap every offender is reported, and by its offset in the buffer + // rather than its position among the offenders. + std::vector TwoPaddingBytes(*PaddedSize); + fillPoison(TwoPaddingBytes.data(), TwoPaddingBytes.size(), PoisonSeed); + TwoPaddingBytes[28] ^= 0xff; + TwoPaddingBytes[29] ^= 0xff; + std::vector TwoOffsets; + std::optional AfterTwo = countTouchedBytesOutsideElements( + ComponentType::U32, 2, 3, PaddedLayout, TwoPaddingBytes.data(), + TwoPaddingBytes.size(), PoisonSeed, &TwoOffsets); + VERIFY_IS_TRUE(AfterTwo.has_value()); + VERIFY_ARE_EQUAL(size_t(2), *AfterTwo); + VERIFY_ARE_EQUAL(size_t(2), TwoOffsets.size()); + VERIFY_ARE_EQUAL(size_t(28), TwoOffsets[0]); + VERIFY_ARE_EQUAL(size_t(29), TwoOffsets[1]); + + // A buffer too small for the layout cannot be checked at all. + std::vector TooSmall(*Size - 1); + fillPoison(TooSmall.data(), TooSmall.size(), PoisonSeed); + VERIFY_IS_FALSE(CountTouched(TooSmall).has_value()); +} + class LinAlgCapabilityTests { public: BEGIN_TEST_CLASS(LinAlgCapabilityTests)