This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Fri, Oct 6, 2017 at 2:06 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> 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.
>
> 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.
>

Note also that besides -Wstrict-prototypes, there's also a warning
from -Wold-style-definition if using it for a definition instead of a
declaration. Also, if unprototyped functions are completely removed
from C as Joseph mentioned in another branch of this thread, the code
would be completely invalid:
https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00257.html

> 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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]