[Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Paul Berry]

On 16 December 2011 11:09, Ian Romanick <idr at freedesktop.org> wrote:

> On 12/16/2011 10:44 AM, Paul Berry wrote:
>
>> On 16 December 2011 10:04, Paul Berry <stereotype441 at gmail.com
>> <mailto:stereotype441 at gmail.**com <stereotype441 at gmail.com>>> wrote:
>>
>>    On 15 December 2011 15:20, Kenneth Graunke <kenneth at whitecape.org
>>    <mailto:kenneth at whitecape.org>**> wrote:
>>
>>        Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
>>        <mailto:kenneth at whitecape.org>**>
>>
>>        ---
>>          src/mesa/drivers/dri/i965/brw_**context.c |    1 +
>>          src/mesa/drivers/dri/i965/brw_**context.h |    3 ++
>>          src/mesa/drivers/dri/i965/brw_**gs_emit.c |   10 ++++++++
>>          src/mesa/drivers/dri/i965/**gen6_sol.c    |   38
>>        ++++++++++++++++++++++++++++++**+
>>          4 files changed, 52 insertions(+), 0 deletions(-)
>>
>>        diff --git a/src/mesa/drivers/dri/i965/**brw_context.c
>>        b/src/mesa/drivers/dri/i965/**brw_context.c
>>        index 7d360b0..fd60853 100644
>>        --- a/src/mesa/drivers/dri/i965/**brw_context.c
>>        +++ b/src/mesa/drivers/dri/i965/**brw_context.c
>>        @@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct
>>        dd_function_table *functions )
>>            brw_init_queryobj_functions(**functions);
>>
>>            functions->PrepareExecBegin = brwPrepareExecBegin;
>>        +   functions->**BeginTransformFeedback =
>>        brw_begin_transform_feedback;
>>            functions->**EndTransformFeedback =
>> brw_end_transform_feedback;
>>          }
>>
>>        diff --git a/src/mesa/drivers/dri/i965/**brw_context.h
>>        b/src/mesa/drivers/dri/i965/**brw_context.h
>>        index 8e52488..20623d4 100644
>>        --- a/src/mesa/drivers/dri/i965/**brw_context.h
>>        +++ b/src/mesa/drivers/dri/i965/**brw_context.h
>>        @@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(**const struct
>>        gl_fragment_program *fprog);
>>
>>          /* gen6_sol.c */
>>          void
>>        +brw_begin_transform_feedback(**struct gl_context *ctx,
>>        +                            struct gl_transform_feedback_object
>>        *obj);
>>        +void
>>          brw_end_transform_feedback(**struct gl_context *ctx,
>>                                    struct gl_transform_feedback_object
>>        *obj);
>>
>>        diff --git a/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>        b/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>        index 72d4eca..5dd3734 100644
>>        --- a/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>        +++ b/src/mesa/drivers/dri/i965/**brw_gs_emit.c
>>        @@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c,
>>        struct brw_gs_prog_key *key,
>>                */
>>               brw_MOV(p, get_element_ud(c->reg.header, 5),
>>                       get_element_ud(c->reg.SVBI, 0));
>>        +
>>        +      /* Make sure that the buffers have enough room for all
>>        the vertices. */
>>        +      brw_ADD(p, get_element_ud(c->reg.temp, 0),
>>        +                get_element_ud(c->reg.SVBI, 0),
>>        brw_imm_ud(num_verts));
>>        +      brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
>>        +                get_element_ud(c->reg.temp, 0),
>>        +                get_element_ud(c->reg.SVBI, 4));
>>        +      brw_IF(p, BRW_EXECUTE_1);
>>        +
>>
>>
>> Whoops, one other correction.  There's an off-by-one error--this should
>> be BRW_CONDITIONAL_LE.
>>
>> For example, let's say we're outputting triangles, the transform
>> feedback buffer is large enough to hold 6 vertices, and we've output 3
>> vertices already.  In that case num_verts is 3, SVBI.0 is 3, and SVBI.4
>> is 6.  The above logic compares SVBI.0 + num_verts < SVBI.4 (6 < 6), so
>> it concludes that there isn't room for the second triangle.  It should
>> be computing SVBI.0 + num_verts <= SVBI.4 (6 <= 6), because there is
>> just barely room for the second triangle.
>>
>
> Do we have piglit test cases for these edge conditions?  If we don't, we
> need them. :)
>

This one was caught by piglit tests that I'm hoping to send to the list
today.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111216/b78d07a5/attachment.htm>