Skip to content

Conversation

@Keenuts
Copy link
Contributor

@Keenuts Keenuts commented Dec 23, 2025

Unparsed HLSL semantics have 2 int/bool arguments with default values. Those values are loaded using checkUInt32Argument, and thus this should never fail.

But if something is wrong in the code above (or the def changed), this code wouldn't catch it and we'd read uninitialized integers. This commits checks both return values and assert if one fails. On release builds, this would early return, causing a bad codegen, but such change will be caught by tests.

Unparsed HLSL semantics have 2 int/bool arguments with default
values. Those values are loaded using checkUInt32Argument, and thus
this should never fail.

But if something is wrong in the code above (or the def changed), this
code wouldn't catch it and we'd read uninitialized integers.
This commits checks both return values and assert if one fails.
On release builds, this would early return, causing a bad codegen, but
such change will be caught by tests.
@Keenuts Keenuts requested a review from luciechoi December 23, 2025 10:22
@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 Dec 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 23, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Nathan Gauër (Keenuts)

Changes

Unparsed HLSL semantics have 2 int/bool arguments with default values. Those values are loaded using checkUInt32Argument, and thus this should never fail.

But if something is wrong in the code above (or the def changed), this code wouldn't catch it and we'd read uninitialized integers. This commits checks both return values and assert if one fails. On release builds, this would early return, causing a bad codegen, but such change will be caught by tests.


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

1 Files Affected:

  • (modified) clang/lib/Sema/SemaHLSL.cpp (+5-3)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 06130c985876f..0b1996b4789d6 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1908,9 +1908,11 @@ void SemaHLSL::diagnoseSystemSemanticAttr(Decl *D, const ParsedAttr &AL,
 }
 
 void SemaHLSL::handleSemanticAttr(Decl *D, const ParsedAttr &AL) {
-  uint32_t IndexValue, ExplicitIndex;
-  SemaRef.checkUInt32Argument(AL, AL.getArgAsExpr(0), IndexValue);
-  SemaRef.checkUInt32Argument(AL, AL.getArgAsExpr(1), ExplicitIndex);
+  uint32_t IndexValue(0), ExplicitIndex(0);
+  if (!SemaRef.checkUInt32Argument(AL, AL.getArgAsExpr(0), IndexValue) ||
+      !SemaRef.checkUInt32Argument(AL, AL.getArgAsExpr(1), ExplicitIndex)) {
+    assert(0 && "HLSLUnparsedSemantic is expected to have 2 int arguments.");
+  }
   assert(IndexValue > 0 ? ExplicitIndex : true);
   std::optional<unsigned> Index =
       ExplicitIndex ? std::optional<unsigned>(IndexValue) : std::nullopt;

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.

2 participants