https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/159475
>From 86775ab587591375a00f47e825f58b3bfc02dd80 Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Wed, 17 Sep 2025 16:03:57 -0700 Subject: [PATCH 1/8] correct validation --- clang/lib/Sema/SemaHLSL.cpp | 12 ++++++++---- .../SemaHLSL/RootSignature-resource-ranges-err.hlsl | 6 +++++- .../test/SemaHLSL/RootSignature-resource-ranges.hlsl | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 0af38472b0fec..33edd7f9a916a 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1361,6 +1361,7 @@ bool SemaHLSL::handleRootSignatureElements( bool HasAnySampler = false; bool HasAnyNonSampler = false; uint32_t Offset = 0; + bool Unbound = false; for (const auto &[Clause, ClauseElem] : UnboundClauses) { SourceLocation Loc = ClauseElem->getLocation(); if (Clause->Type == llvm::dxil::ResourceClass::Sampler) @@ -1381,15 +1382,16 @@ bool SemaHLSL::handleRootSignatureElements( llvm::hlsl::rootsig::DescriptorTableOffsetAppend) { // Manually specified the offset Offset = Clause->Offset; + Unbound = false; + } else if (Unbound) { + // Trying to append onto unbound offset + Diag(Loc, diag::err_hlsl_appending_onto_unbound); } uint64_t RangeBound = llvm::hlsl::rootsig::computeRangeBound( Offset, Clause->NumDescriptors); - if (!llvm::hlsl::rootsig::verifyBoundOffset(Offset)) { - // Trying to append onto unbound offset - Diag(Loc, diag::err_hlsl_appending_onto_unbound); - } else if (!llvm::hlsl::rootsig::verifyNoOverflowedOffset(RangeBound)) { + if (!llvm::hlsl::rootsig::verifyNoOverflowedOffset(RangeBound)) { // Upper bound overflows maximum offset Diag(Loc, diag::err_hlsl_offset_overflow) << Offset << RangeBound; } @@ -1397,6 +1399,8 @@ bool SemaHLSL::handleRootSignatureElements( Offset = RangeBound == llvm::hlsl::rootsig::NumDescriptorsUnbounded ? uint32_t(RangeBound) : uint32_t(RangeBound + 1); + Unbound = Clause->NumDescriptors == + llvm::hlsl::rootsig::NumDescriptorsUnbounded; // Compute the register bounds and track resource binding uint32_t LowerBound(Clause->Reg.Number); diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl index 2d025d0e6e5ce..5ce4419149592 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -141,4 +141,8 @@ void append_offset_overflow_signature() {} // expected-error@+1 {{descriptor range offset overflows [4294967292, 4294967296]}} [RootSignature("DescriptorTable(CBV(b0, offset = 4294967292, numDescriptors = 5))")] -void offset_() {} +void offset_overflow() {} + +// expected-error@+1 {{descriptor range offset overflows [4294967295, 4294967296]}} +[RootSignature("DescriptorTable(CBV(b0, offset = 4294967294), CBV(b1, numDescriptors = 2))")] +void appended_offset_overflow() {} diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl index 10e7215eccf6e..37bb4f173aac4 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges.hlsl @@ -25,3 +25,6 @@ void valid_root_signature_6() {} [RootSignature("DescriptorTable(CBV(b0, offset = 4294967292), CBV(b1, numDescriptors = 3))")] void valid_root_signature_7() {} + +[RootSignature("DescriptorTable(CBV(b0, offset = 4294967294), CBV(b1))")] +void valid_root_signature_8() {} >From 7c5915c43986015ed013a0b1e1a0bd0b2e9511d5 Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Wed, 17 Sep 2025 16:04:02 -0700 Subject: [PATCH 2/8] remove unused function --- llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h | 1 - llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp | 4 ---- 2 files changed, 5 deletions(-) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h index ea96094b18300..45353142267a6 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h @@ -38,7 +38,6 @@ LLVM_ABI bool verifyMipLODBias(float MipLODBias); LLVM_ABI bool verifyMaxAnisotropy(uint32_t MaxAnisotropy); LLVM_ABI bool verifyLOD(float LOD); -LLVM_ABI bool verifyBoundOffset(uint32_t Offset); LLVM_ABI bool verifyNoOverflowedOffset(uint64_t Offset); LLVM_ABI uint64_t computeRangeBound(uint32_t Offset, uint32_t Size); diff --git a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp index 0970977b5064f..f3ea1698f3bf8 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp @@ -125,10 +125,6 @@ bool verifyMaxAnisotropy(uint32_t MaxAnisotropy) { bool verifyLOD(float LOD) { return !std::isnan(LOD); } -bool verifyBoundOffset(uint32_t Offset) { - return Offset != NumDescriptorsUnbounded; -} - bool verifyNoOverflowedOffset(uint64_t Offset) { return Offset <= std::numeric_limits<uint32_t>::max(); } >From 29da77aead335b06fd37df835595013e5dfd6002 Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Wed, 17 Sep 2025 16:15:09 -0700 Subject: [PATCH 3/8] self-review: remove redundant set --- clang/lib/Sema/SemaHLSL.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 33edd7f9a916a..afca31ba8440b 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1382,7 +1382,6 @@ bool SemaHLSL::handleRootSignatureElements( llvm::hlsl::rootsig::DescriptorTableOffsetAppend) { // Manually specified the offset Offset = Clause->Offset; - Unbound = false; } else if (Unbound) { // Trying to append onto unbound offset Diag(Loc, diag::err_hlsl_appending_onto_unbound); >From 8fe5b16b9b0521e05b53584ba0de3250e24360db Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Thu, 18 Sep 2025 11:09:26 -0700 Subject: [PATCH 4/8] review: add test gap --- clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl index 5ce4419149592..cb2569f380b2c 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -146,3 +146,7 @@ void offset_overflow() {} // expected-error@+1 {{descriptor range offset overflows [4294967295, 4294967296]}} [RootSignature("DescriptorTable(CBV(b0, offset = 4294967294), CBV(b1, numDescriptors = 2))")] void appended_offset_overflow() {} + +// expected-error@+1 {{descriptor range offset overflows [4294967296, 4294967296]}} +[RootSignature("DescriptorTable(CBV(b0, offset = 4294967294), CBV(b1), CBV(b2))")] +void multiple_appended_offset_overflow() {} >From b5f608d00c6513c642f02501e58a011069d7d4a9 Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Thu, 18 Sep 2025 11:12:17 -0700 Subject: [PATCH 5/8] review: fixes problematic uint32_t truncation - this fixes the added test gap --- clang/lib/Sema/SemaHLSL.cpp | 6 ++---- llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h | 2 +- llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index afca31ba8440b..3014f263b7100 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1360,7 +1360,7 @@ bool SemaHLSL::handleRootSignatureElements( "Number of unbound elements must match the number of clauses"); bool HasAnySampler = false; bool HasAnyNonSampler = false; - uint32_t Offset = 0; + uint64_t Offset = 0; bool Unbound = false; for (const auto &[Clause, ClauseElem] : UnboundClauses) { SourceLocation Loc = ClauseElem->getLocation(); @@ -1395,9 +1395,7 @@ bool SemaHLSL::handleRootSignatureElements( Diag(Loc, diag::err_hlsl_offset_overflow) << Offset << RangeBound; } - Offset = RangeBound == llvm::hlsl::rootsig::NumDescriptorsUnbounded - ? uint32_t(RangeBound) - : uint32_t(RangeBound + 1); + Offset = RangeBound + 1; Unbound = Clause->NumDescriptors == llvm::hlsl::rootsig::NumDescriptorsUnbounded; diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h index 45353142267a6..49c5967aebd3e 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h @@ -39,7 +39,7 @@ LLVM_ABI bool verifyMaxAnisotropy(uint32_t MaxAnisotropy); LLVM_ABI bool verifyLOD(float LOD); LLVM_ABI bool verifyNoOverflowedOffset(uint64_t Offset); -LLVM_ABI uint64_t computeRangeBound(uint32_t Offset, uint32_t Size); +LLVM_ABI uint64_t computeRangeBound(uint64_t Offset, uint32_t Size); } // namespace rootsig } // namespace hlsl diff --git a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp index f3ea1698f3bf8..c2c3bf6d1b8dc 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp @@ -129,12 +129,12 @@ bool verifyNoOverflowedOffset(uint64_t Offset) { return Offset <= std::numeric_limits<uint32_t>::max(); } -uint64_t computeRangeBound(uint32_t Offset, uint32_t Size) { +uint64_t computeRangeBound(uint64_t Offset, uint32_t Size) { assert(0 < Size && "Must be a non-empty range"); if (Size == NumDescriptorsUnbounded) return NumDescriptorsUnbounded; - return uint64_t(Offset) + uint64_t(Size) - 1; + return Offset + uint64_t(Size) - 1; } } // namespace rootsig >From a7741fb6cab8c2d33f9f720b6a28f5f3859c35b7 Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Thu, 18 Sep 2025 11:14:49 -0700 Subject: [PATCH 6/8] fixes outputting multiple errors in the unbound append case --- clang/lib/Sema/SemaHLSL.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 3014f263b7100..24745ed6c3cb1 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1378,26 +1378,23 @@ bool SemaHLSL::handleRootSignatureElements( if (Clause->NumDescriptors == 0) return true; - if (Clause->Offset != - llvm::hlsl::rootsig::DescriptorTableOffsetAppend) { - // Manually specified the offset + bool IsAppending = + Clause->Offset == llvm::hlsl::rootsig::DescriptorTableOffsetAppend; + if (!IsAppending) Offset = Clause->Offset; - } else if (Unbound) { - // Trying to append onto unbound offset - Diag(Loc, diag::err_hlsl_appending_onto_unbound); - } uint64_t RangeBound = llvm::hlsl::rootsig::computeRangeBound( Offset, Clause->NumDescriptors); - if (!llvm::hlsl::rootsig::verifyNoOverflowedOffset(RangeBound)) { - // Upper bound overflows maximum offset + if (Unbound && IsAppending) + Diag(Loc, diag::err_hlsl_appending_onto_unbound); + else if (!llvm::hlsl::rootsig::verifyNoOverflowedOffset(RangeBound)) Diag(Loc, diag::err_hlsl_offset_overflow) << Offset << RangeBound; - } + // Update offset to be 1 past this range's bound Offset = RangeBound + 1; Unbound = Clause->NumDescriptors == - llvm::hlsl::rootsig::NumDescriptorsUnbounded; + llvm::hlsl::rootsig::NumDescriptorsUnbounded; // Compute the register bounds and track resource binding uint32_t LowerBound(Clause->Reg.Number); >From a927e8ad82e2403ef83d4cf42674d8f3ba1ddab6 Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Thu, 18 Sep 2025 11:10:15 -0700 Subject: [PATCH 7/8] nfc, review: small clean up to reuse function --- clang/lib/Sema/SemaHLSL.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 24745ed6c3cb1..2e2112ed20c88 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1398,9 +1398,8 @@ bool SemaHLSL::handleRootSignatureElements( // Compute the register bounds and track resource binding uint32_t LowerBound(Clause->Reg.Number); - uint32_t UpperBound = Clause->NumDescriptors == ~0u - ? ~0u - : LowerBound + Clause->NumDescriptors - 1; + uint32_t UpperBound = llvm::hlsl::rootsig::computeRangeBound( + LowerBound, Clause->NumDescriptors); BindingChecker.trackBinding( Table->Visibility, >From ec92c4805bc212abbe4bb3ad514f08076c089f7e Mon Sep 17 00:00:00 2001 From: Finn Plummer <m...@inbelic.dev> Date: Thu, 18 Sep 2025 11:18:49 -0700 Subject: [PATCH 8/8] clang-format --- clang/lib/Sema/SemaHLSL.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 2e2112ed20c88..aae66dc81c1a9 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1394,7 +1394,7 @@ bool SemaHLSL::handleRootSignatureElements( // Update offset to be 1 past this range's bound Offset = RangeBound + 1; Unbound = Clause->NumDescriptors == - llvm::hlsl::rootsig::NumDescriptorsUnbounded; + llvm::hlsl::rootsig::NumDescriptorsUnbounded; // Compute the register bounds and track resource binding uint32_t LowerBound(Clause->Reg.Number); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits