Skip to content

Commit

Permalink
Add earlier check for invalid SV_Target[n] (#6771)
Browse files Browse the repository at this point in the history
According to the HLSL semantics documentation, the valid semantic
indices for SV_Target[n] are 0 <= n <= 7:

https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-semantics

A check for this already exists in DXIL validation, but for large values
of n, crashes and/or buffer overruns may occur during compilation before
validation, so an earlier check is needed.

Fixes #6115
  • Loading branch information
sudonatalie authored Jul 12, 2024
1 parent b18fe87 commit a54abe8
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 2 deletions.
2 changes: 2 additions & 0 deletions tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -7962,6 +7962,8 @@ def warn_hlsl_barrier_no_mem_with_required_device_scope: Warning<
def warn_hlsl_legacy_integer_literal_signedness: Warning<
"literal value is treated as signed in HLSL 2021 and earlier, and unsigned in later language versions">,
InGroup<HLSLLegacyLiterals>, DefaultIgnore;
def err_hlsl_unsupported_semantic_index: Error<
"'%0' is defined with semantic index %1, but only values 0 through %2 are supported">;
// HLSL Change Ends

// SPIRV Change Starts
Expand Down
2 changes: 2 additions & 0 deletions tools/clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -9108,6 +9108,8 @@ class Sema {
bool isImplicit);
QualType getHLSLDefaultSpecialization(TemplateDecl *Decl);
// HLSL Change End - adjust this from T* to T&-like

void DiagnoseSemanticDecl(hlsl::SemanticDecl *Decl); // HLSL Change
};

/// \brief RAII object that enters a new expression evaluation context.
Expand Down
1 change: 1 addition & 0 deletions tools/clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ bool Parser::MaybeParseHLSLAttributes(std::vector<hlsl::UnusualAnnotation *> &ta
}
hlsl::SemanticDecl *pUA = new (context) hlsl::SemanticDecl(semanticName);
pUA->Loc = Tok.getLocation();
Actions.DiagnoseSemanticDecl(pUA);
ConsumeToken(); // consume semantic

target.push_back(pUA);
Expand Down
22 changes: 20 additions & 2 deletions tools/clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
//
//===----------------------------------------------------------------------===//

#include "clang/Sema/SemaInternal.h"
#include "TypeLocBuilder.h"
#include "dxc/DXIL/DxilSemantic.h" // HLSL Change
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTLambda.h"
Expand Down Expand Up @@ -42,13 +42,14 @@
#include "clang/Sema/ParsedTemplate.h"
#include "clang/Sema/Scope.h"
#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/SemaHLSL.h" // HLSL Change
#include "clang/Sema/SemaInternal.h"
#include "clang/Sema/Template.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/Triple.h"
#include <algorithm>
#include <cstring>
#include <functional>
#include "clang/Sema/SemaHLSL.h" // HLSL Change
using namespace clang;
using namespace sema;

Expand Down Expand Up @@ -14629,3 +14630,20 @@ AvailabilityResult Sema::getCurContextAvailability() const {
return D ? D->getAvailability() : AR_Available;
}

// HLSL Change Begin
void Sema::DiagnoseSemanticDecl(hlsl::SemanticDecl *Decl) {
StringRef SemName = Decl->SemanticName;

StringRef BaseSemName; // The 'FOO' in 'FOO1'
uint32_t SemIndex; // The '1' in 'FOO1'

// Split name and index.
hlsl::Semantic::DecomposeNameAndIndex(SemName, &BaseSemName, &SemIndex);

// The valid semantic indices for SV_Target[n] are 0 <= n <= 7.
if (BaseSemName.equals("SV_Target") && SemIndex > 7) {
Diag(Decl->Loc, diag::err_hlsl_unsupported_semantic_index)
<< SemName << SemIndex << "7";
}
}
// HLSL Change Ends
10 changes: 10 additions & 0 deletions tools/clang/test/SemaHLSL/hlsl/semantics/sv_target.index.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// RUN: %dxc -T ps_6_0 -E zero -verify %s
// RUN: %dxc -T ps_6_0 -E seven -verify %s

float4 zero() : SV_Target0 { /* expected-no-diagnostics */
return float4(0, 1, 2, 3);
}

float4 seven() : SV_Target7 { /* expected-no-diagnostics */
return float4(0, 1, 2, 3);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// RUN: %dxc -T ps_6_0 -verify %s

float4 bad() : SV_Target8 { /* expected-error{{'SV_Target8' is defined with semantic index 8, but only values 0 through 7 are supported}} */
return float4(0, 1, 2, 3);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// RUN: %dxc -T ps_6_0 -verify %s

struct S {
float4 t : SV_Target12345; /* expected-error{{'SV_Target12345' is defined with semantic index 12345, but only values 0 through 7 are supported}} */
};

S main() {
return S(float4(0, 1, 2, 3));
}

0 comments on commit a54abe8

Please sign in to comment.