Should this patch be cc'd to stable branches? Without it, the compiler crashes on invalid inputs, instead of generating an error.
This patch applies cleanly to 10.5 and 10.6. -Mark Renaud Gaubert <ren...@lse.epita.fr> writes: > 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 assignment of a void variable > which will be optimized out (the assignment) 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. > > Signed-off-by: Renaud Gaubert <ren...@lse.epita.fr> > Reviewed-by: Gabriel Laskar <gabr...@lse.epita.fr> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85252 > > --- > Piglit tests were sent to the Piglit mailing list: > * glsl-1.10 Adds tests on how void functions are handled > > src/glsl/ast_function.cpp | 9 ++++++++- > src/glsl/ast_to_hir.cpp | 9 ++++++++- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp > index 92e26bf..ac32723 100644 > --- a/src/glsl/ast_function.cpp > +++ b/src/glsl/ast_function.cpp > @@ -1785,7 +1785,14 @@ 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); > + instructions->push_tail(tmp); > + 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..0d0ad2a 100644 > --- a/src/glsl/ast_to_hir.cpp > +++ b/src/glsl/ast_to_hir.cpp > @@ -1270,7 +1270,14 @@ 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 > " > -- > 2.4.5 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
