Bug 49165 - [4.4/4.5 Regression] ICE on for-loop/throw combination
Summary: [4.4/4.5 Regression] ICE on for-loop/throw combination
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.5.1
: P2 normal
Target Milestone: 4.4.7
Assignee: Jakub Jelinek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-25 18:14 UTC by Vijay Rao
Modified: 2011-07-19 19:50 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-05-25 18:43:15


Attachments
gcc46-pr49165.patch (810 bytes, patch)
2011-05-26 07:27 UTC, Jakub Jelinek
Details | Diff
gcc46-pr49165.patch (771 bytes, patch)
2011-05-27 12:30 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vijay Rao 2011-05-25 18:14:04 UTC
Source file /tmp/redux.cc:

#include <iostream>
#include <string>

int main(int argc,char**argv)
{
  std::string str;
  for ( int field(0);
        (field<10) && ((std::cin>>str)?true:(throw std::string("failure")));
        field++ );
}

produces:

/tmp/redux.cc: In function ‘int main(int, char**)’:
/tmp/redux.cc:7:3: internal compiler error: in fold_convert_loc, at fold-const.c:2670
Comment 1 Vijay Rao 2011-05-25 18:34:31 UTC
/tmp/redux.cc

#include <iostream>
#include <string>

int main(int argc,char**argv)
{
  std::string str;
  for ( int field(0);
        (field<10) && ((std::cin>>str)? 1 :(throw std::string("failure")));
        field++ );
}

g++ /tmp/redux.cc 
/tmp/redux.cc: In function ‘int main(int, char**)’:
/tmp/redux.cc:8:65: error: void value not ignored as it ought to be

This bug looks a lot like what I reported some years ago:

This:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14083

shows that it should be accepted by the compiler.
Comment 2 Jakub Jelinek 2011-05-25 18:43:15 UTC
Shorter testcase:
int
foo (bool x, int y)
{
  if (y < 10 && (x ? true : throw 1))
    y++;
  return y;
}

ICE started between r112000 and r113000, before that it failed to compile with
error: void value not ignored as it ought to be error.
The ICE is during gimple_boolify, throw has void_type_node and fold_convert to boolean_type_node ICEs.
Comment 3 Jakub Jelinek 2011-05-25 19:47:52 UTC
--- gimplify.c (revision 174199)
+++ gimplify.c (working copy)
@@ -2848,7 +2848,7 @@
     default:
       /* Other expressions that get here must have boolean values, but
          might need to be converted to the appropriate mode.  */
-      if (type == boolean_type_node)
+      if (type == boolean_type_node || type == void_type_node)
         return expr;
       return fold_convert_loc (loc, boolean_type_node, expr);
     }
fixes the ICE, but we error out then still during gimplification:
error: using result of function returning ‘void’
I think the only problematic case is when one COND_EXPR arm is THROW_EXPR (i.e. void_type_node) and the other one has gimple reg type.  Either the FE (genericization or during gimplification) should replace THROW_EXPR with
COMPOUND_EXPR <THROW_EXPR, <fold_convert <type, integer_zero_node>>>, or gimplifier needs to special case it in some places.  Is the only possibility to create COND_EXPR in C++ with void_type_node on one arm and some other type on the other arm and COND_EXPR itself a throw?
Comment 4 Jason Merrill 2011-05-25 20:04:47 UTC
Yes, that's right.  5.16/2:

If either the second or the third operand has type (possibly cv-qualified) void, then the lvalue-to-rvalue (4.1), array-to-pointer (4.2), and function-to-pointer (4.3) standard conversions are performed on the second and third operands, and one of the following shall hold:
— The second or the third operand (but not both) is a throw-expression (15.1); the result is of the type of the other and is a prvalue.
— Both the second and the third operands have type void; the result is of type void and is a prvalue. [ Note: This includes the case where both operands are throw-expressions. — end note ]
Comment 5 Jakub Jelinek 2011-05-26 07:27:45 UTC
Created attachment 24361 [details]
gcc46-pr49165.patch

Fix.  Will commit after bootstrap/regtest.
Comment 6 Richard Biener 2011-05-26 08:25:26 UTC
	   && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 1)))
2578 	   && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 2))))

shouldn't either none or both arms be VOID_TYPE_P?  So,

           && (!VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 1)))
               || !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 2))))
       {
         gcc[_checking]_assert (!VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 1))))
