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

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

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.

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 start to believe that the type "void (*) (void)" is better suited for
this purpose, and it is already used in many programs as a type-less
wildcard function type.  I see examples in libgo and libffi at least.



Bernd.