C++ PATCH for c++/85045, ICE when printing RDIV_EXPR

Message ID 20180323104900.GN3522@redhat.com
State New
Headers show
Series
  • C++ PATCH for c++/85045, ICE when printing RDIV_EXPR
Related show

Commit Message

Marek Polacek March 23, 2018, 10:49 a.m.
cxx_pretty_printer::multiplicative_expression didn't handle RDIV_EXPR, so when
we tried to print a RDIV_EXPR, we printed it as a pm-expression.  That led to
printing it as a cast-expression.  That led to printing it as a
primary-expression.  That led to printing it as a multiplicative expression.
That led to printing it as a pm-expression.  That led to printing it as a
cast-expression.  That led to printing it as a primary-expression.  That led
to... a crash.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-03-23  Marek Polacek  <polacek@redhat.com>

	PR c++/85045
	* cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression):
	Handle RDIV_EXPR.

	* g++.dg/cpp0x/Wnarrowing5.C: New test.


	Marek

Comments

Jakub Jelinek March 23, 2018, 10:59 a.m. | #1
On Fri, Mar 23, 2018 at 11:49:00AM +0100, Marek Polacek wrote:
> cxx_pretty_printer::multiplicative_expression didn't handle RDIV_EXPR, so when

> we tried to print a RDIV_EXPR, we printed it as a pm-expression.  That led to

> printing it as a cast-expression.  That led to printing it as a

> primary-expression.  That led to printing it as a multiplicative expression.

> That led to printing it as a pm-expression.  That led to printing it as a

> cast-expression.  That led to printing it as a primary-expression.  That led

> to... a crash.

> 

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

> 

> 2018-03-23  Marek Polacek  <polacek@redhat.com>

> 

> 	PR c++/85045

> 	* cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression):

> 	Handle RDIV_EXPR.

> 

> 	* g++.dg/cpp0x/Wnarrowing5.C: New test.


I think that is not enough.  This is closely related to c-pretty-print.c,
which has:
c_pretty_printer::multiplicative_expression
    case MULT_EXPR:
    case TRUNC_DIV_EXPR:
    case TRUNC_MOD_EXPR:
    case EXACT_DIV_EXPR:
    case RDIV_EXPR:
and is broken:
      else if (code == TRUNC_DIV_EXPR)
        pp_slash (this);
      else
        pp_modulo (this);
That code == TRUNC_DIV_EXPR really should have been code != TRUNC_MOD_EXPR,
because all the above expressions are / except MULT_EXPR handled earlier and
TRUNC_MOD_EXPR.
c_pretty_printer::expression
then has:
    case MULT_EXPR:
    case TRUNC_MOD_EXPR:
    case TRUNC_DIV_EXPR:
    case EXACT_DIV_EXPR:
    case RDIV_EXPR:
      multiplicative_expression (e);
      break;

So, I think you want:
1) in cxx_pretty_printer::multiplicative_expression add
EXACT_DIV_EXPR and (like you did) RDIV_EXPR, and change the pp_slash
condition to code != TRUNC_MOD_EXPR
2) in cxx_pretty_printer::expression above the multiplicative_expression
call add case EXACT_DIV_EXPR: and case RDIV_EXPR
3) in c_pretty_printer::multiplicative_expression change the
code == TRUNC_DIV_EXPR condition to code != TRUNC_MOD_EXPR.
4) see if you can reproduce the c-pretty-print.c bug in some C testcase,
whether you get really % instead of / printed for floating point division.

> 

> diff --git gcc/cp/cxx-pretty-print.c gcc/cp/cxx-pretty-print.c

> index ca99997f5e1..8527a326c57 100644

> --- gcc/cp/cxx-pretty-print.c

> +++ gcc/cp/cxx-pretty-print.c

> @@ -899,11 +899,12 @@ cxx_pretty_printer::multiplicative_expression (tree e)

>      case MULT_EXPR:

>      case TRUNC_DIV_EXPR:

>      case TRUNC_MOD_EXPR:

> +    case RDIV_EXPR:

>        multiplicative_expression (TREE_OPERAND (e, 0));

>        pp_space (this);

>        if (code == MULT_EXPR)

>  	pp_star (this);

> -      else if (code == TRUNC_DIV_EXPR)

> +      else if (code == TRUNC_DIV_EXPR || code == RDIV_EXPR)

>  	pp_slash (this);

>        else

>  	pp_modulo (this);

> diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C

> index e69de29bb2d..9acdc6b0c91 100644

> --- gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C

> +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C

> @@ -0,0 +1,11 @@

> +// PR c++/85045

> +// { dg-do compile { target c++11 } }

> +

> +typedef struct tt {

> +  unsigned short h;

> +} tt;

> +

> +void mainScreen(float a)