&& !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 2))))
...

would be better?
Comment 7 Jakub Jelinek 2011-05-26 08:47:36 UTC
No.
Both arms shouldn't be VOID_TYPE_P, otherwise the COND_EXPR would be VOID_TYPE_P too and we really shouldn't be using the COND_EXPR's value.
The problem is that the C++ FE gives us
COND_EXPR with bool type (or some other type), with one arm the same type and
the other arm THROW_EXPR (i.e. VOID_TYPE_P).
The gimplifier is already aware of this and handles it right:
      /* Build the new then clause, `tmp = then_;'.  But don't build the
         assignment if the value is void; in C++ it can be if it's a throw.  */
      if (!VOID_TYPE_P (TREE_TYPE (then_)))
        TREE_OPERAND (expr, 1) = build2 (MODIFY_EXPR, type, tmp, then_);
         
      /* Similarly, build the new else clause, `tmp = else_;'.  */
      if (!VOID_TYPE_P (TREE_TYPE (else_)))
        TREE_OPERAND (expr, 2) = build2 (MODIFY_EXPR, type, tmp, else_);
just shortcut_cond_r didn't handle it well (it created
COND_EXPR with void type throw ? goto L1 : goto L2).
Comment 8 Jakub Jelinek 2011-05-26 10:25:23 UTC
Author: jakub
Date: Thu May 26 10:25:21 2011
New Revision: 174273

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174273
Log:
	PR c++/49165
	* gimplify.c (shortcut_cond_r): Don't special case
	COND_EXPRs if they have void type on one of their arms.

	* g++.dg/eh/cond5.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/eh/cond5.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimplify.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 Jakub Jelinek 2011-05-26 10:27:59 UTC
Author: jakub
Date: Thu May 26 10:27:57 2011
New Revision: 174274

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174274
Log:
	PR c++/49165
	* gimplify.c (shortcut_cond_r): Don't special case
	COND_EXPRs if they have void type on one of their arms.

	* g++.dg/eh/cond5.C: New test.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/g++.dg/eh/cond5.C
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/gimplify.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Comment 10 Jakub Jelinek 2011-05-26 10:46:40 UTC
Fixed for 4.6+ so far.
Comment 11 Vijay Rao 2011-05-27 07:58:25 UTC
Does this fix the situation when the 2nd operand of the conditional operator is of type int? 

extern "C" void abort ();

int bar (bool x, int y)
{
  if (y < 10 && (x ? 1 : throw 1))
    y++;
  if (y > 20 || (x ? 1 : throw 2))
    y++;
  return y;
}

int main ()
{
  if (bar (true, 0) != 2
      || bar (true, 10) != 11
      || bar (false, 30) != 31)
    abort ();
  try
    {
      bar (false, 0);
      abort ();
    }
  catch (int i)
    {
      if (i != 1)
        abort ();
    }
  try
    {
      bar (false, 10);
      abort ();
    }
  catch (int i)
    {
      if (i != 2)
        abort ();
    }
}
Comment 12 Jakub Jelinek 2011-05-27 12:30:29 UTC
Created attachment 24369 [details]
gcc46-pr49165.patch

Indeed, c_common_truthvalue_conversion needs to be aware of it too.
Comment 13 Jakub Jelinek 2011-05-27 19:19:39 UTC
Author: jakub
Date: Fri May 27 19:19:36 2011
New Revision: 174350

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174350
Log:
	PR c++/49165
	* c-common.c (c_common_truthvalue_conversion) <case COND_EXPR>: For
	C++ don't call c_common_truthvalue_conversion on void type arms.

	* g++.dg/eh/cond6.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/eh/cond6.C
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/testsuite/ChangeLog
Comment 14 Jakub Jelinek 2011-05-27 19:23:49 UTC
Author: jakub
Date: Fri May 27 19:23:46 2011
New Revision: 174351

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174351
Log:
	PR c++/49165
	* c-common.c (c_common_truthvalue_conversion) <case COND_EXPR>: For
	C++ don't call c_common_truthvalue_conversion on void type arms.

	* g++.dg/eh/cond6.C: New test.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/g++.dg/eh/cond6.C
Modified:
    branches/gcc-4_6-branch/gcc/c-family/ChangeLog
    branches/gcc-4_6-branch/gcc/c-family/c-common.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Comment 15 Vijay Rao 2011-05-27 22:19:00 UTC
