Skip to content

[HLSL] error on out of bounds vector accesses #128952

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

Merged
merged 9 commits into from
Mar 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ Improvements to Clang's diagnostics
under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
- Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
``-Wno-error=parentheses``.
- Adds an error diagnostic for out of bounds vector accesses; produces an error
for compile time statically provable out of bounds vector accesses.
- The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334)
- Fixed diagnostics adding a trailing ``::`` when printing some source code
constructs, like base classes.
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -10684,6 +10684,9 @@ def err_block_on_vm : Error<
def err_sizeless_nonlocal : Error<
"non-local variable with sizeless type %0">;

def err_vector_index_out_of_range : Error<
"vector element index %0 is out of bounds">;

def err_vec_builtin_non_vector : Error<
"%select{first two|all}1 arguments to %0 must be vectors">;
def err_vec_builtin_incompatible_vector : Error<
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -2468,6 +2468,7 @@ class Sema final : public SemaBase {
const ArraySubscriptExpr *ASE = nullptr,
bool AllowOnePastEnd = true, bool IndexNegated = false);
void CheckArrayAccess(const Expr *E);
void CheckVectorAccess(const Expr *BaseExpr, const Expr *IndexExpr);

bool CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall,
const FunctionProtoType *Proto);
Expand Down
23 changes: 23 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14054,6 +14054,23 @@ void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
<< TRange << Op->getSourceRange();
}

void Sema::CheckVectorAccess(const Expr *BaseExpr, const Expr *IndexExpr) {
const auto *VTy = BaseExpr->getType()->getAs<VectorType>();
if (!VTy)
return;

Expr::EvalResult Result;
if (!IndexExpr->EvaluateAsInt(Result, Context, Expr::SE_AllowSideEffects))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally, if a value is not required to be a constant, we don't want different semantic rules depending on whether the value is in fact constant. (There are a few places that do in fact work like this, like array bounds, but it causes weird results. Especially when people do stuff with macros.) So I don't think we want to reject here.

A DiagRuntimeBehavior() is probably appropriate, though.

return;

unsigned DiagID = diag::err_vector_index_out_of_range;

llvm::APSInt index = Result.Val.getInt();
if (index.isNegative() || index >= VTy->getNumElements())
Diag(BaseExpr->getBeginLoc(), DiagID) << toString(index, 10, true);
return;
}

void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
const ArraySubscriptExpr *ASE,
bool AllowOnePastEnd, bool IndexNegated) {
Expand All @@ -14068,6 +14085,12 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
const Type *EffectiveType =
BaseExpr->getType()->getPointeeOrArrayElementType();
BaseExpr = BaseExpr->IgnoreParenCasts();

if (BaseExpr->getType()->isVectorType()) {
CheckVectorAccess(BaseExpr, IndexExpr);
return;
}

const ConstantArrayType *ArrayTy =
Context.getAsConstantArrayType(BaseExpr->getType());

Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenCXX/x86_64-arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,6 @@ union U {
float f1;
char __attribute__((__vector_size__(1))) f2;
};
int f(union U u) { return u.f2[1]; }
int f(union U u) { return u.f2[0]; }
// CHECK-LABEL: define{{.*}} i32 @_ZN6test111fENS_1UE(i32
}
19 changes: 19 additions & 0 deletions clang/test/SemaHLSL/Language/VectorOutOfRange-errors.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -verify

export void fn1() {
int2 A = {1,2};
int X = A[-1];
// expected-error@-1 {{vector element index -1 is out of bounds}}
}

export void fn2() {
int4 A = {1,2,3,4};
int X = A[4];
// expected-error@-1 {{vector element index 4 is out of bounds}}
}

export void fn3() {
bool2 A = {true,true};
bool X = A[-1];
// expected-error@-1 {{vector element index -1 is out of bounds}}
}