From 56f612a0a2e6b6ee86b703fec6568b610fc6d5a8 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 22 Sep 2026 09:39:16 -0500 Subject: [PATCH 1/6] Fix crash when scalarizing OOB access The SROA HLSL pass misses checking that a GEP index is inbounds, which can cause an OOB memory access. This is not the complete fix for #8940, but it addresses the optimizer crash. --- docs/ReleaseNotes.md | 2 + .../Scalar/ScalarReplAggregatesHLSL.cpp | 11 +++ .../ScalarReplHLSL/vector-index-bounds.ll | 71 +++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 375d68d2fa..0582747ab7 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -24,6 +24,8 @@ line upon naming the release. Refer to previous for appropriate section names. #### Bug Fixes +- Fixed an optimizer crash when scalarizing an out-of-bounds vector access + [#8940](https://github.com/microsoft/DirectXShaderCompiler/issues/8940). - Fixed derivative operations being moved into divergent control flow, which could produce incorrect results [#8001](https://github.com/microsoft/DirectXShaderCompiler/issues/8001). diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index d80a678651..8be761aac6 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -1323,6 +1323,11 @@ void MarkUnsafe(AllocaInfo &I, Instruction *User) { DEBUG(dbgs() << " Transformation preventing inst: " << *User << '\n'); } +static bool isValidVectorIndex(const ConstantInt *Index, + unsigned NumElements) { + return Index->getValue().ult(NumElements); +} + /// isSafeGEP - Check if a GEP instruction can be handled for scalar /// replacement. It is safe when all the indices are constant, in-bounds /// references, and when the resulting offset corresponds to an element within @@ -1363,6 +1368,9 @@ void isSafeGEP(GetElementPtrInst *GEPI, uint64_t &Offset, AllocaInfo &Info) { } // Allow dynamic indexing ConstantInt *IdxVal = dyn_cast(GEPIt.getOperand()); + if (GEPIt->isVectorTy() && IdxVal && + !isValidVectorIndex(IdxVal, arraySize)) + return MarkUnsafe(Info, GEPI); if (!IdxVal) { // for dynamic index, use array size - 1 to check the offset *indicesIt = Constant::getIntegerValue( @@ -1386,6 +1394,9 @@ void isSafeGEP(GetElementPtrInst *GEPI, uint64_t &Offset, AllocaInfo &Info) { } // Allow dynamic indexing ConstantInt *IdxVal = dyn_cast(GEPIt.getOperand()); + if (GEPIt->isVectorTy() && IdxVal && + !isValidVectorIndex(IdxVal, arraySize)) + return MarkUnsafe(Info, GEPI); if (!IdxVal) { // for dynamic index, use array size - 1 to check the offset *indicesIt = Constant::getIntegerValue( diff --git a/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll b/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll new file mode 100644 index 0000000000..a3765f76d5 --- /dev/null +++ b/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll @@ -0,0 +1,71 @@ +; RUN: %dxopt %s -hlsl-passes-resume -scalarrepl-param-hlsl -S | FileCheck %s + +; Scalar replacement splits an array of vectors into one scalar array per +; vector lane. Verify that only lane indices which can select one of those +; arrays are scalarized. + +; CHECK-DAG: %valid.3 = alloca [2 x float] +; CHECK-DAG: %nested = alloca [2 x %struct.S] +; CHECK-DAG: %out_of_bounds = alloca [2 x <4 x float>] +; CHECK-DAG: %negative = alloca [2 x <4 x float>] +; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* %out_of_bounds, i32 0, i32 0, i32 7 +; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* %negative, i32 0, i32 0, i32 -1 +; CHECK: getelementptr inbounds [2 x float], [2 x float]* %valid.3, i32 0, i32 0 +; CHECK: getelementptr [2 x %struct.S], [2 x %struct.S]* %nested, i32 0, i32 0, i32 0, i32 7 + +target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64" +target triple = "dxil-ms-dx" + +%ConstantBuffer = type opaque +%struct.S = type { <4 x float> } + +@"$Globals" = external constant %ConstantBuffer + +define <4 x float> @main() { +entry: + %out_of_bounds = alloca [2 x <4 x float>] + %out_of_bounds.vector = getelementptr [2 x <4 x float>], [2 x <4 x float>]* %out_of_bounds, i32 0, i32 0 + %out_of_bounds.element = getelementptr <4 x float>, <4 x float>* %out_of_bounds.vector, i32 0, i32 7 + store float 9.000000e+00, float* %out_of_bounds.element + + %negative = alloca [2 x <4 x float>] + %negative.vector = getelementptr [2 x <4 x float>], [2 x <4 x float>]* %negative, i32 0, i32 0 + %negative.element = getelementptr <4 x float>, <4 x float>* %negative.vector, i32 0, i32 -1 + store float 9.000000e+00, float* %negative.element + + %valid = alloca [2 x <4 x float>] + %valid.vector = getelementptr [2 x <4 x float>], [2 x <4 x float>]* %valid, i32 0, i32 0 + %valid.element = getelementptr <4 x float>, <4 x float>* %valid.vector, i32 0, i32 3 + store float 9.000000e+00, float* %valid.element + + %nested = alloca [2 x %struct.S] + %nested.element = getelementptr [2 x %struct.S], [2 x %struct.S]* %nested, i32 0, i32 0, i32 0, i32 7 + store float 9.000000e+00, float* %nested.element + + ret <4 x float> zeroinitializer +} + +!pauseresume = !{!0} +!dx.version = !{!1} +!dx.valver = !{!2} +!dx.shaderModel = !{!3} +!dx.typeAnnotations = !{!4} +!dx.entryPoints = !{!8} +!dx.fnprops = !{!12} +!dx.options = !{!13, !14} + +!0 = !{!"hlsl-hlemit", !"hlsl-hlensure"} +!1 = !{i32 1, i32 6} +!2 = !{i32 1, i32 10} +!3 = !{!"ps", i32 6, i32 6} +!4 = !{i32 1, <4 x float> ()* @main, !5} +!5 = !{!6} +!6 = !{i32 1, !7, !7} +!7 = !{} +!8 = !{<4 x float> ()* @main, !"main", null, !9, null} +!9 = !{null, null, !10, null} +!10 = !{!11} +!11 = !{i32 0, %ConstantBuffer* @"$Globals", !"$Globals", i32 0, i32 -1, i32 1, i32 0, null} +!12 = !{<4 x float> ()* @main, i32 0, i1 false} +!13 = !{i32 64} +!14 = !{i32 -1} From 1455147a28785bf02467a61c11c6ca80c7a4a097 Mon Sep 17 00:00:00 2001 From: Chris B Date: Tue, 22 Sep 2026 10:32:28 -0500 Subject: [PATCH 2/6] clang-format --- lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index 8be761aac6..2e91df8107 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -1323,8 +1323,7 @@ void MarkUnsafe(AllocaInfo &I, Instruction *User) { DEBUG(dbgs() << " Transformation preventing inst: " << *User << '\n'); } -static bool isValidVectorIndex(const ConstantInt *Index, - unsigned NumElements) { +static bool isValidVectorIndex(const ConstantInt *Index, unsigned NumElements) { return Index->getValue().ult(NumElements); } @@ -1368,8 +1367,7 @@ void isSafeGEP(GetElementPtrInst *GEPI, uint64_t &Offset, AllocaInfo &Info) { } // Allow dynamic indexing ConstantInt *IdxVal = dyn_cast(GEPIt.getOperand()); - if (GEPIt->isVectorTy() && IdxVal && - !isValidVectorIndex(IdxVal, arraySize)) + if (GEPIt->isVectorTy() && IdxVal && !isValidVectorIndex(IdxVal, arraySize)) return MarkUnsafe(Info, GEPI); if (!IdxVal) { // for dynamic index, use array size - 1 to check the offset @@ -1394,8 +1392,7 @@ void isSafeGEP(GetElementPtrInst *GEPI, uint64_t &Offset, AllocaInfo &Info) { } // Allow dynamic indexing ConstantInt *IdxVal = dyn_cast(GEPIt.getOperand()); - if (GEPIt->isVectorTy() && IdxVal && - !isValidVectorIndex(IdxVal, arraySize)) + if (GEPIt->isVectorTy() && IdxVal && !isValidVectorIndex(IdxVal, arraySize)) return MarkUnsafe(Info, GEPI); if (!IdxVal) { // for dynamic index, use array size - 1 to check the offset From 50bb22d4e67487d858242b065f073493722abe39 Mon Sep 17 00:00:00 2001 From: Chris B Date: Tue, 22 Sep 2026 11:35:48 -0500 Subject: [PATCH 3/6] Handle OOB indexing on globals --- lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | 5 ++++- .../test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index 2e91df8107..8d17522418 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -1698,7 +1698,10 @@ bool hasDynamicVectorIndexing(Value *V) { for (; GEPIt != E; ++GEPIt) { if (isa(*GEPIt)) { Value *VecIdx = GEPIt.getOperand(); - if (!isa(VecIdx)) + ConstantInt *ConstantVecIdx = dyn_cast(VecIdx); + if (!ConstantVecIdx || + !isValidVectorIndex(ConstantVecIdx, + GEPIt->getVectorNumElements())) return true; } } diff --git a/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll b/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll index a3765f76d5..9019f35332 100644 --- a/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll +++ b/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll @@ -8,10 +8,12 @@ ; CHECK-DAG: %nested = alloca [2 x %struct.S] ; CHECK-DAG: %out_of_bounds = alloca [2 x <4 x float>] ; CHECK-DAG: %negative = alloca [2 x <4 x float>] +; CHECK-DAG: @global_out_of_bounds = internal global [2 x <4 x float>] zeroinitializer ; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* %out_of_bounds, i32 0, i32 0, i32 7 ; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* %negative, i32 0, i32 0, i32 -1 ; CHECK: getelementptr inbounds [2 x float], [2 x float]* %valid.3, i32 0, i32 0 ; CHECK: getelementptr [2 x %struct.S], [2 x %struct.S]* %nested, i32 0, i32 0, i32 0, i32 7 +; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* @global_out_of_bounds, i32 0, i32 0, i32 7 target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64" target triple = "dxil-ms-dx" @@ -20,6 +22,7 @@ target triple = "dxil-ms-dx" %struct.S = type { <4 x float> } @"$Globals" = external constant %ConstantBuffer +@global_out_of_bounds = internal global [2 x <4 x float>] zeroinitializer define <4 x float> @main() { entry: @@ -42,6 +45,8 @@ entry: %nested.element = getelementptr [2 x %struct.S], [2 x %struct.S]* %nested, i32 0, i32 0, i32 0, i32 7 store float 9.000000e+00, float* %nested.element + store float 9.000000e+00, float* getelementptr inbounds ([2 x <4 x float>], [2 x <4 x float>]* @global_out_of_bounds, i32 0, i32 0, i32 7) + ret <4 x float> zeroinitializer } From c268f06ecd41e1b4c6fbd547d5aff8c1cfa771b2 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 22 Sep 2026 13:16:23 -0500 Subject: [PATCH 4/6] Fix test --- .../DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll b/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll index 9019f35332..82fcea760a 100644 --- a/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll +++ b/tools/clang/test/DXC/Passes/ScalarReplHLSL/vector-index-bounds.ll @@ -1,19 +1,20 @@ ; RUN: %dxopt %s -hlsl-passes-resume -scalarrepl-param-hlsl -S | FileCheck %s ; Scalar replacement splits an array of vectors into one scalar array per -; vector lane. Verify that only lane indices which can select one of those -; arrays are scalarized. +; vector lane. Verify that invalid local lane indices are not scalarized. +; Constant global GEPs are first canonicalized across the entire aggregate, +; turning lane 7 into array index 1, lane 3, which can be scalarized safely. ; CHECK-DAG: %valid.3 = alloca [2 x float] ; CHECK-DAG: %nested = alloca [2 x %struct.S] ; CHECK-DAG: %out_of_bounds = alloca [2 x <4 x float>] ; CHECK-DAG: %negative = alloca [2 x <4 x float>] -; CHECK-DAG: @global_out_of_bounds = internal global [2 x <4 x float>] zeroinitializer +; CHECK-DAG: @global_out_of_bounds.3 = internal global [2 x float] zeroinitializer ; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* %out_of_bounds, i32 0, i32 0, i32 7 ; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* %negative, i32 0, i32 0, i32 -1 ; CHECK: getelementptr inbounds [2 x float], [2 x float]* %valid.3, i32 0, i32 0 ; CHECK: getelementptr [2 x %struct.S], [2 x %struct.S]* %nested, i32 0, i32 0, i32 0, i32 7 -; CHECK: getelementptr inbounds [2 x <4 x float>], [2 x <4 x float>]* @global_out_of_bounds, i32 0, i32 0, i32 7 +; CHECK: getelementptr inbounds ([2 x float], [2 x float]* @global_out_of_bounds.3, i32 0, i64 1) target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64" target triple = "dxil-ms-dx" From 54c3e77fc1ab7ddc2dfb09fb60a626d9cec11065 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 22 Sep 2026 14:51:54 -0500 Subject: [PATCH 5/6] clang-format --- lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index 8d17522418..91da958d66 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -1701,7 +1701,7 @@ bool hasDynamicVectorIndexing(Value *V) { ConstantInt *ConstantVecIdx = dyn_cast(VecIdx); if (!ConstantVecIdx || !isValidVectorIndex(ConstantVecIdx, - GEPIt->getVectorNumElements())) + GEPIt->getVectorNumElements())) return true; } } From ffd17c1f258f1e4d8659d48e4c4b79ce1e76f170 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Tue, 22 Sep 2026 14:54:37 -0500 Subject: [PATCH 6/6] Copilot review feedback --- lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index 91da958d66..11421f5067 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -1324,7 +1324,7 @@ void MarkUnsafe(AllocaInfo &I, Instruction *User) { } static bool isValidVectorIndex(const ConstantInt *Index, unsigned NumElements) { - return Index->getValue().ult(NumElements); + return !Index->isNegative() && Index->getValue().ult(NumElements); } /// isSafeGEP - Check if a GEP instruction can be handled for scalar