> +{

> +  tt numlrect = {int(100/a)}; // { dg-error "narrowing conversion" }

> +}


	Jakub
Marek Polacek March 23, 2018, 1:16 p.m. | #2
On Fri, Mar 23, 2018 at 11:59:04AM +0100, Jakub Jelinek wrote:
> On Fri, Mar 23, 2018 at 11:49:00AM +0100, Marek Polacek wrote:

> > cxx_pretty_printer::multiplicative_expression didn't handle RDIV_EXPR, so when

> > we tried to print a RDIV_EXPR, we printed it as a pm-expression.  That led to

> > printing it as a cast-expression.  That led to printing it as a

> > primary-expression.  That led to printing it as a multiplicative expression.

> > That led to printing it as a pm-expression.  That led to printing it as a

> > cast-expression.  That led to printing it as a primary-expression.  That led

> > to... a crash.

> > 

> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> > 

> > 2018-03-23  Marek Polacek  <polacek@redhat.com>

> > 

> > 	PR c++/85045

> > 	* cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression):

> > 	Handle RDIV_EXPR.

> > 

> > 	* g++.dg/cpp0x/Wnarrowing5.C: New test.

> 

> I think that is not enough.  This is closely related to c-pretty-print.c,

> which has:

> c_pretty_printer::multiplicative_expression

>     case MULT_EXPR:

>     case TRUNC_DIV_EXPR:

>     case TRUNC_MOD_EXPR:

>     case EXACT_DIV_EXPR:

>     case RDIV_EXPR:

> and is broken:

>       else if (code == TRUNC_DIV_EXPR)

>         pp_slash (this);

>       else

>         pp_modulo (this);

> That code == TRUNC_DIV_EXPR really should have been code != TRUNC_MOD_EXPR,

> because all the above expressions are / except MULT_EXPR handled earlier and

> TRUNC_MOD_EXPR.

> c_pretty_printer::expression

> then has:

>     case MULT_EXPR:

>     case TRUNC_MOD_EXPR:

>     case TRUNC_DIV_EXPR:

>     case EXACT_DIV_EXPR:

>     case RDIV_EXPR:

>       multiplicative_expression (e);

>       break;


You're right, sorry for being sloppy.

> So, I think you want:

> 1) in cxx_pretty_printer::multiplicative_expression add

> EXACT_DIV_EXPR and (like you did) RDIV_EXPR, and change the pp_slash

> condition to code != TRUNC_MOD_EXPR

> 2) in cxx_pretty_printer::expression above the multiplicative_expression

> call add case EXACT_DIV_EXPR: and case RDIV_EXPR

> 3) in c_pretty_printer::multiplicative_expression change the

> code == TRUNC_DIV_EXPR condition to code != TRUNC_MOD_EXPR.


Done.

> 4) see if you can reproduce the c-pretty-print.c bug in some C testcase,

> whether you get really % instead of / printed for floating point division.


I've found one, though it's quite bizzare.  But it showed that we were printing
"a % b" despite the user code having "a / b".

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-03-23  Marek Polacek  <polacek@redhat.com>

	PR c++/85045
	* c-pretty-print.c (c_pretty_printer::multiplicative_expression)
	<case RDIV_EXPR>: Tweak condition.

	* cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression):
	Handle EXACT_DIV_EXPR and RDIV_EXPR.  Tweak condition.
	(cxx_pretty_printer::expression): Handle EXACT_DIV_EXPR and RDIV_EXPR.

	* g++.dg/cpp0x/Wnarrowing5.C: New test.
	* gcc.dg/pr85045.c: New test.

diff --git gcc/c-family/c-pretty-print.c gcc/c-family/c-pretty-print.c
index c9dd8aefff9..dc76c9957d3 100644
--- gcc/c-family/c-pretty-print.c
+++ gcc/c-family/c-pretty-print.c
@@ -1841,7 +1841,7 @@ c_pretty_printer::multiplicative_expression (tree e)
       pp_c_whitespace (this);
       if (code == MULT_EXPR)
 	pp_c_star (this);
-      else if (code == TRUNC_DIV_EXPR)
+      else if (code != TRUNC_MOD_EXPR)
 	pp_slash (this);
       else
 	pp_modulo (this);
diff --git gcc/cp/cxx-pretty-print.c gcc/cp/cxx-pretty-print.c
index ca99997f5e1..9a5545cb5cc 100644
--- gcc/cp/cxx-pretty-print.c
+++ gcc/cp/cxx-pretty-print.c
@@ -899,11 +899,13 @@ cxx_pretty_printer::multiplicative_expression (tree e)
     case MULT_EXPR:
     case TRUNC_DIV_EXPR:
     case TRUNC_MOD_EXPR:
