Ping... for the latest version of my patch which can be found here:
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. >
