Skip to content

[HLSL] Disallow writing to readonly resources #147806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

V-FEXrt
Copy link
Contributor

@V-FEXrt V-FEXrt commented Jul 9, 2025

Fixes #141842

Only add the non-const subscript operator to write resources

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Jul 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 9, 2025

@llvm/pr-subscribers-clang

Author: Ashley Coleman (V-FEXrt)

Changes

Fixes #141842

Only add the non-const subscript operator to write resources


Full diff: https://github.com/llvm/llvm-project/pull/147806.diff

4 Files Affected:

  • (modified) clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp (+14-1)
  • (modified) clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h (+1)
  • (modified) clang/test/SemaHLSL/BuiltIns/Buffers.hlsl (+4)
  • (modified) clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl (+4)
diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
index e5c6220bfb47d..7c08a917f24be 100644
--- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
+++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
@@ -613,6 +613,17 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addHandleMember(
     ResourceClass RC, bool IsROV, bool RawBuffer, AccessSpecifier Access) {
   assert(!Record->isCompleteDefinition() && "record is already complete");
 
+  switch (RC) {
+  case llvm::dxil::ResourceClass::UAV:
+    IsConstResourceClass = false;
+    break;
+  case llvm::dxil::ResourceClass::SRV:
+  case llvm::dxil::ResourceClass::CBuffer:
+  case llvm::dxil::ResourceClass::Sampler:
+    IsConstResourceClass = true;
+    break;
+  };
+
   ASTContext &Ctx = SemaRef.getASTContext();
   TypeSourceInfo *ElementTypeInfo =
       Ctx.getTrivialTypeSourceInfo(getHandleElementType(), SourceLocation());
@@ -697,7 +708,9 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addArraySubscriptOperators() {
       AST.DeclarationNames.getCXXOperatorName(OO_Subscript);
 
   addHandleAccessFunction(Subscript, /*IsConst=*/true, /*IsRef=*/true);
-  addHandleAccessFunction(Subscript, /*IsConst=*/false, /*IsRef=*/true);
+  if (!IsConstResourceClass)
+    addHandleAccessFunction(Subscript, /*IsConst=*/false, /*IsRef=*/true);
+
   return *this;
 }
 
diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
index a52e2938104c7..d8fac3cebc94a 100644
--- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
+++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
@@ -52,6 +52,7 @@ class BuiltinTypeDeclBuilder {
   ClassTemplateDecl *PrevTemplate = nullptr;
   NamespaceDecl *HLSLNamespace = nullptr;
   llvm::StringMap<FieldDecl *> Fields;
+  bool IsConstResourceClass = false;
 
 public:
   friend struct TemplateParameterListBuilder;
diff --git a/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl b/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
index 477a16a454a9c..d7c6876d3b9e3 100644
--- a/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
@@ -111,4 +111,8 @@ Buffer<threeDoubles> BufferErr3;
 void main() {
   (void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::Buffer<vector<float, 3>>'}}
   // expected-note@* {{implicitly declared private here}}
+
+  // expected-error@+2 {{cannot assign to return value because function 'operator[]' returns a const value}}
+  // expected-note@* {{function 'operator[]' which returns const-qualified type 'vector<float const hlsl_device &, 3>' declared here}}
+  Buff[0] = 0.0;
 }
diff --git a/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl b/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
index bf541f4a07da7..fbd9288590adc 100644
--- a/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
@@ -28,4 +28,8 @@ StructuredBuffer<Empty> BufferErr4;
 void main() {
   (void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::StructuredBuffer<vector<float, 3>>'}}
   // expected-note@* {{implicitly declared private here}}
+
+  // expected-error@+2 {{cannot assign to return value because function 'operator[]' returns a const value}}
+  // expected-note@* {{function 'operator[]' which returns const-qualified type 'vector<float const hlsl_device &, 3>' declared here}}
+  Buff[0] = 0.0;
 }

@llvmbot
Copy link
Member

llvmbot commented Jul 9, 2025

@llvm/pr-subscribers-hlsl

Author: Ashley Coleman (V-FEXrt)

Changes

Fixes #141842

Only add the non-const subscript operator to write resources


Full diff: https://github.com/llvm/llvm-project/pull/147806.diff

4 Files Affected:

  • (modified) clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp (+14-1)
  • (modified) clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h (+1)
  • (modified) clang/test/SemaHLSL/BuiltIns/Buffers.hlsl (+4)
  • (modified) clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl (+4)
diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
index e5c6220bfb47d..7c08a917f24be 100644
--- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
+++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp
@@ -613,6 +613,17 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addHandleMember(
     ResourceClass RC, bool IsROV, bool RawBuffer, AccessSpecifier Access) {
   assert(!Record->isCompleteDefinition() && "record is already complete");
 
+  switch (RC) {
+  case llvm::dxil::ResourceClass::UAV:
+    IsConstResourceClass = false;
+    break;
+  case llvm::dxil::ResourceClass::SRV:
+  case llvm::dxil::ResourceClass::CBuffer:
+  case llvm::dxil::ResourceClass::Sampler:
+    IsConstResourceClass = true;
+    break;
+  };
+
   ASTContext &Ctx = SemaRef.getASTContext();
   TypeSourceInfo *ElementTypeInfo =
       Ctx.getTrivialTypeSourceInfo(getHandleElementType(), SourceLocation());
@@ -697,7 +708,9 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addArraySubscriptOperators() {
       AST.DeclarationNames.getCXXOperatorName(OO_Subscript);
 
   addHandleAccessFunction(Subscript, /*IsConst=*/true, /*IsRef=*/true);
-  addHandleAccessFunction(Subscript, /*IsConst=*/false, /*IsRef=*/true);
+  if (!IsConstResourceClass)
+    addHandleAccessFunction(Subscript, /*IsConst=*/false, /*IsRef=*/true);
+
   return *this;
 }
 
diff --git a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
index a52e2938104c7..d8fac3cebc94a 100644
--- a/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
+++ b/clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h
@@ -52,6 +52,7 @@ class BuiltinTypeDeclBuilder {
   ClassTemplateDecl *PrevTemplate = nullptr;
   NamespaceDecl *HLSLNamespace = nullptr;
   llvm::StringMap<FieldDecl *> Fields;
+  bool IsConstResourceClass = false;
 
 public:
   friend struct TemplateParameterListBuilder;
diff --git a/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl b/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
index 477a16a454a9c..d7c6876d3b9e3 100644
--- a/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
@@ -111,4 +111,8 @@ Buffer<threeDoubles> BufferErr3;
 void main() {
   (void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::Buffer<vector<float, 3>>'}}
   // expected-note@* {{implicitly declared private here}}
+
+  // expected-error@+2 {{cannot assign to return value because function 'operator[]' returns a const value}}
+  // expected-note@* {{function 'operator[]' which returns const-qualified type 'vector<float const hlsl_device &, 3>' declared here}}
+  Buff[0] = 0.0;
 }
diff --git a/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl b/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
index bf541f4a07da7..fbd9288590adc 100644
--- a/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
@@ -28,4 +28,8 @@ StructuredBuffer<Empty> BufferErr4;
 void main() {
   (void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::StructuredBuffer<vector<float, 3>>'}}
   // expected-note@* {{implicitly declared private here}}
+
+  // expected-error@+2 {{cannot assign to return value because function 'operator[]' returns a const value}}
+  // expected-note@* {{function 'operator[]' which returns const-qualified type 'vector<float const hlsl_device &, 3>' declared here}}
+  Buff[0] = 0.0;
 }

Copy link
Contributor

@spall spall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm from my understanding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] Read Only Resources incorrectly allow writing
5 participants