[Mesa-dev] [PATCH v2 2/2] glsl: raise warning when using uninitialized variables

[Alejandro Piñeiro]

On 18/04/16 00:47, Ilia Mirkin wrote:
> On Mon, Mar 28, 2016 at 2:50 PM, Alejandro Piñeiro <apinheiro at igalia.com> wrote:
>> v2:
>>  * Take into account out varyings too (Timothy Arceri)
>>  * Fix style (Timothy Arceri)
>>  * Use a new ast_expression variable, instead of an
>>    ast_expression::hir new parameter (Timothy Arceri)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94129
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
>> index eb45f29..e38ab10 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -1901,6 +1901,13 @@ ast_expression::do_hir(exec_list *instructions,
>>        if (var != NULL) {
>>           var->data.used = true;
>>           result = new(ctx) ir_dereference_variable(var);
>> +
>> +         if ((var->data.mode == ir_var_auto || var->data.mode == ir_var_shader_out)
>> +             && !this->is_lhs
>> +             && result->variable_referenced()->data.assigned != true) {
>> +            _mesa_glsl_warning(&loc, state, "`%s' used uninitialized",
>> +                               this->primary_expression.identifier);
> This also appears to warn in the case of
>
> void foo(out float x) { x = 1.0; }
>
> float bar;
> foo(bar);
>
> It thinks that bar is being used uninitialized. How do we fix this? It
> happens a ton in Talos Principle.

After checking a little: initially I thought it would be easy to solve
because there is already some checks related to the parameters modes
(in, out, inout) at verify_parameter_modes (ast_function.cpp). This
method compares the formal parameter (x in your example) and the actual
parameter (bar in your example), and checks that the modes are correct
and other stuff, like for example setting assigned to true on the actual
parameter. The problem is that this is done after the actual variable is
processed, that is where the warning is raised. And if fact, in order to
work, the actual parameter needs to be already processed, as the modes
are part of the ir variable, not of the ast_node.

I think that a way to solve that would be set is_lhs for any function
parameter, and raise the uninitialized warning too on
verify_parameter_modes. That would mean that we raise the warning in two
different places, but I think that it would not be a big issue, because
as mentioned, that code is already checking errors related with the
actual and formal parameters and their modes.

I will try to do this tomorrow.

BR