+    case EXACT_DIV_EXPR:
+    case RDIV_EXPR:
       multiplicative_expression (TREE_OPERAND (e, 0));
       pp_space (this);
       if (code == MULT_EXPR)
 	pp_star (this);
-      else if (code == TRUNC_DIV_EXPR)
+      else if (code != TRUNC_MOD_EXPR)
 	pp_slash (this);
       else
 	pp_modulo (this);
@@ -1113,6 +1115,8 @@ cxx_pretty_printer::expression (tree t)
     case MULT_EXPR:
     case TRUNC_DIV_EXPR:
     case TRUNC_MOD_EXPR:
+    case EXACT_DIV_EXPR:
+    case RDIV_EXPR:
       multiplicative_expression (t);
       break;
 
diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C
index e69de29bb2d..d2abf03313e 100644
--- gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C
+++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C
@@ -0,0 +1,11 @@
+// PR c++/85045
+// { dg-do compile { target c++11 } }
+
+typedef struct tt {
+  unsigned short h;
+} tt;
+
+void mainScreen(float a)
+{
+  tt numlrect = {int(100/a)}; // { dg-error "narrowing conversion" }
+}
diff --git gcc/testsuite/gcc.dg/pr85045.c gcc/testsuite/gcc.dg/pr85045.c
index e69de29bb2d..8c4a7aa36a4 100644
--- gcc/testsuite/gcc.dg/pr85045.c
+++ gcc/testsuite/gcc.dg/pr85045.c
@@ -0,0 +1,9 @@
+/* PR c/85045 */
+/* { dg-do compile } */
+/* { dg-options "-fno-diagnostics-show-caret" } */
+
+void
+f (double a, double b)
+{
+  (a / b) (); /* { dg-error "called object .a / b. is not a function or function pointer" } */
+}

	Marek
Jakub Jelinek March 23, 2018, 1:18 p.m. | #3
On Fri, Mar 23, 2018 at 02:16:32PM +0100, Marek Polacek wrote:
> > So, I think you want:

> > 1) in cxx_pretty_printer::multiplicative_expression add

> > EXACT_DIV_EXPR and (like you did) RDIV_EXPR, and change the pp_slash

> > condition to code != TRUNC_MOD_EXPR

> > 2) in cxx_pretty_printer::expression above the multiplicative_expression

> > call add case EXACT_DIV_EXPR: and case RDIV_EXPR

> > 3) in c_pretty_printer::multiplicative_expression change the

> > code == TRUNC_DIV_EXPR condition to code != TRUNC_MOD_EXPR.

> 

> Done.

> 

> > 4) see if you can reproduce the c-pretty-print.c bug in some C testcase,

> > whether you get really % instead of / printed for floating point division.

> 

> I've found one, though it's quite bizzare.  But it showed that we were printing

> "a % b" despite the user code having "a / b".

> 

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

> 

> 2018-03-23  Marek Polacek  <polacek@redhat.com>

> 

> 	PR c++/85045

> 	* c-pretty-print.c (c_pretty_printer::multiplicative_expression)

> 	<case RDIV_EXPR>: Tweak condition.

> 

> 	* cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression):

> 	Handle EXACT_DIV_EXPR and RDIV_EXPR.  Tweak condition.

> 	(cxx_pretty_printer::expression): Handle EXACT_DIV_EXPR and RDIV_EXPR.

> 

> 	* g++.dg/cpp0x/Wnarrowing5.C: New test.

> 	* gcc.dg/pr85045.c: New test.


Ok, thanks.

	Jakub

Patch

diff --git gcc/cp/cxx-pretty-print.c gcc/cp/cxx-pretty-print.c
index ca99997f5e1..8527a326c57 100644
--- gcc/cp/cxx-pretty-print.c
+++ gcc/cp/cxx-pretty-print.c
@@ -899,11 +899,12 @@  cxx_pretty_printer::multiplicative_expression (tree e)
     case MULT_EXPR:
     case TRUNC_DIV_EXPR:
     case TRUNC_MOD_EXPR:
+    case RDIV_EXPR:
       multiplicative_expression (TREE_OPERAND (e, 0));
       pp_space (this);
       if (code == MULT_EXPR)
 	pp_star (this);
-      else if (code == TRUNC_DIV_EXPR)
+      else if (code == TRUNC_DIV_EXPR || code == RDIV_EXPR)
 	pp_slash (this);
       else
 	pp_modulo (this);
diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C
index e69de29bb2d..9acdc6b0c91 100644
--- gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C
+++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C
@@ -0,0 +1,11 @@ 
+// PR c++/85045
+// { dg-do compile { target c++11 } }
+
+typedef struct tt {
+  unsigned short h;
+} tt;
+
+void mainScreen(float a)
+{
+  tt numlrect = {int(100/a)}; // { dg-error "narrowing conversion" }
+}