Ok, thanks for this. While using 4.5.1 I'm also stuck in a toolchain using 3.4.6 for which the latter doesn't fail when one of the conditional args is of type bool but does give the "void value not ignored as it ought to be" error when one of the args is of int type which I wasn't getting while using 3.3.2. I was getting an ICE when using bool for one of the conditional args in 3.3.2 (reported in 14083).

I was wondering whether to try a compiler version > 3.3.2 and < 3.4.6 but if both issues are going to be covered then it'll be worth the effort, ceteris paribus, of introducing a version 4 compiler in the toolchain. 

Just goofing around with test cases code:

template<typename R, typename Y, typename C>
struct X 
{
  R f(int x, Y y);
};

template<typename R, typename Y>
struct X<R,Y,bool> 
{
  R f(int x, Y y) {
    return y && (x<2?true:throw 14);
  }
};

template<typename R, typename Y>
struct X<R,Y,int> 
{
  R f(int x, Y y) {
    return y && (x<2? 1 :throw 14);
  }
};

int main(int argc,char**argv)
{
  int x(2);
  try {
    int j(X<int,int,int>()
          .f(x,
             X<int,int,bool>()
             .f(x,
                X<int,bool,int>()
                .f(x,
                   X<bool,bool,bool>()
                   .f(x,
                      X<bool,bool,int>()
                      .f(x,
                         X<bool,int,int>()
                         .f(x, x)
                         )
                      )
                   )
                )
             )
          );
  } catch ( int e ) { if (e!=14) throw; }

  x=argc;
  int k(X<int,int,int>()
        .f(x,
           X<int,int,bool>()
           .f(x,
              X<int,bool,int>()
              .f(x,
                 X<bool,bool,bool>()
                 .f(x,
                    X<bool,bool,int>()
                    .f(x,
                       X<bool,int,int>()
                       .f(x, x)
                       )
                    )
                 )
              )
           )
        );
}
Comment 16 Richard Biener 2011-06-27 12:13:28 UTC
4.3 branch is being closed, moving to 4.4.7 target.
Comment 17 Jakub Jelinek 2011-07-19 12:54:37 UTC
Author: jakub
Date: Tue Jul 19 12:54:34 2011
New Revision: 176452

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176452
Log:
	Backport from mainline
	2011-05-26  Jakub Jelinek  <jakub@redhat.com>

	PR c++/49165
	* gimplify.c (shortcut_cond_r): Don't special case
	COND_EXPRs if they have void type on one of their arms.

	* g++.dg/eh/cond5.C: New test.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/eh/cond5.C
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/gimplify.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
Comment 18 Jakub Jelinek 2011-07-19 12:56:50 UTC
Author: jakub
Date: Tue Jul 19 12:56:48 2011
New Revision: 176453

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176453
Log:
	Backport from mainline
	2011-05-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/49165
	* c-common.c (c_common_truthvalue_conversion) <case COND_EXPR>: For
	C++ don't call c_common_truthvalue_conversion on void type arms.

	* g++.dg/eh/cond6.C: New test.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/eh/cond6.C
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/c-common.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
Comment 19 Jakub Jelinek 2011-07-19 19:30:01 UTC
Author: jakub
Date: Tue Jul 19 19:29:57 2011
New Revision: 176483

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176483
Log:
	Backport from mainline
	2011-05-26  Jakub Jelinek  <jakub@redhat.com>

	PR c++/49165
	* gimplify.c (shortcut_cond_r): Don't special case
	COND_EXPRs if they have void type on one of their arms.

	* g++.dg/eh/cond5.C: New test.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/eh/cond5.C
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/gimplify.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog
Comment 20 Jakub Jelinek 2011-07-19 19:31:04 UTC
Author: jakub
Date: Tue Jul 19 19:31:01 2011
New Revision: 176484

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176484
Log:
	Backport from mainline
	2011-05-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/49165
	* c-common.c (c_common_truthvalue_conversion) <case COND_EXPR>: For
	C++ don't call c_common_truthvalue_conversion on void type arms.

	* g++.dg/eh/cond6.C: New test.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/eh/cond6.C
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/c-common.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog
Comment 21 Jakub Jelinek 2011-07-19 19:50:57 UTC
Fixed.