[Mesa-dev] [PATCH] etnaviv: Don't try to use the index buffer if size is zero

[Tomeu Vizoso]

On 05/29/2017 02:47 PM, Lucas Stach wrote:
> Hi Tomeu,
> 
> Am Freitag, den 19.05.2017, 12:40 +0200 schrieb Tomeu Vizoso:
>> If info->index_size is zero, info->index will point to uninitialized
>> memory.
>>
>> Fatal signal 11 (SIGSEGV), code 2, fault addr 0xab5d07a3 in tid 20456 (surfaceflinger)
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>
>> Cc: etnaviv at lists.freedesktop.org
>> Cc: Marek Olšák <marek.olsak at amd.com>
>> Fixes: 330d0607ed60 ("gallium: remove pipe_index_buffer and set_index_buffer")
>> ---
>>  src/gallium/drivers/etnaviv/etnaviv_context.c | 36 +++++++++++++++------------
>>  1 file changed, 20 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c b/src/gallium/drivers/etnaviv/etnaviv_context.c
>> index 306cb6fc498d..dcda4088bfd5 100644
>> --- a/src/gallium/drivers/etnaviv/etnaviv_context.c
>> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
>> @@ -178,24 +178,28 @@ etna_draw_vbo(struct pipe_context *pctx, const struct pipe_draw_info *info)
>>  
>>     /* Upload a user index buffer. */
>>     unsigned index_offset = 0;
>> -   struct pipe_resource *indexbuf = info->has_user_indices ? NULL : info->index.resource;
>> -   if (info->index_size && info->has_user_indices &&
>> -       !util_upload_index_buffer(pctx, info, &indexbuf, &index_offset)) {
>> -      BUG("Index buffer upload failed.");
>> -      return;
>> -   }
>> +   struct pipe_resource *indexbuf = NULL;
>>  
>> -   if (info->index_size && indexbuf) {
>> -      ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo = etna_resource(indexbuf)->bo;
>> -      ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.offset = index_offset;
>> -      ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.flags = ETNA_RELOC_READ;
>> -      ctx->index_buffer.FE_INDEX_STREAM_CONTROL = translate_index_size(info->index_size);
>> -      ctx->dirty |= ETNA_DIRTY_INDEX_BUFFER;
>> -   }
>> +   if (info->index_size) {
>> +      indexbuf = info->has_user_indices ? NULL : info->index.resource;
>> +      if (info->has_user_indices &&
>> +          !util_upload_index_buffer(pctx, info, &indexbuf, &index_offset)) {
>> +         BUG("Index buffer upload failed.");
>> +         return;
>> +      }
>>  
>> -   if (info->index_size && !ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo) {
>> -      BUG("Unsupported or no index buffer");
>> -      return;
>> +      if (indexbuf) {
> 
> This can be simplified: If this is a indexed draw (index_size != 0) then
> we will have a indexbuf at that point in the code, as its either
> provided or constructed via the util_upload_index_buffer() call.
> 
> Mind if squash this simplification into your patch while pushing?

Sounds good to me.

Thanks,

Tomeu

> Regards,
> Lucas
> 
>> +         ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo = etna_resource(indexbuf)->bo;
>> +         ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.offset = index_offset;
>> +         ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.flags = ETNA_RELOC_READ;
>> +         ctx->index_buffer.FE_INDEX_STREAM_CONTROL = translate_index_size(info->index_size);
>> +         ctx->dirty |= ETNA_DIRTY_INDEX_BUFFER;
>> +      }
>> +
>> +      if (!ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo) {
>> +         BUG("Unsupported or no index buffer");
>> +         return;
>> +      }
>>     }
>>  
>>     struct etna_shader_key key = {};
> 
>