From 005b919b03642d465182c4a3486eec7bbb175539 Mon Sep 17 00:00:00 2001 From: Helena Kotas Date: Tue, 9 Sep 2025 17:29:56 -0700 Subject: [PATCH 1/2] [HLSL] Make sure global resources and resource arrays are not editable Closes #154390 --- .../clang/Basic/DiagnosticSemaKinds.td | 2 ++ clang/include/clang/Sema/SemaHLSL.h | 2 ++ clang/lib/Sema/SemaExpr.cpp | 6 ++++ clang/lib/Sema/SemaHLSL.cpp | 28 +++++++++++++++++++ clang/test/CodeGenHLSL/static-local-ctor.hlsl | 19 +++++++------ .../SemaHLSL/prohibit_resource_edits.hlsl | 27 ++++++++++++++++++ 6 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 clang/test/SemaHLSL/prohibit_resource_edits.hlsl diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 711efbe727892..591802bb294de 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13207,6 +13207,8 @@ def note_hlsl_resource_range_here: Note<"overlapping resource range here">; def err_hlsl_incomplete_resource_array_in_function_param: Error< "incomplete resource array in a function parameter">; +def err_hlsl_assign_to_global_resource: Error< + "assignment to global resource variable %0 is not allowed">; // Layout randomization diagnostics. def err_non_designated_init_used : Error< diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 5cbe1b658f5cd..41101b6e7a319 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -131,6 +131,8 @@ class SemaHLSL : public SemaBase { void CheckEntryPoint(FunctionDecl *FD); void CheckSemanticAnnotation(FunctionDecl *EntryPoint, const Decl *Param, const HLSLAnnotationAttr *AnnotationAttr); + bool CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr, Expr *RHSExpr, + SourceLocation Loc); void DiagnoseAttrStageMismatch( const Attr *A, llvm::Triple::EnvironmentType Stage, std::initializer_list AllowedStages); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 317b7caec6fb7..f50019957e420 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -15692,6 +15692,12 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc, RHSExpr = resolvedRHS.get(); } + if (getLangOpts().HLSL && (LHSExpr->getType()->isHLSLResourceRecord() || + LHSExpr->getType()->isHLSLResourceRecordArray())) { + if (!HLSL().CheckResourceBinOp(Opc, LHSExpr, RHSExpr, OpLoc)) + return ExprError(); + } + if (getLangOpts().CPlusPlus) { // Otherwise, build an overloaded op if either expression is type-dependent // or has an overloadable type. diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index fb8f131d1e11b..f97f5e50a0a01 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -3816,6 +3816,34 @@ bool SemaHLSL::ActOnUninitializedVarDecl(VarDecl *VD) { return false; } +// Return true if everything is ok; returns false if there was an error. +bool SemaHLSL::CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr, + Expr *RHSExpr, SourceLocation Loc) { + assert((LHSExpr->getType()->isHLSLResourceRecord() || + LHSExpr->getType()->isHLSLResourceRecordArray()) && + "expected LHS to be a resource record or array of resource records"); + if (Opc != BO_Assign) + return true; + + // If LHS is an array subscript, get the underlying declaration. + Expr *E = LHSExpr; + while (auto *ASE = dyn_cast(E)) + E = ASE->getBase()->IgnoreParenImpCasts(); + + // Report error if LHS is a resource declared at a global scope. + if (DeclRefExpr *DRE = dyn_cast(E->IgnoreParens())) { + if (VarDecl *VD = dyn_cast(DRE->getDecl())) { + if (VD->hasGlobalStorage()) { + // assignment to global resource is not allowed + SemaRef.Diag(Loc, diag::err_hlsl_assign_to_global_resource) << VD; + SemaRef.Diag(VD->getLocation(), diag::note_var_declared_here) << VD; + return false; + } + } + } + return true; +} + // Walks though the global variable declaration, collects all resource binding // requirements and adds them to Bindings void SemaHLSL::collectResourceBindingsOnVarDecl(VarDecl *VD) { diff --git a/clang/test/CodeGenHLSL/static-local-ctor.hlsl b/clang/test/CodeGenHLSL/static-local-ctor.hlsl index 9a4bf66f030ed..0bc088164ea24 100644 --- a/clang/test/CodeGenHLSL/static-local-ctor.hlsl +++ b/clang/test/CodeGenHLSL/static-local-ctor.hlsl @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -o - -disable-llvm-passes %s | FileCheck %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -o - -disable-llvm-passes %s | llvm-cxxfilt | FileCheck %s // Verify that no per variable _Init_thread instructions are emitted for non-trivial static locals // These would normally be emitted by the MicrosoftCXXABI, but the DirectX backend should exlude them @@ -7,23 +7,27 @@ RWBuffer buf[10]; +// CHECK: @[[main_mybuf:.*]] = internal global %"class.hlsl::RWBuffer" zeroinitializer, align 4 +// CHECK: @[[main_mybuf_guard:.*]] = internal global i8 0, align 1 + void InitBuf(RWBuffer buf) { for (unsigned int i = 0; i < 100; i++) buf[i] = 0; } // CHECK-NOT: _Init_thread_epoch -// CHECK: define internal void @_Z4mainv +// CHECK: define internal void @main() // CHECK-NEXT: entry: // CHECK-NEXT: [[Tmp0:%.*]] = alloca %"class.hlsl::RWBuffer" // CHECK-NEXT: [[Tmp1:%.*]] = alloca %"class.hlsl::RWBuffer" -// CHECK-NEXT: [[Tmp2:%.*]] = load i8, ptr @_ZGVZ4mainvE5mybuf -// CHECK-NEXT: [[Tmp3:%.*]] = icmp eq i8 [[Tmp2]], 0 +// CHECK-NEXT: [[GuardVar:%.*]] = load i8, ptr @[[main_mybuf_guard]] +// CHECK-NEXT: [[Tmp3:%.*]] = icmp eq i8 [[GuardVar]], 0 // CHECK-NEXT: br i1 [[Tmp3]] // CHECK-NOT: _Init_thread_header // CHECK: init.check: -// CHECK-NEXT: call void @_ZN4hlsl8RWBufferIiEC1EjijjPKc( -// CHECK-NEXT: store i8 1, ptr @_ZGVZ4mainvE5mybuf +// CHECK-NEXT: call void @hlsl::RWBuffer::RWBuffer(unsigned int, int, unsigned int, unsigned int, char const*)(ptr {{.*}} [[Tmp0]] +// CHECK-NEXT: call void @hlsl::RWBuffer::RWBuffer(hlsl::RWBuffer const&)(ptr {{.*}} @[[main_mybuf]] +// CHECK-NEXT: store i8 1, ptr @[[main_mybuf_guard]] // CHECK-NOT: _Init_thread_footer @@ -31,7 +35,6 @@ void InitBuf(RWBuffer buf) { [numthreads(1,1,1)] void main() { // A non-trivially constructed static local will get checks to verify that it is generated just once - static RWBuffer mybuf; - mybuf = buf[0]; + static RWBuffer mybuf = buf[0]; InitBuf(mybuf); } diff --git a/clang/test/SemaHLSL/prohibit_resource_edits.hlsl b/clang/test/SemaHLSL/prohibit_resource_edits.hlsl new file mode 100644 index 0000000000000..caffa32fd8085 --- /dev/null +++ b/clang/test/SemaHLSL/prohibit_resource_edits.hlsl @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -o - -fsyntax-only %s -verify + +RWBuffer A[10]; // expected-note {{variable 'A' is declared here}} expected-note {{variable 'A' is declared here}} // expected-note {{variable 'A' is declared here}} +RWBuffer B; // expected-note {{variable 'B' is declared here}} +RWBuffer C[10]; + +void test() { + // expected-error@+1{{assignment to global resource variable 'A' is not allowed}} + A = C; + + // expected-error@+1{{assignment to global resource variable 'B' is not allowed}} + B = C[0]; + + // expected-error@+1{{assignment to global resource variable 'A' is not allowed}} + A[1] = B; + + // expected-error@+1{{assignment to global resource variable 'A' is not allowed}} + A[1] = C[2]; + + // local resources are assignable + RWBuffer LocalA[10] = A; // ok + RWBuffer LocalB = B; // ok + + // read-write resources can be written to + A[0][0] = 1.0f; // ok + B[0] = 2.0f; // ok +} From 8ee5f35c88078cbe30cccfc5001e99250f6e3ff3 Mon Sep 17 00:00:00 2001 From: Helena Kotas Date: Thu, 18 Sep 2025 10:05:44 -0700 Subject: [PATCH 2/2] update test after merge --- clang/test/CodeGenHLSL/static-local-ctor.hlsl | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/test/CodeGenHLSL/static-local-ctor.hlsl b/clang/test/CodeGenHLSL/static-local-ctor.hlsl index 82a555fa56594..fd92e413bb3fa 100644 --- a/clang/test/CodeGenHLSL/static-local-ctor.hlsl +++ b/clang/test/CodeGenHLSL/static-local-ctor.hlsl @@ -27,6 +27,7 @@ void InitBuf(RWBuffer buf) { // CHECK-NOT: _Init_thread_header // CHECK: init.check: // CHECK-NEXT: call void @hlsl::RWBuffer::__createFromImplicitBinding +// CHECK-NEXT: call void @hlsl::RWBuffer::RWBuffer(hlsl::RWBuffer const&)(ptr {{.*}} @main()::mybuf, ptr {{.*}}) # // CHECK-NEXT: store i8 1, ptr @guard variable for main()::mybuf // CHECK-NOT: _Init_thread_footer