Skip to content

Commit 9f5b023

Browse files
authored
[HLSL] Make sure global resources and resource arrays cannot be assigned to (#157772)
Global resources are read-only. The compiler needs to report an error when somebody attempts to assign a value to a global resource, a global resource array element or the whole array. Test update in `static-local-ctor.hlsl` includes the use of the llvm-cxxfilt tool which takes care of demangling of function names for a more readable test baseline. Closes #154390
1 parent 54c5521 commit 9f5b023

File tree

6 files changed

+70
-3
lines changed

6 files changed

+70
-3
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13223,6 +13223,8 @@ def note_hlsl_resource_range_here: Note<"overlapping resource range here">;
1322313223

1322413224
def err_hlsl_incomplete_resource_array_in_function_param: Error<
1322513225
"incomplete resource array in a function parameter">;
13226+
def err_hlsl_assign_to_global_resource: Error<
13227+
"assignment to global resource variable %0 is not allowed">;
1322613228

1322713229
// Layout randomization diagnostics.
1322813230
def err_non_designated_init_used : Error<

clang/include/clang/Sema/SemaHLSL.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ class SemaHLSL : public SemaBase {
133133
bool isSemanticValid(FunctionDecl *FD, DeclaratorDecl *D);
134134
void CheckSemanticAnnotation(FunctionDecl *EntryPoint, const Decl *Param,
135135
const HLSLAnnotationAttr *AnnotationAttr);
136+
bool CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr, Expr *RHSExpr,
137+
SourceLocation Loc);
136138
void DiagnoseAttrStageMismatch(
137139
const Attr *A, llvm::Triple::EnvironmentType Stage,
138140
std::initializer_list<llvm::Triple::EnvironmentType> AllowedStages);

clang/lib/Sema/SemaExpr.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15711,6 +15711,12 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
1571115711
RHSExpr = resolvedRHS.get();
1571215712
}
1571315713

15714+
if (getLangOpts().HLSL && (LHSExpr->getType()->isHLSLResourceRecord() ||
15715+
LHSExpr->getType()->isHLSLResourceRecordArray())) {
15716+
if (!HLSL().CheckResourceBinOp(Opc, LHSExpr, RHSExpr, OpLoc))
15717+
return ExprError();
15718+
}
15719+
1571415720
if (getLangOpts().CPlusPlus) {
1571515721
// Otherwise, build an overloaded op if either expression is type-dependent
1571615722
// or has an overloadable type.

clang/lib/Sema/SemaHLSL.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3956,6 +3956,34 @@ bool SemaHLSL::ActOnUninitializedVarDecl(VarDecl *VD) {
39563956
return false;
39573957
}
39583958

3959+
// Return true if everything is ok; returns false if there was an error.
3960+
bool SemaHLSL::CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr,
3961+
Expr *RHSExpr, SourceLocation Loc) {
3962+
assert((LHSExpr->getType()->isHLSLResourceRecord() ||
3963+
LHSExpr->getType()->isHLSLResourceRecordArray()) &&
3964+
"expected LHS to be a resource record or array of resource records");
3965+
if (Opc != BO_Assign)
3966+
return true;
3967+
3968+
// If LHS is an array subscript, get the underlying declaration.
3969+
Expr *E = LHSExpr;
3970+
while (auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
3971+
E = ASE->getBase()->IgnoreParenImpCasts();
3972+
3973+
// Report error if LHS is a resource declared at a global scope.
3974+
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) {
3975+
if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
3976+
if (VD->hasGlobalStorage()) {
3977+
// assignment to global resource is not allowed
3978+
SemaRef.Diag(Loc, diag::err_hlsl_assign_to_global_resource) << VD;
3979+
SemaRef.Diag(VD->getLocation(), diag::note_var_declared_here) << VD;
3980+
return false;
3981+
}
3982+
}
3983+
}
3984+
return true;
3985+
}
3986+
39593987
// Walks though the global variable declaration, collects all resource binding
39603988
// requirements and adds them to Bindings
39613989
void SemaHLSL::collectResourceBindingsOnVarDecl(VarDecl *VD) {

clang/test/CodeGenHLSL/static-local-ctor.hlsl

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
RWBuffer<int> buf[10];
1010

11+
// CHECK: @[[main_mybuf:.*]] = internal global %"class.hlsl::RWBuffer" zeroinitializer, align 4
12+
// CHECK: @[[main_mybuf_guard:.*]] = internal global i8 0, align 1
13+
1114
void InitBuf(RWBuffer<int> buf) {
1215
for (unsigned int i = 0; i < 100; i++)
1316
buf[i] = 0;
@@ -24,15 +27,14 @@ void InitBuf(RWBuffer<int> buf) {
2427
// CHECK-NOT: _Init_thread_header
2528
// CHECK: init.check:
2629
// CHECK-NEXT: call void @hlsl::RWBuffer<int>::__createFromImplicitBinding
30+
// CHECK-NEXT: call void @hlsl::RWBuffer<int>::RWBuffer(hlsl::RWBuffer<int> const&)(ptr {{.*}} @main()::mybuf, ptr {{.*}}) #
2731
// CHECK-NEXT: store i8 1, ptr @guard variable for main()::mybuf
2832
// CHECK-NOT: _Init_thread_footer
2933

30-
3134
[shader("compute")]
3235
[numthreads(1,1,1)]
3336
void main() {
3437
// A non-trivially constructed static local will get checks to verify that it is generated just once
35-
static RWBuffer<int> mybuf;
36-
mybuf = buf[0];
38+
static RWBuffer<int> mybuf = buf[0];
3739
InitBuf(mybuf);
3840
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -o - -fsyntax-only %s -verify
2+
3+
RWBuffer<float> A[10]; // expected-note {{variable 'A' is declared here}} expected-note {{variable 'A' is declared here}} // expected-note {{variable 'A' is declared here}}
4+
RWBuffer<float> B; // expected-note {{variable 'B' is declared here}}
5+
RWBuffer<float> C[10];
6+
7+
void test() {
8+
// expected-error@+1{{assignment to global resource variable 'A' is not allowed}}
9+
A = C;
10+
11+
// expected-error@+1{{assignment to global resource variable 'B' is not allowed}}
12+
B = C[0];
13+
14+
// expected-error@+1{{assignment to global resource variable 'A' is not allowed}}
15+
A[1] = B;
16+
17+
// expected-error@+1{{assignment to global resource variable 'A' is not allowed}}
18+
A[1] = C[2];
19+
20+
// local resources are assignable
21+
RWBuffer<float> LocalA[10] = A; // ok
22+
RWBuffer<float> LocalB = B; // ok
23+
24+
// read-write resources can be written to
25+
A[0][0] = 1.0f; // ok
26+
B[0] = 2.0f; // ok
27+
}

0 commit comments

Comments
 (0)