[Mesa-dev] [PATCH] ilo: Ensure array 'shaders' in fully initialized.

[Chia-I Wu]

On Fri, May 31, 2013 at 3:48 PM, Vinson Lee <vlee at freedesktop.org> wrote:
> On Fri, May 31, 2013 at 12:35 AM, Chia-I Wu <olvaffe at gmail.com> wrote:
>> On Fri, May 31, 2013 at 2:59 PM, Vinson Lee <vlee at freedesktop.org> wrote:
>>> Fixes "Uninitialized pointer read" defect reported by Coverity.
>> This looks like a false alarm, as shaders are not read when
>> num_shaders is zero.  Does the report give more details?
>>
I am not sure how to follow the report... I eye-checked the code, and
I failed to spot any uninitialized pointer read.  Do you have an
example that the code would read past what it should?

>         1. var_decl: Declaring variable "shaders" without initializer.
>   86   struct ilo_shader *shaders[PIPE_SHADER_TYPES];
>   87   int num_shaders = 0, i;
>   88
>         2. Condition "i < 4", taking true branch
>         5. Condition "i < 4", taking true branch
>         8. Condition "i < 4", taking false branch
>   89   for (i = 0; i < PIPE_SHADER_TYPES; i++) {
>   90      /* no state bound */
>         3. Condition "!sh[i].state", taking true branch
>         6. Condition "!sh[i].state", taking true branch
>   91      if (!sh[i].state)
>         4. Continuing loop
>         7. Continuing loop
>   92         continue;
>   93
>   94      /* switch variant if the shader or the states it depends on changed */
>   95      if (ilo->dirty & (sh[i].dirty | sh[i].deps)) {
>   96         struct ilo_shader_variant variant;
>   97
>   98         ilo_shader_variant_init(&variant, &sh[i].state->info, ilo);
>   99         ilo_shader_state_use_variant(sh[i].state, &variant);
>  100      }
>  101
>  102      shaders[num_shaders++] = sh[i].state->shader;
>  103   }
>  104
>         Uninitialized pointer read (UNINIT)
>         9. uninit_use_in_call: Using uninitialized element of array
> "shaders" when calling "ilo_shader_cache_set(struct ilo_shader_cache
> *, struct ilo_shader **, int)".
>  105   ilo_shader_cache_set(ilo->shader_cache, shaders, num_shaders);
> /src/gallium/drivers/ilo/ilo_shader.c
> 556ilo_shader_cache_set(struct ilo_shader_cache *shc,
> 557                     struct ilo_shader **shaders,
> 558                     int num_shaders)
> 559{
> 560   int new_cur, i;
> 561
> 562   /* calculate the space needed */
> 563   new_cur = shc->cur;
>         1. Condition "i < num_shaders", taking true branch
> 564   for (i = 0; i < num_shaders; i++) {
>         2. read_parm: Reading a parameter value.
> 565      if (shaders[i]->cache_seqno != shc->seqno)
> 566         new_cur = align(new_cur, 64) + shaders[i]->kernel_size;
> 567   }
>
>
>>> Signed-off-by: Vinson Lee <vlee at freedesktop.org>
>>> ---
>>>  src/gallium/drivers/ilo/ilo_state.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/gallium/drivers/ilo/ilo_state.c b/src/gallium/drivers/ilo/ilo_state.c
>>> index 33da429..f071292 100644
>>> --- a/src/gallium/drivers/ilo/ilo_state.c
>>> +++ b/src/gallium/drivers/ilo/ilo_state.c
>>> @@ -83,7 +83,7 @@ finalize_shader_states(struct ilo_context *ilo)
>>>           .deps = 0,
>>>        },
>>>     };
>>> -   struct ilo_shader *shaders[PIPE_SHADER_TYPES];
>>> +   struct ilo_shader *shaders[PIPE_SHADER_TYPES] = { 0 };
>>>     int num_shaders = 0, i;
>>>
>>>     for (i = 0; i < PIPE_SHADER_TYPES; i++) {
>>> --
>>> 1.8.2.1
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>>
>> --
>> olv at LunarG.com



--
olv at LunarG.com