From: Henri Verbeet Subject: Re: [PATCH vkd3d v2 3/3] vkd3d-shader: Emit SPIR-V for descriptor arrays and arrayed bindings. Message-Id: Date: Wed, 16 Jun 2021 10:01:44 +0200 In-Reply-To: <9b9a847d7aab407270972a182a30e259@codeweavers.com> References: <20210603050124.45254-1-cmccarthy@codeweavers.com> <20210603050124.45254-3-cmccarthy@codeweavers.com> <9b9a847d7aab407270972a182a30e259@codeweavers.com> On Wed, 16 Jun 2021 at 08:14, Conor McCarthy wrote: > June 15, 2021 12:39 AM, "Henri Verbeet" wrote: > >> + || current->register_index > register_first > >> + || current->register_index + current->binding.count <= register_last) > >> continue; > > > > "current->register_index + current->binding.count" can overflow. > > The result would be it doesn't match any binding. We could emit an error, but I think overflow should be checked in d3d12_root_signature_append_vk_binding() in state.c. Other uses of vkd3d-shader could send this type of invalid binding, but I think the resulting "Could not find binding" is accurate enough. Well, it would be a nominally valid binding. Setting "binding.count = ~0u" in particular would trigger this, but even if the caller set a binding count that's larger than the Vulkan descriptor limits, that would be no reason for vkd3d-shader to reject the binding. Beyond that, I think it's simply good practice to write range checks like this in a way that avoids the overflow. I.e., "current->binding.count <= register_last - current->register_index".