Skip to content

Commit c06f354

Browse files
joaosaffranjoaosaffranJoao Saffranjoaosaffran-zz
authored
[DirectX] Adding missing descriptor table validations (llvm#153276)
This patch adds 2 small validation to DirectX backend. First, it checks if registers in descriptor tables are not overflowing, meaning they don't try to bind registers over the maximum allowed value, this is checked both on the offset and on the number of descriptors inside the range; second, it checks if samplers are being mixed with other resource types. Closes: llvm#153057, llvm#153058 --------- Co-authored-by: joaosaffran <joao.saffran@microsoft.com> Co-authored-by: Joao Saffran <{ID}+{username}@users.noreply.github.com> Co-authored-by: Joao Saffran <jderezende@microsoft.com>
1 parent d2239fb commit c06f354

11 files changed

+293
-35
lines changed

‎llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,91 @@ class RootSignatureValidationError
4848
}
4949
};
5050

51+
class OffsetAppendAfterOverflow : public ErrorInfo<OffsetAppendAfterOverflow> {
52+
public:
53+
static char ID;
54+
dxil::ResourceClass Type;
55+
uint32_t Register;
56+
uint32_t Space;
57+
58+
OffsetAppendAfterOverflow(dxil::ResourceClass Type, uint32_t Register,
59+
uint32_t Space)
60+
: Type(Type), Register(Register), Space(Space) {}
61+
62+
void log(raw_ostream &OS) const override {
63+
OS << "Range " << getResourceClassName(Type) << "(register=" << Register
64+
<< ", space=" << Space << ") "
65+
<< "cannot be appended after an unbounded range ";
66+
}
67+
68+
std::error_code convertToErrorCode() const override {
69+
return llvm::inconvertibleErrorCode();
70+
}
71+
};
72+
73+
class ShaderRegisterOverflowError
74+
: public ErrorInfo<ShaderRegisterOverflowError> {
75+
public:
76+
static char ID;
77+
dxil::ResourceClass Type;
78+
uint32_t Register;
79+
uint32_t Space;
80+
81+
ShaderRegisterOverflowError(dxil::ResourceClass Type, uint32_t Register,
82+
uint32_t Space)
83+
: Type(Type), Register(Register), Space(Space) {}
84+
85+
void log(raw_ostream &OS) const override {
86+
OS << "Overflow for shader register range: " << getResourceClassName(Type)
87+
<< "(register=" << Register << ", space=" << Space << ").";
88+
}
89+
90+
std::error_code convertToErrorCode() const override {
91+
return llvm::inconvertibleErrorCode();
92+
}
93+
};
94+
95+
class OffsetOverflowError : public ErrorInfo<OffsetOverflowError> {
96+
public:
97+
static char ID;
98+
dxil::ResourceClass Type;
99+
uint32_t Register;
100+
uint32_t Space;
101+
102+
OffsetOverflowError(dxil::ResourceClass Type, uint32_t Register,
103+
uint32_t Space)
104+
: Type(Type), Register(Register), Space(Space) {}
105+
106+
void log(raw_ostream &OS) const override {
107+
OS << "Offset overflow for descriptor range: " << getResourceClassName(Type)
108+
<< "(register=" << Register << ", space=" << Space << ").";
109+
}
110+
111+
std::error_code convertToErrorCode() const override {
112+
return llvm::inconvertibleErrorCode();
113+
}
114+
};
115+
116+
class TableSamplerMixinError : public ErrorInfo<TableSamplerMixinError> {
117+
public:
118+
static char ID;
119+
dxil::ResourceClass Type;
120+
uint32_t Location;
121+
122+
TableSamplerMixinError(dxil::ResourceClass Type, uint32_t Location)
123+
: Type(Type), Location(Location) {}
124+
125+
void log(raw_ostream &OS) const override {
126+
OS << "Samplers cannot be mixed with other "
127+
<< "resource types in a descriptor table, " << getResourceClassName(Type)
128+
<< "(location=" << Location << ")";
129+
}
130+
131+
std::error_code convertToErrorCode() const override {
132+
return llvm::inconvertibleErrorCode();
133+
}
134+
};
135+
51136
class GenericRSMetadataError : public ErrorInfo<GenericRSMetadataError> {
52137
public:
53138
LLVM_ABI static char ID;

‎llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ namespace rootsig {
2727
char GenericRSMetadataError::ID;
2828
char InvalidRSMetadataFormat::ID;
2929
char InvalidRSMetadataValue::ID;
30+
char TableSamplerMixinError::ID;
31+
char ShaderRegisterOverflowError::ID;
32+
char OffsetOverflowError::ID;
33+
char OffsetAppendAfterOverflow::ID;
34+
3035
template <typename T> char RootSignatureValidationError<T>::ID;
3136

3237
static std::optional<uint32_t> extractMdIntValue(MDNode *Node,
@@ -55,8 +60,9 @@ static std::optional<StringRef> extractMdStringValue(MDNode *Node,
5560
template <typename T, typename = std::enable_if_t<
5661
std::is_enum_v<T> &&
5762
std::is_same_v<std::underlying_type_t<T>, uint32_t>>>
58-
Expected<T> extractEnumValue(MDNode *Node, unsigned int OpId, StringRef ErrText,
59-
llvm::function_ref<bool(uint32_t)> VerifyFn) {
63+
static Expected<T>
64+
extractEnumValue(MDNode *Node, unsigned int OpId, StringRef ErrText,
65+
llvm::function_ref<bool(uint32_t)> VerifyFn) {
6066
if (std::optional<uint32_t> Val = extractMdIntValue(Node, OpId)) {
6167
if (!VerifyFn(*Val))
6268
return make_error<RootSignatureValidationError<uint32_t>>(ErrText, *Val);
@@ -538,6 +544,60 @@ Error MetadataParser::parseRootSignatureElement(mcdxbc::RootSignatureDesc &RSD,
538544
llvm_unreachable("Unhandled RootSignatureElementKind enum.");
539545
}
540546

547+
static Error
548+
validateDescriptorTableSamplerMixin(const mcdxbc::DescriptorTable &Table,
549+
uint32_t Location) {
550+
dxil::ResourceClass CurrRC = dxil::ResourceClass::Sampler;
551+
for (const mcdxbc::DescriptorRange &Range : Table.Ranges) {
552+
if (Range.RangeType == dxil::ResourceClass::Sampler &&
553+
CurrRC != dxil::ResourceClass::Sampler)
554+
return make_error<TableSamplerMixinError>(CurrRC, Location);
555+
CurrRC = Range.RangeType;
556+
}
557+
return Error::success();
558+
}
559+
560+
static Error
561+
validateDescriptorTableRegisterOverflow(const mcdxbc::DescriptorTable &Table,
562+
uint32_t Location) {
563+
uint64_t Offset = 0;
564+
bool IsPrevUnbound = false;
565+
for (const mcdxbc::DescriptorRange &Range : Table.Ranges) {
566+
// Validation of NumDescriptors should have happened by this point.
567+
if (Range.NumDescriptors == 0)
568+
continue;
569+
570+
const uint64_t RangeBound = llvm::hlsl::rootsig::computeRangeBound(
571+
Range.BaseShaderRegister, Range.NumDescriptors);
572+
573+
if (!verifyNoOverflowedOffset(RangeBound))
574+
return make_error<ShaderRegisterOverflowError>(
575+
Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
576+
577+
bool IsAppending =
578+
Range.OffsetInDescriptorsFromTableStart == DescriptorTableOffsetAppend;
579+
if (!IsAppending)
580+
Offset = Range.OffsetInDescriptorsFromTableStart;
581+
582+
if (IsPrevUnbound && IsAppending)
583+
return make_error<OffsetAppendAfterOverflow>(
584+
Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
585+
586+
const uint64_t OffsetBound =
587+
llvm::hlsl::rootsig::computeRangeBound(Offset, Range.NumDescriptors);
588+
589+
if (!verifyNoOverflowedOffset(OffsetBound))
590+
return make_error<OffsetOverflowError>(
591+
Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
592+
593+
Offset = OffsetBound + 1;
594+
IsPrevUnbound =
595+
Range.NumDescriptors == llvm::hlsl::rootsig::NumDescriptorsUnbounded;
596+
}
597+
598+
return Error::success();
599+
}
600+
541601
Error MetadataParser::validateRootSignature(
542602
const mcdxbc::RootSignatureDesc &RSD) {
543603
Error DeferredErrs = Error::success();
@@ -611,6 +671,14 @@ Error MetadataParser::validateRootSignature(
611671
joinErrors(std::move(DeferredErrs),
612672
make_error<RootSignatureValidationError<uint32_t>>(
613673
"DescriptorFlag", Range.Flags));
674+
675+
if (Error Err =
676+
validateDescriptorTableSamplerMixin(Table, Info.Location))
677+
DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err));
678+
679+
if (Error Err =
680+
validateDescriptorTableRegisterOverflow(Table, Info.Location))
681+
DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err));
614682
}
615683
break;
616684
}

‎llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ uint64_t computeRangeBound(uint64_t Offset, uint32_t Size) {
136136

137137
return Offset + uint64_t(Size) - 1;
138138
}
139-
140139
} // namespace rootsig
141140
} // namespace hlsl
142141
} // namespace llvm

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1111

1212
!dx.rootsignatures = !{!2} ; list of function/root signature pairs
1313
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
14-
!3 = !{ !5 } ; list of root signature elements
15-
!5 = !{ !"DescriptorTable", i32 0, !6, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20 }
14+
!3 = !{ !5, !21 } ; list of root signature elements
15+
!5 = !{ !"DescriptorTable", i32 0, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20 }
16+
!21 = !{ !"DescriptorTable", i32 0, !6, !8, !9 }
1617

1718
; typedef enum D3D12_DESCRIPTOR_RANGE_FLAGS {
1819
; NONE = 0,
@@ -53,37 +54,20 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
5354
!20 = !{ !"UAV", i32 5, i32 1, i32 15, i32 5, i32 65540 }
5455

5556
;DXC:- Name: RTS0
56-
;DXC-NEXT: Size: 380
57+
;DXC-NEXT: Size: 400
5758
;DXC-NEXT: RootSignature:
5859
;DXC-NEXT: Version: 2
59-
;DXC-NEXT: NumRootParameters: 1
60+
;DXC-NEXT: NumRootParameters: 2
6061
;DXC-NEXT: RootParametersOffset: 24
6162
;DXC-NEXT: NumStaticSamplers: 0
62-
;DXC-NEXT: StaticSamplersOffset: 380
63+
;DXC-NEXT: StaticSamplersOffset: 400
6364
;DXC-NEXT: Parameters:
6465
;DXC-NEXT: - ParameterType: DescriptorTable
6566
;DXC-NEXT: ShaderVisibility: All
6667
;DXC-NEXT: Table:
67-
;DXC-NEXT: NumRanges: 14
68-
;DXC-NEXT: RangesOffset: 44
68+
;DXC-NEXT: NumRanges: 11
69+
;DXC-NEXT: RangesOffset: 56
6970
;DXC-NEXT: Ranges:
70-
;DXC-NEXT: - RangeType: Sampler
71-
;DXC-NEXT: NumDescriptors: 1
72-
;DXC-NEXT: BaseShaderRegister: 0
73-
;DXC-NEXT: RegisterSpace: 1
74-
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
75-
;DXC-NEXT: - RangeType: Sampler
76-
;DXC-NEXT: NumDescriptors: 1
77-
;DXC-NEXT: BaseShaderRegister: 0
78-
;DXC-NEXT: RegisterSpace: 3
79-
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
80-
;DXC-NEXT: DESCRIPTORS_VOLATILE: true
81-
;DXC-NEXT: - RangeType: Sampler
82-
;DXC-NEXT: NumDescriptors: 1
83-
;DXC-NEXT: BaseShaderRegister: 0
84-
;DXC-NEXT: RegisterSpace: 4
85-
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
86-
;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
8771
;DXC-NEXT: - RangeType: SRV
8872
;DXC-NEXT: NumDescriptors: 1
8973
;DXC-NEXT: BaseShaderRegister: 0
@@ -155,3 +139,26 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
155139
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
156140
;DXC-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true
157141
;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
142+
;DXC-NEXT: - ParameterType: DescriptorTable
143+
;DXC-NEXT: ShaderVisibility: All
144+
;DXC-NEXT: Table:
145+
;DXC-NEXT: NumRanges: 3
146+
;DXC-NEXT: RangesOffset: 328
147+
;DXC-NEXT: Ranges:
148+
;DXC-NEXT: - RangeType: Sampler
149+
;DXC-NEXT: NumDescriptors: 1
150+
;DXC-NEXT: BaseShaderRegister: 0
151+
;DXC-NEXT: RegisterSpace: 1
152+
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
153+
;DXC-NEXT: - RangeType: Sampler
154+
;DXC-NEXT: NumDescriptors: 1
155+
;DXC-NEXT: BaseShaderRegister: 0
156+
;DXC-NEXT: RegisterSpace: 3
157+
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
158+
;DXC-NEXT: DESCRIPTORS_VOLATILE: true
159+
;DXC-NEXT: - RangeType: Sampler
160+
;DXC-NEXT: NumDescriptors: 1
161+
;DXC-NEXT: BaseShaderRegister: 0
162+
;DXC-NEXT: RegisterSpace: 4
163+
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
164+
;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,40 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1111

1212
!dx.rootsignatures = !{!2} ; list of function/root signature pairs
1313
!2 = !{ ptr @main, !3, i32 1 } ; function, root signature
14-
!3 = !{ !5 } ; list of root signature elements
15-
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
14+
!3 = !{ !5, !8 } ; list of root signature elements
15+
!5 = !{ !"DescriptorTable", i32 0, !6 }
1616
!6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
17+
!8 = !{ !"DescriptorTable", i32 0, !7 }
1718
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
1819

1920

2021
; DXC: - Name: RTS0
21-
; DXC-NEXT: Size: 84
22+
; DXC-NEXT: Size: 104
2223
; DXC-NEXT: RootSignature:
2324
; DXC-NEXT: Version: 1
24-
; DXC-NEXT: NumRootParameters: 1
25+
; DXC-NEXT: NumRootParameters: 2
2526
; DXC-NEXT: RootParametersOffset: 24
2627
; DXC-NEXT: NumStaticSamplers: 0
27-
; DXC-NEXT: StaticSamplersOffset: 84
28+
; DXC-NEXT: StaticSamplersOffset: 104
2829
; DXC-NEXT: Parameters:
2930
; DXC-NEXT: - ParameterType: DescriptorTable
3031
; DXC-NEXT: ShaderVisibility: All
3132
; DXC-NEXT: Table:
32-
; DXC-NEXT: NumRanges: 2
33-
; DXC-NEXT: RangesOffset: 44
33+
; DXC-NEXT: NumRanges: 1
34+
; DXC-NEXT: RangesOffset: 56
3435
; DXC-NEXT: Ranges:
3536
; DXC-NEXT: - RangeType: Sampler
3637
; DXC-NEXT: NumDescriptors: 1
3738
; DXC-NEXT: BaseShaderRegister: 1
3839
; DXC-NEXT: RegisterSpace: 0
3940
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
40-
; DXC-NEXT: - RangeType: UAV
41+
; DXC-NEXT: - ParameterType: DescriptorTable
42+
; DXC-NEXT: ShaderVisibility: All
43+
; DXC-NEXT: Table:
44+
; DXC-NEXT: NumRanges: 1
45+
; DXC-NEXT: RangesOffset: 84
46+
; DXC-NEXT: Ranges:
47+
; DXC-NEXT: - RangeType: UAV
4148
; DXC-NEXT: NumDescriptors: 5
4249
; DXC-NEXT: BaseShaderRegister: 1
4350
; DXC-NEXT: RegisterSpace: 10

‎llvm/test/CodeGen/DirectX/rootsignature-validation-appending-limits.ll‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
; RUN: opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
2+
; A descriptor range can be placed at UINT_MAX, matching DXC's behaviour
3+
; CHECK-NOT: error:
4+
5+
define void @CSMain() "hlsl.shader"="compute" {
6+
entry:
7+
ret void
8+
}
9+
10+
!dx.rootsignatures = !{!0}
11+
12+
!0 = !{ptr @CSMain, !1, i32 2}
13+
!1 = !{!3}
14+
!3 = !{!"DescriptorTable", i32 0, !4, !5}
15+
!4 = !{!"UAV", i32 1, i32 1, i32 0, i32 4294967294, i32 0}
16+
!5 = !{!"UAV", i32 1, i32 0, i32 0, i32 -1, i32 0}

‎llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-limits-multiples.ll‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
2+
; CHECK: error: Offset overflow for descriptor range: CBV(register=2, space=0).
3+
4+
define void @CSMain() "hlsl.shader"="compute" {
5+
entry:
6+
ret void
7+
}
8+
9+
!dx.rootsignatures = !{!0}
10+
11+
!0 = !{ptr @CSMain, !1, i32 2}
12+
!1 = !{!3}
13+
!3 = !{!"DescriptorTable", i32 0, !4, !5, !6}
14+
!4 = !{!"CBV", i32 1, i32 0, i32 0, i32 4294967294, i32 0}
15+
!5 = !{!"CBV", i32 1, i32 1, i32 0, i32 -1, i32 0}
16+
!6 = !{!"CBV", i32 1, i32 2, i32 0, i32 -1, i32 0}

‎llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
2+
; This test checks if a resource is implicitly overflowing. That means, it is appending a resource after an unbounded range.
3+
4+
; CHECK: error: Range UAV(register=0, space=0) cannot be appended after an unbounded range
5+
6+
define void @CSMain() "hlsl.shader"="compute" {
7+
entry:
8+
ret void
9+
}
10+
11+
!dx.rootsignatures = !{!0}
12+
13+
!0 = !{ptr @CSMain, !1, i32 2}
14+
!1 = !{!3}
15+
!3 = !{!"DescriptorTable", i32 0, !4, !5}
16+
!4 = !{!"UAV", i32 -1, i32 1, i32 0, i32 2, i32 0}
17+
!5 = !{!"UAV", i32 1, i32 0, i32 0, i32 -1, i32 0}

‎llvm/test/CodeGen/DirectX/rootsignature-validation-fail-offset-overflow.ll‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
2+
; CHECK: error: Offset overflow for descriptor range: UAV(register=0, space=0).
3+
4+
define void @CSMain() "hlsl.shader"="compute" {
5+
entry:
6+
ret void
7+
}
8+
9+
!dx.rootsignatures = !{!0}
10+
11+
!0 = !{ptr @CSMain, !1, i32 2}
12+
!1 = !{!3}
13+
!3 = !{!"DescriptorTable", i32 0, !4, !5}
14+
!4 = !{!"UAV", i32 100, i32 0, i32 0, i32 4294967294, i32 0}
15+
!5 = !{!"UAV", i32 1, i32 101, i32 0, i32 10, i32 0}

0 commit comments

Comments
 (0)