gpuav: Bounds checking for shared memory accesses#11872
gpuav: Bounds checking for shared memory accesses#11872jeffbolznv wants to merge 5 commits intoKhronosGroup:mainfrom
Conversation
|
CI Vulkan-ValidationLayers build queued with queue ID 678858. |
|
CI Vulkan-ValidationLayers build # 22772 running. |
spencer-lunarg
left a comment
spencer-lunarg
left a comment
There was a problem hiding this comment.
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
|
CI Vulkan-ValidationLayers build queued with queue ID 678919. |
|
CI Vulkan-ValidationLayers build # 22774 running. |
|
CI Vulkan-ValidationLayers build queued with queue ID 679085. |
|
CI Vulkan-ValidationLayers build # 22775 running. |
|
CI Vulkan-ValidationLayers build # 22775 passed. |
|
CI Vulkan-ValidationLayers build queued with queue ID 681511. |
|
CI Vulkan-ValidationLayers build # 22806 running. |
|
CI Vulkan-ValidationLayers build # 22806 passed. |
spencer-lunarg
force-pushed
the
shared_memory_oob
branch
from
622f850 to
347f9c2
Compare
|
CI Vulkan-ValidationLayers build queued with queue ID 686166. |
| continue; | ||
| } | ||
|
|
||
| auto& block_instructions = current_block.instructions_; |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
I didn't do this for now.
| } | ||
| instrumentations_count_++; | ||
|
|
||
| CreateFunctionCall(current_block, &inst_it, meta); |
There was a problem hiding this comment.
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!)
There was a problem hiding this comment.
unless it will only crash if the bad value is above the compute max size
There was a problem hiding this comment.
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.
|
CI Vulkan-ValidationLayers build # 22882 running. |
|
CI Vulkan-ValidationLayers build # 22882 passed. |
|
CI Vulkan-ValidationLayers build queued with queue ID 686260. |
|
CI Vulkan-ValidationLayers build # 22884 running. |
|
CI Vulkan-ValidationLayers build # 22884 failed. |
4 participants
Closes #11773.
This was entirely written by AI, but I've reviewed the code in detail.