Skip to content

Commit 0bb5218

Browse files
authored
Make sure that locations don't roll over in spirv-val (KhronosGroup#6680)
Fixes KhronosGroup#6436
1 parent c58415d commit 0bb5218

3 files changed

Lines changed: 129 additions & 6 deletions

File tree

source/val/validate_interfaces.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -411,13 +411,16 @@ spv_result_t GetLocationsForVariable(
411411
uint32_t num_components = NumConsumedComponents(_, sub_type);
412412

413413
for (uint32_t array_idx = 0; array_idx < array_size; ++array_idx) {
414-
uint32_t array_location = location + (num_locations * array_idx);
415-
uint32_t start = array_location * 4;
416-
if (kMaxLocations <= start) {
414+
uint64_t array_location_u64 =
415+
static_cast<uint64_t>(location) +
416+
(static_cast<uint64_t>(num_locations) * array_idx);
417+
if (kMaxLocations <= array_location_u64 * 4) {
417418
// Too many locations, give up.
418419
break;
419420
}
420421

422+
uint32_t array_location = static_cast<uint32_t>(array_location_u64);
423+
uint32_t start = array_location * 4;
421424
uint32_t end = (array_location + num_locations) * 4;
422425
if (num_components != 0) {
423426
start += component;
@@ -487,19 +490,22 @@ spv_result_t GetLocationsForVariable(
487490
component = member_components[i - 1];
488491
}
489492

490-
uint32_t start = location * 4;
491-
if (kMaxLocations <= start) {
493+
uint64_t start_u64 = static_cast<uint64_t>(location) * 4;
494+
if (kMaxLocations <= start_u64) {
492495
// Too many locations, give up.
493496
continue;
494497
}
495498

499+
uint32_t start = static_cast<uint32_t>(start_u64);
496500
if (member->opcode() == spv::Op::OpTypeArray && num_components >= 1 &&
497501
num_components < 4) {
498502
// When an array has an element that takes less than a location in
499503
// size, calculate the used locations in a strided manner.
500504
for (uint32_t l = location; l < num_locations + location; ++l) {
501505
for (uint32_t c = component; c < component + num_components; ++c) {
502-
uint32_t check = 4 * l + c;
506+
uint64_t check_u64 = static_cast<uint64_t>(l) * 4 + c;
507+
if (kMaxLocations <= check_u64) continue;
508+
uint32_t check = static_cast<uint32_t>(check_u64);
503509
if (!locations->insert(check).second) {
504510
return _.diag(SPV_ERROR_INVALID_DATA, entry_point)
505511
<< (is_output ? _.VkErrorID(8722) : _.VkErrorID(8721))

test/val/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ add_spvtools_unittest(TARGET val_fghijklmnop
8686
val_interfaces_test.cpp
8787
val_layout_test.cpp
8888
val_literals_test.cpp
89+
val_location_rollover_test.cpp
8990
val_logical_pointers_test.cpp
9091
val_logicals_test.cpp
9192
val_memory_test.cpp
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// Copyright (c) 2026 Google Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Tests location integer rollover is correctly handles
16+
17+
#include <string>
18+
19+
#include "gmock/gmock.h"
20+
#include "test/unit_spirv.h"
21+
#include "test/val/val_fixtures.h"
22+
23+
namespace spvtools {
24+
namespace val {
25+
namespace {
26+
27+
using ::testing::HasSubstr;
28+
29+
using ValidateLocationRollover = spvtest::ValidateBase<bool>;
30+
31+
TEST_F(ValidateLocationRollover, VulkanLocationRollover) {
32+
std::string text = R"(
33+
OpCapability Shader
34+
OpMemoryModel Logical GLSL450
35+
OpEntryPoint Fragment %1 "func" %var1 %var2
36+
OpExecutionMode %1 OriginUpperLeft
37+
OpDecorate %var1 Location 0
38+
OpDecorate %var2 Location 1073741824
39+
%2 = OpTypeVoid
40+
%3 = OpTypeInt 32 0
41+
%4 = OpTypePointer Output %3
42+
%var1 = OpVariable %4 Output
43+
%var2 = OpVariable %4 Output
44+
%6 = OpTypeFunction %2
45+
%1 = OpFunction %2 None %6
46+
%7 = OpLabel
47+
OpReturn
48+
OpFunctionEnd
49+
)";
50+
51+
// 1073741824 * 4 = 4294967296 (2^32), which overflows to 0 in 32-bit.
52+
// This would cause it to be treated as Location 0 and conflict with %var1.
53+
54+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_2);
55+
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_2))
56+
<< getDiagnosticString();
57+
}
58+
59+
TEST_F(ValidateLocationRollover, VulkanLocationRolloverStructMember) {
60+
std::string text = R"(
61+
OpCapability Shader
62+
OpMemoryModel Logical GLSL450
63+
OpEntryPoint Fragment %1 "func" %var
64+
OpExecutionMode %1 OriginUpperLeft
65+
OpMemberDecorate %struct 0 Location 0
66+
OpMemberDecorate %struct 1 Location 1073741824
67+
OpDecorate %struct Block
68+
%2 = OpTypeVoid
69+
%3 = OpTypeInt 32 0
70+
%struct = OpTypeStruct %3 %3
71+
%4 = OpTypePointer Output %struct
72+
%var = OpVariable %4 Output
73+
%6 = OpTypeFunction %2
74+
%1 = OpFunction %2 None %6
75+
%7 = OpLabel
76+
OpReturn
77+
OpFunctionEnd
78+
)";
79+
80+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_2);
81+
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_2))
82+
<< getDiagnosticString();
83+
}
84+
85+
TEST_F(ValidateLocationRollover, VulkanLocationRolloverStructMemberStrided) {
86+
std::string text = R"(
87+
OpCapability Shader
88+
OpMemoryModel Logical GLSL450
89+
OpEntryPoint Fragment %1 "func" %var
90+
OpExecutionMode %1 OriginUpperLeft
91+
OpMemberDecorate %struct 0 Location 0
92+
OpMemberDecorate %struct 1 Location 1073741824
93+
OpDecorate %struct Block
94+
%void = OpTypeVoid
95+
%u32 = OpTypeInt 32 0
96+
%u32_1 = OpConstant %u32 1
97+
%u32_2 = OpTypeVector %u32 2
98+
%array = OpTypeArray %u32_2 %u32_1
99+
%struct = OpTypeStruct %u32 %array
100+
%ptr = OpTypePointer Output %struct
101+
%var = OpVariable %ptr Output
102+
%func_type = OpTypeFunction %void
103+
%1 = OpFunction %void None %func_type
104+
%label = OpLabel
105+
OpReturn
106+
OpFunctionEnd
107+
)";
108+
109+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_2);
110+
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_2))
111+
<< getDiagnosticString();
112+
}
113+
114+
} // namespace
115+
} // namespace val
116+
} // namespace spvtools

0 commit comments

Comments
 (0)