Re: [PATCH] Fix up tree-ssa-strlen.c ICEs (PR tree-optimization/89703)

[Martin Sebor <msebor at gmail dot com>]

On 3/14/19 2:43 AM, Jakub Jelinek wrote:
> On Thu, Mar 14, 2019 at 12:50:28AM +0000, Joseph Myers wrote:
> > > The C FE sadly passes through some really bad prototypes of builtin
> > > functions as "harmless":
> > >            /* Accept "harmless" mismatches in function types such
> > >               as missing qualifiers or pointer vs same size integer
> > >               mismatches.  This is for the ffs and fprintf builtins.
> > >               However, with -Wextra in effect, diagnose return and
> > >               argument types that are incompatible according to
> > >               language rules.  */
> >
> > Note that this isn't just about pre-standard headers with unexpected types
> > (if it were, it might be obsolete).  It's also needed for building glibc
> > (to build glibc with -Wextra with GCC trunk, one of the -Wno- options
> > needed is -Wno-builtin-declaration-mismatch), because of how various code
> > creates function aliases between functions that have the same ABI but
> > different types (long versus int, etc.).
>
> I was mainly woried about the pointer vs same sized integer mismatches.
> Seems we refuse them on arguments:
>        /* Fail for types with incompatible modes/sizes.  */
>        if (TYPE_MODE (TREE_VALUE (oldargs))
>            != TYPE_MODE (TREE_VALUE (newargs)))
>          return NULL_TREE;
>        /* Fail for function and object pointer mismatches.  */
>        if ((FUNCTION_POINTER_TYPE_P (oldtype)
>             != FUNCTION_POINTER_TYPE_P (newtype))
>            || POINTER_TYPE_P (oldtype) != POINTER_TYPE_P (newtype))
>          return NULL_TREE;
> but not on the return type, where there is just:
>    if (TYPE_MODE (oldrettype) != TYPE_MODE (newrettype))
>      return NULL_TREE;
> Whether a builtin returns a pointer or integer is at least for the
> middle-end quite important difference, so I'm just surprised more bugs in
> other passes haven't been filed yet.  I can deal with that in the
> tree-ssa-strlen.c pass with the patch I've posted, just am worried about
> potential issues elsewhere.

What solution would you like to see for these (pointer vs integer)
mismatches?

I think they should not only be diagnosed even without -Wextra but
at a minimum, treated as ordinary library functions, same as if
their arguments fail to match in the same way(*).

(Since some of the impetus for the fix was these sorts of ICEs,
I suspect I either missed this case when I fixed PR86125 or
I intentionally left it conservative because of the comment.)

[*] Making a library call will just crash at runtime, except
possibly much later, so I think it would be best to issue
a warning by default, like Clang does. It might also be worth
considering replacing the calls with a trap (with some new
option to disable it).

Whatever solution we can agree on I will tighten it in GCC 10.

Martin