[clang] [llvm] [SemaHLSL] Correct descriptor range overflow validation (PR #159475)

[Tex Riddell via cfe-commits]

================
@@ -1381,22 +1382,24 @@ bool SemaHLSL::handleRootSignatureElements(
             llvm::hlsl::rootsig::DescriptorTableOffsetAppend) {
           // Manually specified the offset
           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::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;
         }
 
         Offset = RangeBound == llvm::hlsl::rootsig::NumDescriptorsUnbounded
                      ? uint32_t(RangeBound)
                      : uint32_t(RangeBound + 1);
+        Unbound = Clause->NumDescriptors ==
+                  llvm::hlsl::rootsig::NumDescriptorsUnbounded;
----------------
tex3d wrote:

The code in this vicinity does seem to have a few minor nits that make it harder to reason about:
- Offset will be set to the next available offset, unless that's UINT_MAX, so could there be a bug where a prior not-unbounded Clause ends at UINT_MAX, and now the next Clause of size 1 appears to fit starting at UINT_MAX? I think Offset should be a `uint64_t` and always point to the next location. Otherwise, every value for a `uint32_t Offset` is a valid location for one descriptor to fit. If that's larger than UINT_MAX, the next one won't fit.
- Inconsistent use of `llvm::hlsl::rootsig::NumDescriptorsUnbounded` vs. `~0u` when comparing `Clause->NumDescriptors`. We should use the same `llvm::hlsl::rootsig::NumDescriptorsUnbounded` everywhere.
- You use `llvm::hlsl::rootsig::computeRangeBound` to compute upper bound elsewhere, but you duplicate that logic to initialize `UpperBound` below. Why not use `computeRangeBound` for `UpperBound` too?

https://github.com/llvm/llvm-project/pull/159475