Re: [PATCH] Add a warning for invalid function casts

[Martin Sebor <msebor at gmail dot com>]

On 10/09/2017 11:50 AM, Bernd Edlinger wrote:
> On 10/09/17 18:44, Martin Sebor wrote:
> > On 10/07/2017 10:48 AM, Bernd Edlinger wrote:
> > > Hi!
> > >
> > > I think I have now something useful, it has a few more heuristics
> > > added, to reduce the number of false-positives so that it
> > > is able to find real bugs, for instance in openssl it triggers
> > > at a function cast which has already a TODO on it.
> > >
> > > The heuristics are:
> > > - handle void (*)(void) as a wild-card function type.
> > > - ignore volatile, const qualifiers on parameters/return.
> > > - handle any pointers as equivalent.
> > > - handle integral types, enums, and booleans of same precision
> > >    and signedness as equivalent.
> > > - stop parameter validation at the first "...".
> >
> > These sound quite reasonable to me.  I have a reservation about
> > just one of them, and some comments about other aspects of the
> > warning.  Sorry if this seems like a lot.  I'm hoping you'll
> > find the feedback constructive.
> >
> > I don't think using void(*)(void) to suppress the warning is
> > a robust solution because it's not safe to call a function that
> > takes arguments through such a pointer (especially not if one
> > or more of the arguments is a pointer).  Depending on the ABI,
> > calling a function that expects arguments with none could also
> > mess up the stack as the callee may pop arguments that were
> > never passed to it.
>
> This is of course only a heuristic, and if there is no warning
> that does not mean any guarantee that there can't be a problem
> at runtime.  The heuristic is only meant to separate the
> bad from the very bad type-cast.  In my personal opinion there
> is not a single good type cast.

I agree.  Since the warning uses one kind of a cast as an escape
mechanism from the checking it should be one whose result can
the most likely be used to call the function without undefined
behavior.

Since it's possible to call any function through a pointer to
a function with no arguments (simply by providing arguments of
matching types) it's a reasonable candidate.

On the other hand, since it is not safe to call an arbitrary
function through void (*)(void), it's not as good a candidate.

Another reason why I think a protoype-less function is a good
choice is because the alias and ifunc attributes already use it
as an escape mechanism from their type incompatibility warning.

Martin