[clang] [llvm] [SemaHLSL] Correct descriptor range overflow validation (PR #159475)
Finn Plummer via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 18 11:22:30 PDT 2025
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 <mail at 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 at +1 {{descriptor range offset overflows [4294967292, 4294967296]}}
[RootSignature("DescriptorTable(CBV(b0, offset = 4294967292, numDescriptors = 5))")]
-void offset_() {}
+void offset_overflow() {}
+
+// expected-error at +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 <mail at 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 <mail at 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 <mail at 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 at +1 {{descriptor range offset overflows [4294967295, 4294967296]}}
[RootSignature("DescriptorTable(CBV(b0, offset = 4294967294), CBV(b1, numDescriptors = 2))")]
void appended_offset_overflow() {}
+
+// expected-error at +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 <mail at 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 <mail at 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 <mail at 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 <mail at 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);
More information about the cfe-commits
mailing list