This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFA] [PATCH] Add a warning for invalid function casts
On 10/06/2017 12:06 PM, Bernd Edlinger wrote:
On 10/06/17 17:43, Martin Sebor wrote:
On 10/06/2017 07:25 AM, Bernd Edlinger wrote:
On 10/05/17 18:16, Martin Sebor wrote:
In my (very quick) tests the warning appears to trigger on all
strictly incompatible conversions, even if they are otherwise
benign, such as:
int f (const int*);
typedef int F (int*);
F* pf1 = f; // -Wincompatible-pointer-types
F* pf2 = (F*)f; // -Wcast-function-type
Likewise by:
int f (signed char);
typedef int F (unsigned char);
F* pf = (F*)f; // -Wcast-function-type
I'd expect these conversions to be useful and would view warning
for them with -Wextra as excessive. In fact, I'm not sure I see
the benefit of warning on these casts under any circumstances.
Well, while the first example should be safe,
the second one is probably not safe:
Because the signed and unsigned char are promoted to int,
by the ABI but one is in the range -128..127 while the
other is in the range 0..255, right?
Right. The cast is always safe but whether or not a call to such
a function through the incompatible pointer is also safe depends
on the definition of the function (and on the caller). If the
function uses just the low bits of the argument then it's most
likely fine for any argument. If the caller only calls it with
values in the 7-bit range (e.g., the ASCII subset) then it's also
fine. Otherwise there's the potential for the problem you pointed
out. (Similarly, if in the first example I gave the cast added
constness to the argument rather than removing it and the function
modified the pointed-to object calling it through the incompatible
pointer on a constant object would also be unsafe.)
Another class of cases to consider are casts between functions
taking pointers to different but related structs. Code like this
could be written to mimic C++ calling a base class function on
a derived object.
struct Base { ... };
struct Derived { Base b; ... };
typedef void F (Derived*);
void foo (Base*);
F* pf = (F*)foo;
Hmm, yes.
I start to believe, that this warning should treat all pointers
as equivalent, but everything else need to be of the same type.
That would at least cover the majority of all "safe" use cases.
Perhaps basing the warning on some form of structural equivalence
between function arguments might be useful. For instance, in
ILP32, casting between 'int foo (int)' and 'long foo (long)' and
calling the function is probably generally considered safe (even
though it's undefined by the language) and works as people expect
so avoiding the warning there would help keep the false positive
rate down. (Something like this might also work for the kernel
alias macros.)
Similarly, casting between a function that returns a scalar smaller
than int and int and then calling it is probably safe (or maybe
even long, depending on the ABI).
Casting a function returning a value to one returning void and
calling it through the result should always be safe.
I would also suggest to consider disregarding qualifiers as those
are often among the reasons for intentional casts (e.g., when
mixing a legacy API and a more modern const-correct one).
Casts are also not uncommon between variadic and ordinary function
types so some heuristic might be appropriate there.
And I need a way to by-pass the warning with a generic function
pointer type. uintptr_t is not the right choice, as you pointed
out already.
But I also see problems with "int (*) ()" as a escape mechanism
because this declaration creates a warning in C with
-Wstrict-prototypes, and in C++ this syntax means "int (*) (void)"
while the C++ type "int (*) (...)" is rejected by the C front end.
I wouldn't consider it a problem if the suppression mechanism were
different between languages. Most such casts are going to be in
source files (as opposed to C headers) so it should be fine to use
each language's unique form of function without a prototype.
Martin