Skip to content

Commit 4ce34f1

Browse files
V-FEXrtV-FEXrt
authored
[HLSL] Disallow writing to readonly resources (#147806)
Fixes #141842 Only add the non-const subscript operator to write resources
1 parent 8c1b516 commit 4ce34f1

File tree

6 files changed

+53
-37
lines changed

6 files changed

+53
-37
lines changed

‎clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,9 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addArraySubscriptOperators() {
697697
AST.DeclarationNames.getCXXOperatorName(OO_Subscript);
698698

699699
addHandleAccessFunction(Subscript, /*IsConst=*/true, /*IsRef=*/true);
700-
addHandleAccessFunction(Subscript, /*IsConst=*/false, /*IsRef=*/true);
700+
if (getResourceAttrs().ResourceClass == llvm::dxil::ResourceClass::UAV)
701+
addHandleAccessFunction(Subscript, /*IsConst=*/false, /*IsRef=*/true);
702+
701703
return *this;
702704
}
703705

@@ -714,7 +716,7 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addLoadMethods() {
714716
return *this;
715717
}
716718

717-
FieldDecl *BuiltinTypeDeclBuilder::getResourceHandleField() {
719+
FieldDecl *BuiltinTypeDeclBuilder::getResourceHandleField() const {
718720
auto I = Fields.find("__handle");
719721
assert(I != Fields.end() &&
720722
I->second->getType()->isHLSLAttributedResourceType() &&
@@ -738,6 +740,12 @@ QualType BuiltinTypeDeclBuilder::getHandleElementType() {
738740
return SemaRef.getASTContext().Char8Ty;
739741
}
740742

743+
HLSLAttributedResourceType::Attributes
744+
BuiltinTypeDeclBuilder::getResourceAttrs() const {
745+
QualType HandleType = getResourceHandleField()->getType();
746+
return cast<HLSLAttributedResourceType>(HandleType)->getAttrs();
747+
}
748+
741749
// BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::startDefinition() {
742750
// assert(!Record->isCompleteDefinition() && "record is already complete");
743751
// Record->startDefinition();

‎clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,11 @@ class BuiltinTypeDeclBuilder {
9191
BuiltinTypeDeclBuilder &addConsumeMethod();
9292

9393
private:
94-
FieldDecl *getResourceHandleField();
94+
FieldDecl *getResourceHandleField() const;
9595
QualType getFirstTemplateTypeParam();
9696
QualType getHandleElementType();
9797
Expr *getConstantIntExpr(int value);
98+
HLSLAttributedResourceType::Attributes getResourceAttrs() const;
9899
};
99100

100101
} // namespace hlsl

‎clang/test/AST/HLSL/StructuredBuffers-AST.hlsl

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
//
1313
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump \
1414
// RUN: -DRESOURCE=RWStructuredBuffer %s | FileCheck -DRESOURCE=RWStructuredBuffer \
15-
// RUN: -check-prefixes=CHECK,CHECK-UAV,CHECK-SUBSCRIPT,CHECK-COUNTER,CHECK-LOAD %s
15+
// RUN: -check-prefixes=CHECK,CHECK-UAV,CHECK-SUBSCRIPT,CHECK-SUBSCRIPT-UAV,CHECK-COUNTER,CHECK-LOAD %s
1616
//
1717
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -DEMPTY \
1818
// RUN: -DRESOURCE=AppendStructuredBuffer %s | FileCheck -DRESOURCE=AppendStructuredBuffer \
@@ -36,7 +36,7 @@
3636
//
3737
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump \
3838
// RUN: -DRESOURCE=RasterizerOrderedStructuredBuffer %s | FileCheck -DRESOURCE=RasterizerOrderedStructuredBuffer \
39-
// RUN: -check-prefixes=CHECK,CHECK-UAV,CHECK-ROV,CHECK-SUBSCRIPT,CHECK-LOAD %s
39+
// RUN: -check-prefixes=CHECK,CHECK-UAV,CHECK-ROV,CHECK-SUBSCRIPT,CHECK-SUBSCRIPT-UAV,CHECK-LOAD %s
4040

4141
// This test tests two different AST generations for each structured buffer.
4242
// The "EMPTY" test mode verifies the AST generated by forward declaration
@@ -170,22 +170,22 @@ RESOURCE<float> Buffer;
170170
// CHECK-SUBSCRIPT-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'Index' 'unsigned int'
171171
// CHECK-SUBSCRIPT-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
172172

173-
// CHECK-SUBSCRIPT-NEXT: CXXMethodDecl {{.*}} operator[] 'hlsl_device element_type &(unsigned int)'
174-
// CHECK-SUBSCRIPT-NEXT: ParmVarDecl {{.*}} Index 'unsigned int'
175-
// CHECK-SUBSCRIPT-NEXT: CompoundStmt
176-
// CHECK-SUBSCRIPT-NEXT: ReturnStmt
177-
// CHECK-SUBSCRIPT-NEXT: UnaryOperator {{.*}} 'hlsl_device element_type' prefix '*' cannot overflow
178-
// CHECK-SUBSCRIPT-NEXT: CallExpr {{.*}} 'hlsl_device element_type *'
179-
// CHECK-SUBSCRIPT-NEXT: ImplicitCastExpr {{.*}} <BuiltinFnToFnPtr>
180-
// CHECK-SUBSCRIPT-NEXT: DeclRefExpr {{.*}} '<builtin fn type>' Function {{.*}} '__builtin_hlsl_resource_getpointer' 'void (...) noexcept'
181-
// CHECK-SUBSCRIPT-NEXT: MemberExpr {{.*}} '__hlsl_resource_t
182-
// CHECK-SUBSCRIPT-SAME{LITERAL}: [[hlsl::resource_class(
183-
// CHECK-SUBSCRIPT-SAME{LITERAL}: [[hlsl::raw_buffer]]
184-
// CHECK-SUBSCRIPT-SAME{LITERAL}: [[hlsl::contained_type(element_type)]]
185-
// CHECK-SUBSCRIPT-SAME: ' lvalue .__handle {{.*}}
186-
// CHECK-SUBSCRIPT-NEXT: CXXThisExpr {{.*}} '[[RESOURCE]]<element_type>' lvalue implicit this
187-
// CHECK-SUBSCRIPT-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'Index' 'unsigned int'
188-
// CHECK-SUBSCRIPT-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
173+
// CHECK-SUBSCRIPT-UAV-NEXT: CXXMethodDecl {{.*}} operator[] 'hlsl_device element_type &(unsigned int)'
174+
// CHECK-SUBSCRIPT-UAV-NEXT: ParmVarDecl {{.*}} Index 'unsigned int'
175+
// CHECK-SUBSCRIPT-UAV-NEXT: CompoundStmt
176+
// CHECK-SUBSCRIPT-UAV-NEXT: ReturnStmt
177+
// CHECK-SUBSCRIPT-UAV-NEXT: UnaryOperator {{.*}} 'hlsl_device element_type' prefix '*' cannot overflow
178+
// CHECK-SUBSCRIPT-UAV-NEXT: CallExpr {{.*}} 'hlsl_device element_type *'
179+
// CHECK-SUBSCRIPT-UAV-NEXT: ImplicitCastExpr {{.*}} <BuiltinFnToFnPtr>
180+
// CHECK-SUBSCRIPT-UAV-NEXT: DeclRefExpr {{.*}} '<builtin fn type>' Function {{.*}} '__builtin_hlsl_resource_getpointer' 'void (...) noexcept'
181+
// CHECK-SUBSCRIPT-UAV-NEXT: MemberExpr {{.*}} '__hlsl_resource_t
182+
// CHECK-SUBSCRIPT-UAV-SAME{LITERAL}: [[hlsl::resource_class(
183+
// CHECK-SUBSCRIPT-UAV-SAME{LITERAL}: [[hlsl::raw_buffer]]
184+
// CHECK-SUBSCRIPT-UAV-SAME{LITERAL}: [[hlsl::contained_type(element_type)]]
185+
// CHECK-SUBSCRIPT-UAV-SAME: ' lvalue .__handle {{.*}}
186+
// CHECK-SUBSCRIPT-UAV-NEXT: CXXThisExpr {{.*}} '[[RESOURCE]]<element_type>' lvalue implicit this
187+
// CHECK-SUBSCRIPT-UAV-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'Index' 'unsigned int'
188+
// CHECK-SUBSCRIPT-UAV-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
189189

190190
// CHECK-NOSUBSCRIPT-NOT: CXXMethodDecl {{.*}} operator[] 'const hlsl_device element_type &(unsigned int) const'
191191
// CHECK-NOSUBSCRIPT-NOT: CXXMethodDecl {{.*}} operator[] 'hlsl_device element_type &(unsigned int)'

‎clang/test/AST/HLSL/TypedBuffers-AST.hlsl

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ RESOURCE<float> Buffer;
126126
// CHECK-NEXT: DeclRefExpr {{.*}} 'const char *' ParmVar {{.*}} 'name' 'const char *'
127127
// CHECK-NEXT: AlwaysInlineAttr
128128

129-
// Subsctript operators
129+
// Subscript operators
130130

131131
// CHECK: CXXMethodDecl {{.*}} operator[] 'const hlsl_device element_type &(unsigned int) const'
132132
// CHECK-NEXT: ParmVarDecl {{.*}} Index 'unsigned int'
@@ -145,22 +145,21 @@ RESOURCE<float> Buffer;
145145
// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'Index' 'unsigned int'
146146
// CHECK-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
147147

148-
// CHECK-NEXT: CXXMethodDecl {{.*}} operator[] 'hlsl_device element_type &(unsigned int)'
149-
// CHECK-NEXT: ParmVarDecl {{.*}} Index 'unsigned int'
150-
// CHECK-NEXT: CompoundStmt
151-
// CHECK-NEXT: ReturnStmt
152-
// CHECK-NEXT: UnaryOperator {{.*}} 'hlsl_device element_type' prefix '*' cannot overflow
153-
// CHECK-NEXT: CallExpr {{.*}} 'hlsl_device element_type *'
154-
// CHECK-NEXT: ImplicitCastExpr {{.*}} <BuiltinFnToFnPtr>
155-
// CHECK-NEXT: DeclRefExpr {{.*}} '<builtin fn type>' Function {{.*}} '__builtin_hlsl_resource_getpointer' 'void (...) noexcept'
156-
// CHECK-NEXT: MemberExpr {{.*}} '__hlsl_resource_t
148+
// CHECK-UAV-NEXT: CXXMethodDecl {{.*}} operator[] 'hlsl_device element_type &(unsigned int)'
149+
// CHECK-UAV-NEXT: ParmVarDecl {{.*}} Index 'unsigned int'
150+
// CHECK-UAV-NEXT: CompoundStmt
151+
// CHECK-UAV-NEXT: ReturnStmt
152+
// CHECK-UAV-NEXT: UnaryOperator {{.*}} 'hlsl_device element_type' prefix '*' cannot overflow
153+
// CHECK-UAV-NEXT: CallExpr {{.*}} 'hlsl_device element_type *'
154+
// CHECK-UAV-NEXT: ImplicitCastExpr {{.*}} <BuiltinFnToFnPtr>
155+
// CHECK-UAV-NEXT: DeclRefExpr {{.*}} '<builtin fn type>' Function {{.*}} '__builtin_hlsl_resource_getpointer' 'void (...) noexcept'
156+
// CHECK-UAV-NEXT: MemberExpr {{.*}} '__hlsl_resource_t
157157
// CHECK-UAV-SAME{LITERAL}: [[hlsl::resource_class(UAV)]]
158-
// CHECK-SRV-SAME{LITERAL}: [[hlsl::resource_class(SRV)]]
159-
// CHECK-SAME{LITERAL}: [[hlsl::contained_type(element_type)]]
160-
// CHECK-SAME: ' lvalue .__handle {{.*}}
161-
// CHECK-NEXT: CXXThisExpr {{.*}} '[[RESOURCE]]<element_type>' lvalue implicit this
162-
// CHECK-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'Index' 'unsigned int'
163-
// CHECK-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
158+
// CHECK-UAV-SAME{LITERAL}: [[hlsl::contained_type(element_type)]]
159+
// CHECK-UAV-SAME: ' lvalue .__handle {{.*}}
160+
// CHECK-UAV-NEXT: CXXThisExpr {{.*}} '[[RESOURCE]]<element_type>' lvalue implicit this
161+
// CHECK-UAV-NEXT: DeclRefExpr {{.*}} 'unsigned int' ParmVar {{.*}} 'Index' 'unsigned int'
162+
// CHECK-UAV-NEXT: AlwaysInlineAttr {{.*}} Implicit always_inline
164163

165164
// Load method
166165

‎clang/test/SemaHLSL/BuiltIns/Buffers.hlsl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,8 @@ Buffer<threeDoubles> BufferErr3;
111111
void main() {
112112
(void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::Buffer<vector<float, 3>>'}}
113113
// expected-note@* {{implicitly declared private here}}
114+
115+
// expected-error@+2 {{cannot assign to return value because function 'operator[]' returns a const value}}
116+
// expected-note@* {{function 'operator[]' which returns const-qualified type 'vector<float const hlsl_device &, 3>' declared here}}
117+
Buff[0] = 0.0;
114118
}

‎clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,8 @@ StructuredBuffer<Empty> BufferErr4;
2828
void main() {
2929
(void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::StructuredBuffer<vector<float, 3>>'}}
3030
// expected-note@* {{implicitly declared private here}}
31+
32+
// expected-error@+2 {{cannot assign to return value because function 'operator[]' returns a const value}}
33+
// expected-note@* {{function 'operator[]' which returns const-qualified type 'vector<float const hlsl_device &, 3>' declared here}}
34+
Buff[0] = 0.0;
3135
}

0 commit comments

Comments
 (0)