Ping... for the C++ part of this patch:
https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00559.html Thanks Bernd. > On 10/10/17 00:30, Bernd Edlinger wrote: >> On 10/09/17 20:34, Martin Sebor wrote: >>> 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. >>> >> >> I know of pre-existing code-bases where a type-cast to type: >> void (*) (void); >> >> .. is already used as a generic function pointer: libffi and >> libgo, I would not want to break these. >> >> Actually when I have a type: >> X (*) (...); >> >> I would like to make sure that the warning checks that >> only functions returning X are assigned. >> >> and for X (*) (Y, ....); >> >> I would like to check that anything returning X with >> first argument of type Y is assigned. >> >> There are code bases where such a scheme is used. >> For instance one that I myself maintain: the OPC/UA AnsiC Stack, >> where I have this type definition: >> >> typedef OpcUa_StatusCode (OpcUa_PfnInvokeService)(OpcUa_Endpoint >> hEndpoint, ...); >> >> And this plays well together with this warning, because only >> functions are assigned that match up to the ...); >> Afterwards this pointer is cast back to the original signature, >> so everything is perfectly fine. >> >> Regarding the cast from pointer to member to function, I see also a >> warning without -Wpedantic: >> Warnung: converting from »void (S::*)(int*)« to »void (*)(int*)« >> [-Wpmf-conversions] >> F *pf = (F*)&S::foo; >> ^~~ >> >> And this one is even default-enabled, so I think that should be >> more than sufficient. >> >> I also changed the heuristic, so that your example with the enum should >> now work. I did not add it to the test case, because it would >> break with -fshort-enums :( >> >> Attached I have an updated patch that extends this warning to the >> pointer-to-member function cast, and relaxes the heuristic on the >> benign integral type differences a bit further. >> >> >> Is it OK for trunk after bootstrap and reg-testing? >> >> >> Thanks >> Bernd. >>
