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

[Vinson Lee]

On Fri, May 31, 2013 at 1:09 AM, Chia-I Wu <olvaffe at gmail.com> wrote:
> 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?
>

This looks like a false positive. The report did not track the value
of num_shaders.


>>         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