Possibly incorrect length check in BDA validation

[baldurk]

Environment:

• OS: Windows 11
• GPU and driver version: AMD 7900 XTX driver 24.10.1
• SDK or header version if building from repo: Built from b10d21a
• Options enabled (synchronization, best practices, etc.): GPU validation & synchronization

Describe the Issue

When running with GPU-AV I encountered an assert inside the BDA SPIR-V pass:

Vulkan-ValidationLayers/layers/gpu/spirv/buffer_device_address_pass.cpp

Lines 80 to 84 in f00d38b

	const uint32_t alignment_word_index = opcode == spv::OpLoad ? 5 : 4; // OpStore is at [4]
	if (inst.Length() < alignment_word_index) {
	return false;
	}
	alignment_literal_ = inst.Word(alignment_word_index);

In this case the OpLoad was 5 words long but then an access to word index [5] was out of bounds. There was no subsequent crash but I'm guessing that was just luck.

Expected behavior

There would be no assert when processing shaders.

Valid Usage ID
N/A

Additional context

I think the problem here is that although OpLoad is normally only 4 words, it can be 5 words if there are memory operands which do not include Aligned (since [4] is the memory operands and [5] if any will be the parameters) - in my case this is some generated SPIR-V inside RenderDoc and it always emits the memory operands parameter even if it's None, but the same could happen if there was a parameter-less memory operand. The length check should maybe be <= to cover this case.

I'm not sure if you want to also add a check that the memory operands have Aligned set as well, since although e.g. MakePointerAvailable would also have a param the pointer can't validly be BDA if it doesn't include Aligned (whose operand would come first). The 'alignment' value loaded in that case would not be used for anything currently, but might be confusing?