[Bug middle-end/59630] New: [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[Bug middle-end/59630] New: [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[reichelt at gcc dot gnu.org]

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

            Bug ID: 59630
           Summary: [4.7/4.8/4.9 Regression] ICE converting the return
                    type of a builtin function
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: reichelt at gcc dot gnu.org

The following valid code snippet (compiled with "-std=c99 -O")
triggers an ICE since GCC 4.7.0:

==============================================================
_Bool foo()
{
  _Bool (*f)(int) = __builtin_abs;
  return f(0);
}
==============================================================

bug.c: In function 'foo':
bug.c:3:21: warning: initialization from incompatible pointer type [enabled by
default]
   _Bool (*f)(int) = __builtin_abs;
                     ^
bug.c:5:1: error: non-trivial conversion at assignment
 }
 ^
_Bool
int
_4 = 0;
bug.c:5:1: internal compiler error: verify_gimple failed
0x9f1726 verify_gimple_in_cfg(function*)
        ../../gcc/gcc/tree-cfg.c:4854
0x9195f2 execute_function_todo
        ../../gcc/gcc/passes.c:1850
0x919f43 execute_todo
        ../../gcc/gcc/passes.c:1884
Please submit a full bug report, [etc.]

[Bug middle-end/59630] [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-01-02
                 CC|                            |mpolacek at gcc dot gnu.org
   Target Milestone|---                         |4.7.4
     Ever confirmed|0                           |1

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Confirmed.

[Bug middle-end/59630] [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with my r182761 but say:
_Bool foo (int x)
{
  _Bool (*f)(int) = __builtin_abs;
  return f(x);
}
ICEs at -O2 already since the gimple_call_fntype introduction in r172310.
Since the r172310 change we also without any warnings happily inline:
inline int fn1 (int x, int y) { return x + y; }
int
fn2 (void)
{
  int (*fn) (long long) = (void *) fn1;
  return fn (5LL);
}
inline int fn3 (long long x) { return x; }
int
fn4 (void)
{
  int (*fn) (int x, int y) = (void *) fn3;
  return fn (5, 6);
}
(well, for fn1 into fn2 we complain about uninitialized y).
This is also related to PR59622.  Even with gimple_call_fntype, I'd say we
should never propagate an indirect call into a gimple_call_fndecl if it doesn't
satisfy some minimal consistency predicate (something along the lines of what I
wrote in PR59622, basically if the stmt has lhs, then the return types need to
be compatible, and TYPE_ARG_TYPES should be compatible (perhaps an exception
can be made for the C () vs. (void) arguments).  Because if the conversion
between fn return type and/or arguments types is non-useless, I'm afraid tons
of issues can happen everywhere.  At minimum we should disallow it if the
fndecl we want to propagate is a builtin, and we should not inline it, or
perhaps clone.

[Bug middle-end/59630] [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[mpolacek at gcc dot gnu.org]

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

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
One alternative could be the following (though completely untested):

--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -14227,6 +14227,10 @@ fold_call_stmt (gimple stmt, bool ignore)
          of a builtin, will use the wrong location information.  */
           if (gimple_has_location (stmt))
                 {
+          tree lhs = gimple_call_lhs (stmt);
+          if (lhs && !useless_type_conversion_p (TREE_TYPE (lhs),
+                             TREE_TYPE (ret)))
+            return NULL_TREE;
           tree realret = ret;
           if (TREE_CODE (ret) == NOP_EXPR)
             realret = TREE_OPERAND (ret, 0);

[Bug middle-end/59630] [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> Started with my r182761 but say:
> _Bool foo (int x)
> {
>   _Bool (*f)(int) = __builtin_abs;
>   return f(x);
> }
> ICEs at -O2 already since the gimple_call_fntype introduction in r172310.
> Since the r172310 change we also without any warnings happily inline:
> inline int fn1 (int x, int y) { return x + y; }
> int
> fn2 (void)
> {
>   int (*fn) (long long) = (void *) fn1;
>   return fn (5LL);
> }
> inline int fn3 (long long x) { return x; }
> int
> fn4 (void)
> {
>   int (*fn) (int x, int y) = (void *) fn3;
>   return fn (5, 6);
> }
> (well, for fn1 into fn2 we complain about uninitialized y).
> This is also related to PR59622.  Even with gimple_call_fntype, I'd say we
> should never propagate an indirect call into a gimple_call_fndecl if it
> doesn't satisfy some minimal consistency predicate (something along the
> lines of what I wrote in PR59622, basically if the stmt has lhs, then the
> return types need to be compatible, and TYPE_ARG_TYPES should be compatible
> (perhaps an exception can be made for the C () vs. (void) arguments). 
> Because if the conversion between fn return type and/or arguments types is
> non-useless, I'm afraid tons of issues can happen everywhere.  At minimum we
> should disallow it if the fndecl we want to propagate is a builtin, and we
> should not inline it, or perhaps clone.

Not at all.  The bug is how we identify a call stmt to be a builtin call
vs. a call to a builtin function.  gimple_call_arg_fn specifies the
function address we call and gimple_call_fntype specifies the function
signature the caller expects.

There shouldn't be many places to fix, and in most cases a proper
predicate can be used / introduced.  useless_type_conversion_p
(TREE_TYPE (fndecl), fntype) should work here to guard the builtins.c
folding.

We really want to aggressively make indirect calls direct ones.

[Bug middle-end/59630] [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine (next week).

[Bug middle-end/59630] [4.7/4.8/4.9 Regression] ICE converting the return type of a builtin function

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Wed Jan  8 09:06:27 2014
New Revision: 206421

URL: http://gcc.gnu.org/viewcvs?rev=206421&root=gcc&view=rev
Log:
2014-01-08  Richard Biener  <rguenther@suse.de>

    PR middle-end/59630
    * gimple.h (is_gimple_builtin_call): Remove.
    (gimple_builtin_call_types_compatible_p): New.
    (gimple_call_builtin_p): New overload.
    * gimple.c (is_gimple_builtin_call): Remove.
    (validate_call): Rename to ...
    (gimple_builtin_call_types_compatible_p): ... this and export.  Also
    check return types.
    (validate_type): New static function.
    (gimple_call_builtin_p): New overload and adjust.
    * gimple-fold.c (gimple_fold_builtin): Fold the return value.
    (gimple_fold_call): Likewise.  Use gimple_call_builtin_p.
    (gimple_fold_stmt_to_constant_1): Likewise.
    * tsan.c (instrument_gimple): Use gimple_call_builtin_p.

    * gcc.dg/pr59630.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/pr59630.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-fold.c
    trunk/gcc/gimple.c
    trunk/gcc/gimple.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tsan.c

[Bug middle-end/59630] [4.7/4.8 Regression] ICE converting the return type of a builtin function

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.9.0
            Summary|[4.7/4.8/4.9 Regression]    |[4.7/4.8 Regression] ICE
                   |ICE converting the return   |converting the return type
                   |type of a builtin function  |of a builtin function
      Known to fail|4.9.0                       |

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug middle-end/59630] [4.7/4.8 Regression] ICE converting the return type of a builtin function

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59630

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.4                       |4.8.4

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
The 4.7 branch is being closed, moving target milestone to 4.8.4.

[Bug middle-end/59630] [4.8 Regression] ICE converting the return type of a builtin function

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59630

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug middle-end/59630] [4.8 Regression] ICE converting the return type of a builtin function

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59630

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug middle-end/59630] [4.8 Regression] ICE converting the return type of a builtin function

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59630

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|4.8.5                       |4.9.0
      Known to fail|                            |4.8.5

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for 4.9.0.