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

2015-07-12 Thread Renaud Gaubert 
Hi Samuel.

Thanks, patch v2 is on the way.


 I guess the piglit tests you mention in the commit log are going to
 check these cases, right?
That's right following tests are done:
  * binary operator
  * unary operators
  * ternary operator
  * comparison operators (except equal and nequal operator)
  * assignement operators (=, +=, =, ...)

These are added in the following patches sent to the piglit mailing list:
  * glsl 1.10: Fix broken ternary void test
  * glsl-1.10 Adds tests on how void functions are handled

 Have you run piglit to verify
 that it doesn't add any regression?
Yes I have. It doesn't

-- 
Renaud Gaubert
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-07-09 Thread Renaud Gaubert 
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 ren...@lse.epita.fr
Reviewed-by: Gabriel Laskar gabr...@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);
+  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 
-- 
2.4.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-07-09 Thread Renaud Gaubert 
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 ren...@lse.epita.fr
Reviewed-by: Gabriel Laskar gabr...@lse.epita.fr
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85252
---
 src/glsl/ast_function.cpp |  7 ++-
 src/glsl/ast_to_hir.cpp   | 10 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 92e26bf..3c2b1ea 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -1785,7 +1785,12 @@ 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);
+  value = new(ctx) ir_dereference_variable(tmp);
+  instructions-push_tail(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 
-- 
2.4.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-07-09 Thread 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 ren...@lse.epita.fr
 Reviewed-by: Gabriel Laskar gabr...@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 
 
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-07-07 Thread Renaud Gaubert 
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 ren...@lse.epita.fr
Reviewed-by: Gabriel Laskar gabr...@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);
+  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 
-- 
2.4.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev