Skip to content

gpuav: Bounds checking for shared/private/function arrays#11872

Open
jeffbolznv wants to merge 10 commits intoKhronosGroup:mainfrom
jeffbolznv:shared_memory_oob
Open

gpuav: Bounds checking for shared/private/function arrays#11872
jeffbolznv wants to merge 10 commits intoKhronosGroup:mainfrom
jeffbolznv:shared_memory_oob

Conversation

@jeffbolznv
Copy link
Copy Markdown
Contributor

Closes #11773.

This was entirely written by AI, but I've reviewed the code in detail.

@jeffbolznv jeffbolznv requested a review from a team as a code owner March 13, 2026 18:43
@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 678858.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22772 running.

Copy link
Copy Markdown
Contributor

@spencer-lunarg spencer-lunarg left a comment

Choose a reason for hiding this comment

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

will try to take a closer look Monday (didn't look at the test or the main heart of the RequireInstrumentation pass), some quick obvious things

Comment thread layers/gpuav/core/gpuav_settings.h Outdated
Comment thread layers/gpuav/instrumentation/shared_memory_oob.cpp Outdated
Comment thread layers/gpuav/spirv/shared_memory_oob_pass.cpp Outdated
Comment thread layers/gpuav/spirv/module.cpp Outdated
Comment thread layers/gpuav/spirv/shared_memory_oob_pass.cpp Outdated
Comment thread layers/gpuav/spirv/shared_memory_oob_pass.cpp Outdated
Comment thread layers/gpuav/spirv/shared_memory_oob_pass.cpp Outdated
Comment thread layers/gpuav/instrumentation/shared_memory_oob.cpp Outdated
@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 678919.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22774 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 679085.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22775 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22775 passed.

Comment thread layers/gpuav/instrumentation/array_oob.cpp
Comment thread layers/gpuav/spirv/array_oob_pass.cpp
Comment thread layers/gpuav/spirv/array_oob_pass.h
Comment thread tests/unit/gpu_av_shared_memory_oob.cpp Outdated
@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 681511.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22806 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22806 passed.

Comment thread tests/unit/gpu_av_shared_memory_oob.cpp Outdated
@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 686166.

continue;
}

auto& block_instructions = current_block.instructions_;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in descriptor_class_general_buffer_pass.cpp we have this

 vvl::unordered_map<uint32_t, uint32_t> block_highest_offset_map;

basically would be nice to detect if inside a block multiple accesses to the same index are made so we don't need to re-instrument it each time (this can be done in a follow-up PR)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't do this for now.

Comment thread layers/gpuav/spirv/array_oob_pass.cpp Outdated
}
instrumentations_count_++;

CreateFunctionCall(current_block, &inst_it, meta);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

seems we don't have the good o'l

                if (!module_.settings_.safe_mode) {
                    CreateFunctionCall(current_block, &inst_it, meta);
                } else {
                    InjectConditionalData ic_data = InjectFunctionPre(function, block_it, inst_it);
                    ic_data.function_result_id = CreateFunctionCall(current_block, nullptr, meta);
                    InjectFunctionPost(current_block, ic_data);
                    // Skip the newly added valid and invalid block. Start searching again from newly split merge block
                    block_it++;
                    block_it++;
                    break;
                }

which means I guess no drivers break on bad Shared Memory OOB checks (but still good to report regardless!)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unless it will only crash if the bad value is above the compute max size

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We implemented this but I had to revert it. Trying to branch over opaccesschains, the existing code tried to phi in a zero for the else pass and it generated invalid spirv. I think we should skip supporting safe mode for now.

Comment thread tests/unit/gpu_av_shared_memory_oob_positive.cpp Outdated
@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22882 running.

Comment thread layers/gpuav/instrumentation/shared_memory_oob.cpp Outdated
Comment thread tests/unit/gpu_av_shared_memory_oob.cpp Outdated
Comment thread tests/unit/gpu_av_array_oob.cpp
@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22882 passed.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 686260.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22884 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 22884 failed.

@jeffbolznv jeffbolznv force-pushed the shared_memory_oob branch from 8310413 to 4bceb25 Compare May 1, 2026 21:07
@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 725161.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23273 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23273 aborted.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 725273.

@jeffbolznv jeffbolznv changed the title gpuav: Bounds checking for shared memory accesses gpuav: Bounds checking for shared/private/function arrays May 2, 2026
@jeffbolznv
Copy link
Copy Markdown
Contributor Author

Made this more generic, to handle Private/Function for #12020.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23274 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23274 failed.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 725298.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23275 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23275 failed.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 725329.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23276 running.

@ci-tester-lunarg
Copy link
Copy Markdown
Collaborator

CI Vulkan-ValidationLayers build # 23276 failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GPU-AV: Add Shared Memory OOB checks

4 participants