[Mesa-dev] [PATCH] glsl: fix Bug 85252 - Segfault in compiler while processing ternary operator with void arguments

[Samuel Iglesias Gonsálvez]

On 07/07/15 21:47, Renaud Gaubert wrote:
> This is done by returning an rvalue of type void in the
> ast_function_expression::hir function instead of a void expression.
> 
> This produces (in the case of the ternary) an hir with a call
> to the void returning function and an assignement of a void variable
> which will be optimized out (the assignement) during the optimization
> pass.
> 
> This fix results in having a valid subexpression in the many
> different cases where the subexpressions are functions whose
> return values are void.
> 
> Thus preventing to dereference NULL in the following cases:
>   * binary operator
>   * unary operators
>   * ternary operator
>   * comparison operators (except equal and nequal operator)
> 
> Equal and nequal had to be handled as a special case because
> instead of segfaulting on a forbidden syntax it was now accepting
> expressions with a void return value on either (or both) side of
> the expression.
> 
> Piglist tests are on the way
> 
> Signed-off-by: Renaud Gaubert <renaud at lse.epita.fr>
> Reviewed-by: Gabriel Laskar <gabriel at lse.epita.fr>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85252
> ---
>  src/glsl/ast_function.cpp |  6 +++++-
>  src/glsl/ast_to_hir.cpp   | 10 +++++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 92e26bf..776a754 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -1785,7 +1785,11 @@ ast_function_expression::hir(exec_list *instructions,
>  	 /* an error has already been emitted */
>  	 value = ir_rvalue::error_value(ctx);
>        } else {
> -	 value = generate_call(instructions, sig, &actual_parameters, state);
> +        value = generate_call(instructions, sig, &actual_parameters, state);
> +        if (!value) {
> +          ir_variable *const tmp = new(ctx) ir_variable(glsl_type::void_type, "void_var", ir_var_temporary);

You forgot to add the ir_variable to the IR. Something like:

   instructions->push_tail(tmp);

should be enough.

Sam


> +          value = new(ctx) ir_dereference_variable(tmp);
> +        }
>        }
>  
>        return value;
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 8cb46be..00cc16c 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -1270,7 +1270,15 @@ ast_expression::do_hir(exec_list *instructions,
>         *    applied to one operand that can make them match, in which
>         *    case this conversion is done."
>         */
> -      if ((!apply_implicit_conversion(op[0]->type, op[1], state)
> +
> +      if (op[0]->type == glsl_type::void_type || op[1]->type == glsl_type::void_type) {
> +
> +        _mesa_glsl_error(& loc, state, "`%s':  wrong operand types: no operation "
> +          "`%1$s' exists that takes a left-hand operand of type 'void' or a "
> +          "right operand of type 'void'", (this->oper == ast_equal) ? "==" : "!=");
> +
> +         error_emitted = true;
> +      } else if ((!apply_implicit_conversion(op[0]->type, op[1], state)
>             && !apply_implicit_conversion(op[1]->type, op[0], state))
>            || (op[0]->type != op[1]->type)) {
>           _mesa_glsl_error(& loc, state, "operands of `%s' must have the same "
>