On 11/10/2014 08:12 PM, Timothy Arceri wrote:
> On Mon, 2014-11-10 at 06:09 -0800, Ian Romanick wrote:
>> On 11/06/2014 11:40 PM, Kenneth Graunke wrote:
>>> On Thursday, November 06, 2014 08:09:18 PM Ian Romanick wrote:
>>>> While working on some other things, I came across some bounds checking
>>>> code in _mesa_validate_DrawElements (and related functions) in
>>>> api_validate.c.
>>>>
>>>> /* use indices in the buffer object */
>>>> /* make sure count doesn't go outside buffer bounds */
>>>> if (index_bytes(type, count) > ctx->Array.VAO->IndexBufferObj->Size)
>>>> {
>>>> _mesa_warning(ctx, "glDrawElements index out of buffer bounds");
>>>> return GL_FALSE;
>>>> }
>>>>
>>>> index_bytes calculates how many bytes of data "count" indices will
>>>> occupy based on the type. The problem is that this doesn't consider
>>>> the base pointer. As far as I can tell, if I had a 64 byte buffer
>>>> object for my index data, and I did
>>>>
>>>> glDrawElements(GL_POINTS, 16, GL_UNSIGNED_INT, 60);
>>>>
>>>> _mesa_validate_DrawElements would say, "Ok!"
>>>>
>>>> Am I missing something, or is this just broken?
>>>
>>> It sure seems broken to me - but, thankfully, in a conservative fashion.
>>> (It
>>> will say some invalid things are OK, but won't say legal things are
>>> invalid.)
>>>
>>> Software drivers may be relying on this working to avoid a crash.
>>>
>>> I checked the Ivybridge documentation, and found:
>>>
>>> "Software is responsible for ensuring that accesses outside the IB do not
>>> occur. This is possible as software can compute the range of IB values
>>> referenced by a 3DPRIMITIVE command (knowing the StartVertexLocation,
>>> InstanceCount, and VerticesPerInstance values) and can then compare this
>>> range to the IB extent."
>>>
>>> which makes it sound like an accurate computation is necessary. But, right
>>> below that, it says:
>>>
>>> "this field contains the address of the last valid byte in the index buffer.
>>> Any index buffer reads past this address returns an index value of 0 (as if
>>> the index buffer was zero-extended)."
>>>
>>> So the earlier statement is false; i965 will draw the in-bounds elements
>>> correctly, and then repeat element 0 over and over for any out-of-bounds
>>> data,
>>> resulting in one strange primitive and a lot of degenerate ones.
>>>
>>> It's proabbly worth fixing, but I doubt it's critical either.
>>
>> Hmm... I came across this while looking at cachegrind traces of GL
>> applications. Time spent in _mesa_validate_Draw* was non-trivial.
>> Since at least some hardware doesn't need this check, I think I want to
>> move it down into drivers that actually need the check... which is kind
>> of a bummer since I came up with a clever optimization for index_bytes.
>
> I'm not sure what applications you looked at in cachegrind but OpenArena
> and Nexuiz both spend a lot of time in here. On my Ivybridge:
>
> OpenArena 5.4% out of a total 27.94% in i965_dri.so
> Nexuiz 3.28% out of a total of 29.4% in i965_dri.so
I was looking at an apitrace of Tesseract. It's not spending quite as
much time in these functions as OpenArena and Nexuiz, but it's still
quite a bit of time.
> For those not up to speed are you able to give a one line explanation of
> why some hardware can get away without this check?
DX10 requires that the hardware bounds-check accesses to buffer objects.
In most cases DX10 requires that out-of-bounds reads return zero, and
out-of-bounds writes are always dropped.
>>> A more interesting thing to fix, I think, would be enforcing alignment
>>> restrictions (i.e. your offset has to be a multiple of the IB element size).
>>
>> That would probably be useful in debug builds, but I'm pretty sure the
>> GL spec says the behavior is undefined specifically to avoid the check
>> in the hot path.
>>
>>> --Ken
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
