Message ID | AM5PR0701MB265700BCFDF7620223717DCCE4720@AM5PR0701MB2657.eurprd07.prod.outlook.com |
---|---|
State | New |
Headers | show |
Series | [RFA] Add a warning for invalid function casts | expand |
On Tue, 3 Oct 2017, Bernd Edlinger wrote: > invalid, also if both function types have a non-null TYPE_ARG_TYPES > I would say this deserves a warning. As an exception I have I'm not convinced by the TYPE_ARG_TYPES check, at least for C. In C, unprototyped function types are not compatible with variadic function types or functions with argument types changed by default argument promotions (that is, int () and int (char) and int (int, ...) are all incompatible). I'd think it appropriate to warn about such conversions, given that they are cases where calling the converted function has undefined behavior. There may well be cases of interfaces where void (*) (void) is used as a generic function pointer type (always converted to / from the actual type of the function in question), for which this warning would not be suitable.
On Tue, Oct 3, 2017 at 3:33 PM, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > Hi! > > I have implemented a warning -Wcast-function-type that analyzes > type casts which change the function signatures. > > I would consider function pointers with different result type > invalid, also if both function types have a non-null TYPE_ARG_TYPES > I would say this deserves a warning. As an exception I have > used for instance in recog.h, the warning allows casting > a function with the type typedef rtx (*stored_funcptr) (...); > to any function with the same result type. > > I would think a warning like that should be enabled with -Wextra. > > Attached is a first version of the patch and as you can see > the warning found already lots of suspicious type casts. The worst > is the splay-tree which always calls functions with uintptr_t > instead of the correct parameter type. I was unable to find > a solution for this, and just silenced the warning with a > second type-cast. > > Note that I also changed one line in libgo, but that is only > a quick hack which I only did to make the boot-strap with > all languages succeed. > > I'm not sure if this warning may be a bit too strict, but I think > so far it just triggered on rather questionable code. > > Thoughts? > > > Bernd. Sorry if this is a stupid question, but could you explain how this warning is different from -Wbad-function-cast? Something about direct calls to functions vs. passing them as function pointers?
On 10/05/17 02:24, Eric Gallager wrote: > Sorry if this is a stupid question, but could you explain how this > warning is different from -Wbad-function-cast? Something about direct > calls to functions vs. passing them as function pointers? No, it is not :) -Wbad-function-cast is IMHO a strange legacy warning. It is C-only, and it triggers only if the result of a function call is cast to a type with a different TREE_CODE, so for instance int <-> float <-> pointer. It would trigger for perfectly valid code like this: i = (int) floor (f); while we have no warning for i = floor (f); What I want to diagnose is assigning a function pointer via an explicit type cast to another function pointer, when there is no possible implicit conversion between the two function pointer types. Thus the cast was used to silence a warning/error in the first place. The idea for this warning came up when someone spotted a place in openssl, where a type cast was used to change the return value of a callback function from long to int: https://github.com/openssl/openssl/issues/4413 But due to the type cast there was never ever any warning from this invalid type cast. Bernd.
On 10/03/17 23:34, Joseph Myers wrote: > On Tue, 3 Oct 2017, Bernd Edlinger wrote: > >> invalid, also if both function types have a non-null TYPE_ARG_TYPES >> I would say this deserves a warning. As an exception I have > > I'm not convinced by the TYPE_ARG_TYPES check, at least for C. > I will drop that, for C and C++, and try to get it working without that kludge. > In C, unprototyped function types are not compatible with variadic > function types or functions with argument types changed by default > argument promotions (that is, int () and int (char) and int (int, ...) are > all incompatible). I'd think it appropriate to warn about such > conversions, given that they are cases where calling the converted > function has undefined behavior. > Right, interesting is that this does not produce a warning, int test(int); int foo() { int (*x)(); x = test; return x(1); } while the following example produces a warning: int test(int,...); int foo() { int (*x)(int); x = test; return x(1); } gcc -Wall -W -S t1.c t1.c: In function 'foo': t1.c:6:5: warning: assignment to 'int (*)(int)' from incompatible pointer type 'int (*)(int)' [-Wincompatible-pointer-types] x = test; ^ I will send a patch that adds ", ..." to the parameter list in the diagnostics... But why is int(*)(int) compatible to (int)(*)() but not to int(*)(int,...) ? > There may well be cases of interfaces where void (*) (void) is used as a > generic function pointer type (always converted to / from the actual type > of the function in question), for which this warning would not be > suitable. > Yes, those would get a warning, or have to add a cast to uintptr_t, if it is not possible to fix the code otherwise. libffi does this... Bernd.
On Okt 05 2017, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > The idea for this warning came up when someone spotted a place in > openssl, where a type cast was used to change the return value of a > callback function from long to int: > > https://github.com/openssl/openssl/issues/4413 > > But due to the type cast there was never ever any warning from this > invalid type cast. Note that the type cast itself is perfectly valid (all function pointers are alike), but it's the call site that is problematic: unless it casts back to the real type of the function before the call it is causing undefined behviour. Andreas.
On Thu, 5 Oct 2017, Bernd Edlinger wrote: > But why is int(*)(int) compatible to (int)(*)() but not > to int(*)(int,...) ? I think it's a matter of what function types were possible in K&R C. <stdarg.h> variadic types weren't (there was an older <varargs.h>), and neither were types with arguments of type float or narrower-than-int integers (because those always got promoted to a wider type when passed as function arguments to an unprototyped function). And those types that were impossible in K&R C always require function prototypes. (The possibility of function types without a prototype is a legacy feature. There was some suggestion of removing it for C11, but no-one ever produced a paper for WG14 proposing the actual wording changes that would have been needed.)
On 10/03/2017 01:33 PM, Bernd Edlinger wrote: > Hi! > > I have implemented a warning -Wcast-function-type that analyzes > type casts which change the function signatures. > > I would consider function pointers with different result type > invalid, also if both function types have a non-null TYPE_ARG_TYPES > I would say this deserves a warning. As an exception I have > used for instance in recog.h, the warning allows casting > a function with the type typedef rtx (*stored_funcptr) (...); > to any function with the same result type. > > I would think a warning like that should be enabled with -Wextra. > > Attached is a first version of the patch and as you can see > the warning found already lots of suspicious type casts. The worst > is the splay-tree which always calls functions with uintptr_t > instead of the correct parameter type. I was unable to find > a solution for this, and just silenced the warning with a > second type-cast. > > Note that I also changed one line in libgo, but that is only > a quick hack which I only did to make the boot-strap with > all languages succeed. > > I'm not sure if this warning may be a bit too strict, but I think > so far it just triggered on rather questionable code. > > Thoughts? My initial thought is that although casts between incompatible function types undoubtedly mask bugs, the purpose of such casts is to make such conversions possible. Indiscriminately diagnosing them would essentially eliminate this feature of the type system. Having to add another cast to a different type(*) to suppress the warning when such a conversion is intended doesn't seem like a good solution (the next logical step to find bugs in those conversions would then be to add another warning to see through those additional casts, and so on). With that, my question is: under what circumstances does the warning not trigger on a cast to a function of an incompatible type? 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. Similarly, for casts between pointers to the same integer type with a different sign, or those involving ILP32/LP64 portability issues I'd expect the warning not to trigger unless requested (e.g., by some other option targeting those issues). So based on these initial observations and despite the bugs it may have uncovered, I share your concern that the warning in its present form is too strict to be suitable for -Wextra, or possibly even too noisy to be of practical use on its own and outside of -Wextra. Out of curiosity, have you done any tests on other code bases besides GCC to see how many issues (true and false positives) it finds? Martin [*] Strictly speaking, the semantics of casting a function pointer to intptr_t aren't necessarily well-defined. Only void* can be portably converted to intptr_t and back, and only object pointers are required to be convertible to void* and back. And although GCC defines the semantics of these conversions, forcing programs to abandon a well defined language feature in favor of one that's not as cleanly specified would undermine the goal the warning is meant to achieve.
On 10/5/17, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > On 10/05/17 02:24, Eric Gallager wrote: >> Sorry if this is a stupid question, but could you explain how this >> warning is different from -Wbad-function-cast? Something about direct >> calls to functions vs. passing them as function pointers? > > No, it is not :) > > -Wbad-function-cast is IMHO a strange legacy warning. > > It is C-only, and it triggers only if the result of a function call > is cast to a type with a different TREE_CODE, so for instance > int <-> float <-> pointer. > > It would trigger for perfectly valid code like this: > > i = (int) floor (f); > > while we have no warning for > > i = floor (f); > > What I want to diagnose is assigning a function pointer via an explicit > type cast to another function pointer, when there is no possible > implicit conversion between the two function pointer types. > Thus the cast was used to silence a warning/error in the first place. OK, thanks for explaining! I kinda worry about warning on code that was added to silence other warnings in the first place, as it can lead to frustration when making a fix expecting the number of warnings when compiling to decrease, but then they don't actually decrease. But as long as there's a simple way to fix the warned-on code that silences both the original warning being avoided, and this new one, I see how it'll be useful. > > The idea for this warning came up when someone spotted a place in > openssl, where a type cast was used to change the return value of a > callback function from long to int: > > https://github.com/openssl/openssl/issues/4413 > > But due to the type cast there was never ever any warning from this > invalid type cast. > > > Bernd. > Thanks for working on this! Eric
On 10/05/17 18:16, Martin Sebor wrote: > On 10/03/2017 01:33 PM, Bernd Edlinger wrote: >> >> I'm not sure if this warning may be a bit too strict, but I think >> so far it just triggered on rather questionable code. >> >> Thoughts? > > My initial thought is that although casts between incompatible > function types undoubtedly mask bugs, the purpose of such casts > is to make such conversions possible. Indiscriminately diagnosing > them would essentially eliminate this feature of the type system. > Having to add another cast to a different type(*) to suppress > the warning when such a conversion is intended doesn't seem like > a good solution (the next logical step to find bugs in those > conversions would then be to add another warning to see through > those additional casts, and so on). > > With that, my question is: under what circumstances does the > warning not trigger on a cast to a function of an incompatible > type? > > 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. > > Similarly, for casts between pointers to the same integer type > with a different sign, or those involving ILP32/LP64 portability > issues I'd expect the warning not to trigger unless requested > (e.g., by some other option targeting those issues). > > So based on these initial observations and despite the bugs it > may have uncovered, I share your concern that the warning in > its present form is too strict to be suitable for -Wextra, or > possibly even too noisy to be of practical use on its own and > outside of -Wextra. > > Out of curiosity, have you done any tests on other code bases > besides GCC to see how many issues (true and false positives) > it finds? > > Martin > > [*] Strictly speaking, the semantics of casting a function > pointer to intptr_t aren't necessarily well-defined. Only void* > can be portably converted to intptr_t and back, and only object > pointers are required to be convertible to void* and back. And > although GCC defines the semantics of these conversions, forcing > programs to abandon a well defined language feature in favor of > one that's not as cleanly specified would undermine the goal > the warning is meant to achieve. Thanks for your advice. I did look into openssl and linux, both have plenty of -Wcast-function-type warnings. In the case of openssl it is lots of similar stuff crypto/aes/aes_cfb.c:25:27: warning: cast between incompatible function types from 'void (*)(const unsigned char *, unsigned char *, const AES_KEY *) {aka void (*)(const unsigned char *, unsigned char *, const struct aes_key_st *)}' to 'void (*)(const unsigned char *, unsigned char *, const void *)' [-Wcast-function-type] (block128_f) AES_encrypt); The problem is the function is of course called with a different signature than what is declared. They take it somehow for granted, that "void*" or "const void*" parameter are an alias for any pointer or any const pointer. Either as parameter or as return code. I would believe this is not well-defined by the c-standard. But it makes the warning less useful because it would be impossible to spot the few places where the call will actually abort at runtime. Then I tried to compile linux, I noticed that there is a new warning for the alias to incompatible function. I saw it very often, and it is always when a system call is defined: ./include/linux/compat.h:50:18: Warnung: »compat_sys_ptrace« alias between functions of incompatible types »long int(compat_long_t, compat_long_t, compat_long_t, compat_long_t) {alias long int(int, int, int, int)}« and »long int(long int, long int, long int, long int)« [-Wattributes] asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\ ./include/linux/syscalls.h:211:18: Warnung: »sys_rt_sigprocmask« alias between functions of incompatible types »long int(int, sigset_t *, sigset_t *, size_t) {alias long int(int, struct <anonym> *, struct <anonym> *, long unsigned int)}« and »long int(long int, long int, long int, long int)« [-Wattributes] asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ Needless to say that even more Wcast-function-type warning happen. ./include/linux/timer.h:178:23: Warnung: cast between incompatible function types from »void (*)(struct timer_list *)« to »void (*)(long unsigned int)« [-Wcast-function-type] __setup_timer(timer, (TIMER_FUNC_TYPE)callback, So they assume obviously that any int / long / pointer value are compatible with uintptr_t and intptr_t. If the Wattribute stays this way, I can garantee that Linus will simply add -Wno-attribute to the linux makefiles, which is bad because -Wattribute is more than one warning alone. At the least we should rename this particular warning into something else, so that it is not necessary to disable everything when only this specific warning is too hard to fix. Maybe it would be good to not warn in type-casts, when they can be assumed to be safe, for instance void* <-> any pointer (parameter or result), uintptr_t <-> any int, any pointer (parameter or result), void (*) (void) and void (*) (...) <-> any function pointer. I think that applies to both warnings. What do you think? Bernd.
On Thu, 5 Oct 2017, Bernd Edlinger wrote: > Maybe it would be good to not warn in type-casts, when they can be > assumed to be safe, for instance > void* <-> any pointer (parameter or result), > uintptr_t <-> any int, any pointer (parameter or result), > void (*) (void) and void (*) (...) <-> any function pointer. Well, void * and uintptr_t aren't necessarily interchangable at the ABI level. At least, the m68k ABI returns integers in %d0 and pointers in %a0; I don't know if any other ABIs have that peculiarity.
On 10/05/2017 03:04 PM, Bernd Edlinger wrote: > On 10/05/17 18:16, Martin Sebor wrote: >> On 10/03/2017 01:33 PM, Bernd Edlinger wrote: >>> >>> I'm not sure if this warning may be a bit too strict, but I think >>> so far it just triggered on rather questionable code. >>> >>> Thoughts? >> >> My initial thought is that although casts between incompatible >> function types undoubtedly mask bugs, the purpose of such casts >> is to make such conversions possible. Indiscriminately diagnosing >> them would essentially eliminate this feature of the type system. >> Having to add another cast to a different type(*) to suppress >> the warning when such a conversion is intended doesn't seem like >> a good solution (the next logical step to find bugs in those >> conversions would then be to add another warning to see through >> those additional casts, and so on). >> >> With that, my question is: under what circumstances does the >> warning not trigger on a cast to a function of an incompatible >> type? >> >> 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. >> >> Similarly, for casts between pointers to the same integer type >> with a different sign, or those involving ILP32/LP64 portability >> issues I'd expect the warning not to trigger unless requested >> (e.g., by some other option targeting those issues). >> >> So based on these initial observations and despite the bugs it >> may have uncovered, I share your concern that the warning in >> its present form is too strict to be suitable for -Wextra, or >> possibly even too noisy to be of practical use on its own and >> outside of -Wextra. >> >> Out of curiosity, have you done any tests on other code bases >> besides GCC to see how many issues (true and false positives) >> it finds? >> >> Martin >> >> [*] Strictly speaking, the semantics of casting a function >> pointer to intptr_t aren't necessarily well-defined. Only void* >> can be portably converted to intptr_t and back, and only object >> pointers are required to be convertible to void* and back. And >> although GCC defines the semantics of these conversions, forcing >> programs to abandon a well defined language feature in favor of >> one that's not as cleanly specified would undermine the goal >> the warning is meant to achieve. > > Thanks for your advice. > > I did look into openssl and linux, both have plenty of > -Wcast-function-type warnings. > > In the case of openssl it is lots of similar stuff > crypto/aes/aes_cfb.c:25:27: warning: cast between incompatible function > types from 'void (*)(const unsigned char *, unsigned char *, const > AES_KEY *) {aka void (*)(const unsigned char *, unsigned char *, const > struct aes_key_st *)}' to 'void (*)(const unsigned char *, unsigned > char *, const void *)' [-Wcast-function-type] > (block128_f) AES_encrypt); > > The problem is the function is of course called with a different > signature than what is declared. They take it somehow for granted, > that "void*" or "const void*" parameter are an alias for > any pointer or any const pointer. Either as parameter or as return > code. > > I would believe this is not well-defined by the c-standard. > > But it makes the warning less useful because it would be impossible > to spot the few places where the call will actually abort at > runtime. > > Then I tried to compile linux, I noticed that there is a new > warning for the alias to incompatible function. > > I saw it very often, and it is always when a system call > is defined: > > ./include/linux/compat.h:50:18: Warnung: »compat_sys_ptrace« alias > between functions of incompatible types »long int(compat_long_t, > compat_long_t, compat_long_t, compat_long_t) {alias long int(int, > int, int, int)}« and »long int(long int, long int, long int, long > int)« [-Wattributes] > asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\ > > ./include/linux/syscalls.h:211:18: Warnung: »sys_rt_sigprocmask« alias > between functions of incompatible types »long int(int, sigset_t *, > sigset_t *, size_t) {alias long int(int, struct <anonym> *, struct > <anonym> *, long unsigned int)}« and »long int(long int, long int, > long int, long int)« [-Wattributes] > asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ > > > Needless to say that even more Wcast-function-type warning > happen. > > ./include/linux/timer.h:178:23: Warnung: cast between incompatible > function types from »void (*)(struct timer_list *)« to »void (*)(long > unsigned int)« [-Wcast-function-type] > __setup_timer(timer, (TIMER_FUNC_TYPE)callback, > > So they assume obviously that any int / long / pointer value > are compatible with uintptr_t and intptr_t. > > If the Wattribute stays this way, I can garantee that Linus will simply > add -Wno-attribute to the linux makefiles, which is bad because > -Wattribute is more than one warning alone. > > At the least we should rename this particular warning into > something else, so that it is not necessary to disable everything > when only this specific warning is too hard to fix. > > Maybe it would be good to not warn in type-casts, when they can be > assumed to be safe, for instance > void* <-> any pointer (parameter or result), > uintptr_t <-> any int, any pointer (parameter or result), > void (*) (void) and void (*) (...) <-> any function pointer. > > I think that applies to both warnings. > > What do you think? The attribute alias/ifunc incompatibilities are conflicts between declarations of the same symbol. Such conflicts are invalid in C and ordinarily rejected with a hard error. With the exception of ifunc resolvers returning void*, I see the alias/ifunc conflicts as signs of bugs or, at best, benign mistakes. I also can think of no reason why they shouldn't in most cases be corrected to conform to the C requirements. Where they can't be fixed it's usually easy to suppress the warnings by declaring the functions without a prototype. That also makes it clear to the reader that type checking has been suppressed. On the other hand, explicit casts between incompatible pointers are a well-defined feature of the language and their use a clear indication that the programmer was aware of the incompatibility and intentionally suppressed it. There no doubt are bugs among these that would be helpful to find so I think the challenge here is to distinguish the intentional and benign conversions from the others while minimizing false positives. Since there are likely to be a large proportion of the intentional and benign cases, unless the heuristic to find the others is very good, there should also be an easy way to suppress the false positives without compromising readability. Martin PS We are discussing the kernel warnings in bug 82435. I've already moved them under -Wincompatible-pointer-types but it sounds like a new option altogether might be preferable. Ideally, though, if it's possible it would be best to fix them like we did for Glibc so the kernel too would benefit from the better type checking. I should look into that.
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? Bernd.
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; Martin
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.
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
On 10/05/2017 03:47 PM, Joseph Myers wrote: > On Thu, 5 Oct 2017, Bernd Edlinger wrote: > >> Maybe it would be good to not warn in type-casts, when they can be >> assumed to be safe, for instance >> void* <-> any pointer (parameter or result), >> uintptr_t <-> any int, any pointer (parameter or result), >> void (*) (void) and void (*) (...) <-> any function pointer. > > Well, void * and uintptr_t aren't necessarily interchangable at the ABI > level. At least, the m68k ABI returns integers in %d0 and pointers in > %a0; I don't know if any other ABIs have that peculiarity. > The mn103 (live) and mn102 (dead) probably do. But my memory is getting fuzzy on those. jeff
On 10/06/2017 09:43 AM, 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; Yea. And one might even find such code in BFD. It certainly mimicks C++ base and derived classes using C, so it has significant potential to have this kind of code. jeff
On 10/06/17 22:50, Jeff Law wrote: > On 10/06/2017 09:43 AM, 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; > Yea. And one might even find such code in BFD. It certainly mimicks > C++ base and derived classes using C, so it has significant potential to > have this kind of code. > jeff > Yes, absolutely. This use case makes up 99% of all places where I saw the warning until now. I will try to ignore all pointer types and see how that works out on some code bases I have access to. When that works as expected we should be able to see what heuristics need to be added next. FYI I have attached what I am currently bootstrapping. Bernd. gcc: 2017-10-06 Bernd Edlinger <bernd.edlinger@hotmail.de> * doc/invoke.texi: Document -Wcast-function-type. * recog.h (stored_funcptr): Change signature. * tree-dump.c (dump_node): Avoid warning. * typed-splay-tree.h (typed_splay_tree): Avoid warning. libcpp: 2017-10-06 Bernd Edlinger <bernd.edlinger@hotmail.de> * internal.h (maybe_print_line): Change signature. c-family: 2017-10-06 Bernd Edlinger <bernd.edlinger@hotmail.de> * c.opt (Wcast-function-type): New warning option. * c-lex.c (get_fileinfo): Avoid warning. * c-ppoutput.c (scan_translation_unit_directives_only): Remove cast. c: 2017-10-06 Bernd Edlinger <bernd.edlinger@hotmail.de> * c-typeck.c (c_safe_function_type_cast_p): New helper function. (build_c_cast): Implement -Wcast_function_type. cp: 2017-10-06 Bernd Edlinger <bernd.edlinger@hotmail.de> * decl2.c (start_static_storage_duration_function): Aboid warning. * typeck.c (+cxx_safe_function_type_cast_p): New helper function. (build_reinterpret_cast_1): Implement -Wcast_function_type. testsuite: 2017-10-06 Bernd Edlinger <bernd.edlinger@hotmail.de> * c-c++-common/Wcast-function-type.c: New test. Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 253493) +++ gcc/c/c-typeck.c (working copy) @@ -5474,6 +5474,36 @@ handle_warn_cast_qual (location_t loc, tree type, while (TREE_CODE (in_type) == POINTER_TYPE); } +/* Check if a type cast between two function types can be considered safe. */ + +static bool +c_safe_function_type_cast_p (tree t1, tree t2) +{ + if (TREE_TYPE (t1) == void_type_node && + TYPE_ARG_TYPES (t1) == void_list_node) + return true; + + if (TREE_TYPE (t2) == void_type_node && + TYPE_ARG_TYPES (t2) == void_list_node) + return true; + + if (!comptypes (TREE_TYPE (t1), TREE_TYPE (t2))) + return false; + + for (t1 = TYPE_ARG_TYPES (t1), t2 = TYPE_ARG_TYPES (t2); + t1 && t2; + t1 = TREE_CHAIN (t1), t2 = TREE_CHAIN (t2)) + { + if (TREE_CODE (TREE_VALUE (t1)) == POINTER_TYPE + && TREE_CODE (TREE_VALUE (t2)) == POINTER_TYPE) + continue; + if (!comptypes (TREE_VALUE (t1), TREE_VALUE (t2))) + return false; + } + + return true; +} + /* Build an expression representing a cast to type TYPE of expression EXPR. LOC is the location of the cast-- typically the open paren of the cast. */ @@ -5667,6 +5697,16 @@ build_c_cast (location_t loc, tree type, tree expr pedwarn (loc, OPT_Wpedantic, "ISO C forbids " "conversion of object pointer to function pointer type"); + if (TREE_CODE (type) == POINTER_TYPE + && TREE_CODE (otype) == POINTER_TYPE + && TREE_CODE (TREE_TYPE (type)) == FUNCTION_TYPE + && TREE_CODE (TREE_TYPE (otype)) == FUNCTION_TYPE + && !c_safe_function_type_cast_p (TREE_TYPE (type), + TREE_TYPE (otype))) + warning_at (loc, OPT_Wcast_function_type, + "cast between incompatible function types" + " from %qT to %qT", otype, type); + ovalue = value; value = convert (type, value); Index: gcc/c-family/c-lex.c =================================================================== --- gcc/c-family/c-lex.c (revision 253493) +++ gcc/c-family/c-lex.c (working copy) @@ -101,9 +101,11 @@ get_fileinfo (const char *name) struct c_fileinfo *fi; if (!file_info_tree) - file_info_tree = splay_tree_new ((splay_tree_compare_fn) strcmp, + file_info_tree = splay_tree_new ((splay_tree_compare_fn) + (void (*) (void)) strcmp, 0, - (splay_tree_delete_value_fn) free); + (splay_tree_delete_value_fn) + (void (*) (void)) free); n = splay_tree_lookup (file_info_tree, (splay_tree_key) name); if (n) Index: gcc/c-family/c-ppoutput.c =================================================================== --- gcc/c-family/c-ppoutput.c (revision 253493) +++ gcc/c-family/c-ppoutput.c (working copy) @@ -299,7 +299,7 @@ scan_translation_unit_directives_only (cpp_reader struct _cpp_dir_only_callbacks cb; cb.print_lines = print_lines_directives_only; - cb.maybe_print_line = (void (*) (source_location)) maybe_print_line; + cb.maybe_print_line = maybe_print_line; _cpp_preprocess_dir_only (pfile, &cb); } Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 253493) +++ gcc/c-family/c.opt (working copy) @@ -384,6 +384,10 @@ Wc++17-compat C++ ObjC++ Var(warn_cxx17_compat) Warning LangEnabledBy(C++ ObjC++,Wall) Warn about C++ constructs whose meaning differs between ISO C++ 2014 and ISO C++ 2017. +Wcast-function-type +C ObjC C++ ObjC++ Var(warn_cast_function_type) Warning EnabledBy(Wextra) +Warn about casts between incompatible function types. + Wcast-qual C ObjC C++ ObjC++ Var(warn_cast_qual) Warning Warn about casts which discard qualifiers. Index: gcc/cp/decl2.c =================================================================== --- gcc/cp/decl2.c (revision 253493) +++ gcc/cp/decl2.c (working copy) @@ -3484,7 +3484,8 @@ start_static_storage_duration_function (unsigned c priority_info_map = splay_tree_new (splay_tree_compare_ints, /*delete_key_fn=*/0, /*delete_value_fn=*/ - (splay_tree_delete_value_fn) &free); + (splay_tree_delete_value_fn) + (void (*) (void)) free); /* We always need to generate functions for the DEFAULT_INIT_PRIORITY so enter it now. That way when we walk Index: gcc/cp/typeck.c =================================================================== --- gcc/cp/typeck.c (revision 253493) +++ gcc/cp/typeck.c (working copy) @@ -1173,6 +1173,40 @@ comp_template_parms_position (tree t1, tree t2) return true; } +/* Check if a type cast between two function types can be considered safe. */ + +static bool +cxx_safe_function_type_cast_p (tree t1, tree t2) +{ + if (TREE_CODE (t1) != FUNCTION_TYPE + || TREE_CODE (t2) != FUNCTION_TYPE) + return same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)); + + if (TREE_TYPE (t1) == void_type_node && + TYPE_ARG_TYPES (t1) == void_list_node) + return true; + + if (TREE_TYPE (t2) == void_type_node && + TYPE_ARG_TYPES (t2) == void_list_node) + return true; + + if (!same_type_p (TREE_TYPE (t1), TREE_TYPE (t2))) + return false; + + for (t1 = TYPE_ARG_TYPES (t1), t2 = TYPE_ARG_TYPES (t2); + t1 && t2; + t1 = TREE_CHAIN (t1), t2 = TREE_CHAIN (t2)) + { + if (TREE_CODE (TREE_VALUE (t1)) == POINTER_TYPE + && TREE_CODE (TREE_VALUE (t2)) == POINTER_TYPE) + continue; + if (!same_type_p (TREE_VALUE (t1), TREE_VALUE (t2))) + return false; + } + + return true; +} + /* Subroutine in comptypes. */ static bool @@ -7263,7 +7297,15 @@ build_reinterpret_cast_1 (tree type, tree expr, bo return rvalue (expr); else if ((TYPE_PTRFN_P (type) && TYPE_PTRFN_P (intype)) || (TYPE_PTRMEMFUNC_P (type) && TYPE_PTRMEMFUNC_P (intype))) - return build_nop (type, expr); + { + if ((complain & tf_warning) + && !cxx_safe_function_type_cast_p (TREE_TYPE (type), + TREE_TYPE (intype))) + warning (OPT_Wcast_function_type, + "cast between incompatible function types" + " from %qH to %qI", intype, type); + return build_nop (type, expr); + } else if ((TYPE_PTRDATAMEM_P (type) && TYPE_PTRDATAMEM_P (intype)) || (TYPE_PTROBV_P (type) && TYPE_PTROBV_P (intype))) { Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 253493) +++ gcc/doc/invoke.texi (working copy) @@ -267,7 +267,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-builtin-declaration-mismatch @gol -Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol -Wc++-compat -Wc++11-compat -Wc++14-compat @gol --Wcast-align -Wcast-align=strict -Wcast-qual @gol +-Wcast-align -Wcast-align=strict -Wcast-function-type -Wcast-qual @gol -Wchar-subscripts -Wchkp -Wcatch-value -Wcatch-value=@var{n} @gol -Wclobbered -Wcomment -Wconditionally-supported @gol -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol @@ -3899,6 +3899,7 @@ This enables some extra warning flags that are not name is still supported, but the newer name is more descriptive.) @gccoptlist{-Wclobbered @gol +-Wcast-function-type @gol -Wempty-body @gol -Wignored-qualifiers @gol -Wimplicit-fallthrough=3 @gol @@ -5959,6 +5960,15 @@ Warn whenever a pointer is cast such that the requ target is increased. For example, warn if a @code{char *} is cast to an @code{int *} regardless of the target machine. +@item -Wcast-function-type +@opindex Wcast-function-type +@opindex Wno-cast-function-type +Warn when a function pointer is cast to an incompatible function pointer. +When one of the function types uses variable arguments like this +@code{int f(...);}, then only the return value is checked, otherwise +the return value and the parameter types are checked. +This warning is enabled by @option{-Wextra}. + @item -Wwrite-strings @opindex Wwrite-strings @opindex Wno-write-strings Index: gcc/recog.h =================================================================== --- gcc/recog.h (revision 253493) +++ gcc/recog.h (working copy) @@ -294,7 +294,7 @@ struct insn_gen_fn typedef rtx_insn * (*f15) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); typedef rtx_insn * (*f16) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef f0 stored_funcptr; + typedef void (*stored_funcptr) (void); rtx_insn * operator () (void) const { return ((f0)func) (); } rtx_insn * operator () (rtx a0) const { return ((f1)func) (a0); } Index: gcc/testsuite/c-c++-common/Wcast-function-type.c =================================================================== --- gcc/testsuite/c-c++-common/Wcast-function-type.c (revision 0) +++ gcc/testsuite/c-c++-common/Wcast-function-type.c (working copy) @@ -0,0 +1,31 @@ +/* { dg-do compile } */ +/* { dg-options "-Wcast-function-type" } */ + +int f(long); + +typedef int (f1)(long); +typedef int (f2)(void*); +#ifdef __cplusplus +typedef int (f3)(...); +typedef void (f4)(...); +#else +typedef int (f3)(); +typedef void (f4)(); +#endif +typedef void (f5)(void); + +f1 *a; +f2 *b; +f3 *c; +f4 *d; +f5 *e; + +void +foo (void) +{ + a = (f1 *) f; /* { dg-bogus "incompatible function types" } */ + b = (f2 *) f; /* { dg-warning "incompatible function types" } */ + c = (f3 *) f; /* { dg-bogus "incompatible function types" } */ + d = (f4 *) f; /* { dg-warning "incompatible function types" } */ + e = (f5 *) f; /* { dg-bogus "incompatible function types" } */ +} Index: gcc/tree-dump.c =================================================================== --- gcc/tree-dump.c (revision 253493) +++ gcc/tree-dump.c (working copy) @@ -735,7 +735,8 @@ dump_node (const_tree t, dump_flags_t flags, FILE di.flags = flags; di.node = t; di.nodes = splay_tree_new (splay_tree_compare_pointers, 0, - (splay_tree_delete_value_fn) &free); + (splay_tree_delete_value_fn) + (void (*) (void)) free); /* Queue up the first node. */ queue (&di, t, DUMP_NONE); Index: gcc/typed-splay-tree.h =================================================================== --- gcc/typed-splay-tree.h (revision 253493) +++ gcc/typed-splay-tree.h (working copy) @@ -75,9 +75,12 @@ inline typed_splay_tree<KEY_TYPE, VALUE_TYPE>:: delete_key_fn delete_key_fn, delete_value_fn delete_value_fn) { - m_inner = splay_tree_new ((splay_tree_compare_fn)compare_fn, - (splay_tree_delete_key_fn)delete_key_fn, - (splay_tree_delete_value_fn)delete_value_fn); + m_inner = splay_tree_new ((splay_tree_compare_fn) + (void (*) (void)) compare_fn, + (splay_tree_delete_key_fn) + (void (*) (void)) delete_key_fn, + (splay_tree_delete_value_fn) + (void (*) (void)) delete_value_fn); } /* Destructor for typed_splay_tree <K, V>. */ Index: libcpp/internal.h =================================================================== --- libcpp/internal.h (revision 253493) +++ libcpp/internal.h (working copy) @@ -708,7 +708,7 @@ struct _cpp_dir_only_callbacks { /* Called to print a block of lines. */ void (*print_lines) (int, const void *, size_t); - void (*maybe_print_line) (source_location); + bool (*maybe_print_line) (source_location); }; extern void _cpp_preprocess_dir_only (cpp_reader *,
Index: gcc/c/c-typeck.c =================================================================== --- gcc/c/c-typeck.c (revision 253328) +++ gcc/c/c-typeck.c (working copy) @@ -5667,6 +5667,20 @@ build_c_cast (location_t loc, tree type, tree expr pedwarn (loc, OPT_Wpedantic, "ISO C forbids " "conversion of object pointer to function pointer type"); + if (TREE_CODE (type) == POINTER_TYPE + && TREE_CODE (otype) == POINTER_TYPE + && TREE_CODE (TREE_TYPE (type)) == FUNCTION_TYPE + && TREE_CODE (TREE_TYPE (otype)) == FUNCTION_TYPE + && (TYPE_ARG_TYPES (TREE_TYPE (type)) + && TYPE_ARG_TYPES (TREE_TYPE (otype)) + ? !comptypes (TREE_TYPE (type), + TREE_TYPE (otype)) + : !comptypes (TREE_TYPE (TREE_TYPE (type)), + TREE_TYPE (TREE_TYPE (otype))))) + warning_at (loc, OPT_Wcast_function_type, + "cast between incompatible function types" + " from %qT to %qT", otype, type); + ovalue = value; value = convert (type, value); Index: gcc/c-family/c-lex.c =================================================================== --- gcc/c-family/c-lex.c (revision 253328) +++ gcc/c-family/c-lex.c (working copy) @@ -101,9 +101,11 @@ get_fileinfo (const char *name) struct c_fileinfo *fi; if (!file_info_tree) - file_info_tree = splay_tree_new ((splay_tree_compare_fn) strcmp, + file_info_tree = splay_tree_new ((splay_tree_compare_fn) + (uintptr_t) strcmp, 0, - (splay_tree_delete_value_fn) free); + (splay_tree_delete_value_fn) + (uintptr_t) free); n = splay_tree_lookup (file_info_tree, (splay_tree_key) name); if (n) Index: gcc/c-family/c-ppoutput.c =================================================================== --- gcc/c-family/c-ppoutput.c (revision 253328) +++ gcc/c-family/c-ppoutput.c (working copy) @@ -299,7 +299,7 @@ scan_translation_unit_directives_only (cpp_reader struct _cpp_dir_only_callbacks cb; cb.print_lines = print_lines_directives_only; - cb.maybe_print_line = (void (*) (source_location)) maybe_print_line; + cb.maybe_print_line = maybe_print_line; _cpp_preprocess_dir_only (pfile, &cb); } Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 253328) +++ gcc/c-family/c.opt (working copy) @@ -384,6 +384,10 @@ Wc++17-compat C++ ObjC++ Var(warn_cxx17_compat) Warning LangEnabledBy(C++ ObjC++,Wall) Warn about C++ constructs whose meaning differs between ISO C++ 2014 and ISO C++ 2017. +Wcast-function-type +C ObjC C++ ObjC++ Var(warn_cast_function_type) Warning EnabledBy(Wextra) +Warn about casts between incompatible function types. + Wcast-qual C ObjC C++ ObjC++ Var(warn_cast_qual) Warning Warn about casts which discard qualifiers. Index: gcc/cp/cxx-pretty-print.c =================================================================== --- gcc/cp/cxx-pretty-print.c (revision 253328) +++ gcc/cp/cxx-pretty-print.c (working copy) @@ -2935,8 +2935,16 @@ pp_cxx_constraint (cxx_pretty_printer *pp, tree t) -typedef c_pretty_print_fn pp_fun; +static void pp_c_type_specifier_seq (c_pretty_printer *pp, tree t) +{ + pp_cxx_type_specifier_seq ((cxx_pretty_printer *)pp, t); +} +static void pp_c_parameter_declaration_clause (c_pretty_printer *pp, tree t) +{ + pp_cxx_parameter_declaration_clause ((cxx_pretty_printer *)pp, t); +} + /* Initialization of a C++ pretty-printer object. */ cxx_pretty_printer::cxx_pretty_printer () @@ -2943,6 +2951,6 @@ cxx_pretty_printer::cxx_pretty_printer () : c_pretty_printer (), enclosing_scope (global_namespace) { - type_specifier_seq = (pp_fun) pp_cxx_type_specifier_seq; - parameter_list = (pp_fun) pp_cxx_parameter_declaration_clause; + type_specifier_seq = pp_c_type_specifier_seq; + parameter_list = pp_c_parameter_declaration_clause; } Index: gcc/cp/decl2.c =================================================================== --- gcc/cp/decl2.c (revision 253328) +++ gcc/cp/decl2.c (working copy) @@ -3480,7 +3480,8 @@ start_static_storage_duration_function (unsigned c priority_info_map = splay_tree_new (splay_tree_compare_ints, /*delete_key_fn=*/0, /*delete_value_fn=*/ - (splay_tree_delete_value_fn) &free); + (splay_tree_delete_value_fn) + (uintptr_t) free); /* We always need to generate functions for the DEFAULT_INIT_PRIORITY so enter it now. That way when we walk Index: gcc/cp/typeck.c =================================================================== --- gcc/cp/typeck.c (revision 253328) +++ gcc/cp/typeck.c (working copy) @@ -7261,8 +7261,21 @@ build_reinterpret_cast_1 (tree type, tree expr, bo && same_type_p (type, intype)) /* DR 799 */ return rvalue (expr); - else if ((TYPE_PTRFN_P (type) && TYPE_PTRFN_P (intype)) - || (TYPE_PTRMEMFUNC_P (type) && TYPE_PTRMEMFUNC_P (intype))) + else if (TYPE_PTRFN_P (type) && TYPE_PTRFN_P (intype)) + { + if ((complain & tf_warning) + && (TYPE_ARG_TYPES (TREE_TYPE (type)) + && TYPE_ARG_TYPES (TREE_TYPE (intype)) + ? !same_type_p (TREE_TYPE (type), + TREE_TYPE (intype)) + : !same_type_p (TREE_TYPE (TREE_TYPE (type)), + TREE_TYPE (TREE_TYPE (intype))))) + warning (OPT_Wcast_function_type, + "cast between incompatible function types" + " from %qH to %qI", intype, type); + return build_nop (type, expr); + } + else if (TYPE_PTRMEMFUNC_P (type) && TYPE_PTRMEMFUNC_P (intype)) return build_nop (type, expr); else if ((TYPE_PTRDATAMEM_P (type) && TYPE_PTRDATAMEM_P (intype)) || (TYPE_PTROBV_P (type) && TYPE_PTROBV_P (intype))) Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 253328) +++ gcc/doc/invoke.texi (working copy) @@ -267,7 +267,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-builtin-declaration-mismatch @gol -Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol -Wc++-compat -Wc++11-compat -Wc++14-compat @gol --Wcast-align -Wcast-align=strict -Wcast-qual @gol +-Wcast-align -Wcast-align=strict -Wcast-function-type -Wcast-qual @gol -Wchar-subscripts -Wchkp -Wcatch-value -Wcatch-value=@var{n} @gol -Wclobbered -Wcomment -Wconditionally-supported @gol -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol @@ -3899,6 +3899,7 @@ This enables some extra warning flags that are not name is still supported, but the newer name is more descriptive.) @gccoptlist{-Wclobbered @gol +-Wcast-function-type @gol -Wempty-body @gol -Wignored-qualifiers @gol -Wimplicit-fallthrough=3 @gol @@ -5948,6 +5949,15 @@ Warn whenever a pointer is cast such that the requ target is increased. For example, warn if a @code{char *} is cast to an @code{int *} regardless of the target machine. +@item -Wcast-function-type +@opindex Wcast-function-type +@opindex Wno-cast-function-type +Warn when a function pointer is cast to an incompatible function pointer. +When one of the function types uses variable arguments like this +@code{int f(...);}, then only the return value is checked, otherwise +the return value and the parameter types are checked. +This warning is enabled by @option{-Wextra}. + @item -Wwrite-strings @opindex Wwrite-strings @opindex Wno-write-strings Index: gcc/gengtype.c =================================================================== --- gcc/gengtype.c (revision 253328) +++ gcc/gengtype.c (working copy) @@ -4406,8 +4406,8 @@ write_root (outf_p f, pair_p v, type_p type, const if (!start_root_entry (f, v, name, line)) return; - oprintf (f, " (gt_pointer_walker) >_ggc_m_S,\n"); - oprintf (f, " (gt_pointer_walker) >_pch_n_S\n"); + oprintf (f, " >_ggc_m_S_nonconst,\n"); + oprintf (f, " >_pch_n_S_nonconst\n"); oprintf (f, " },\n"); } break; Index: gcc/ggc-page.c =================================================================== --- gcc/ggc-page.c (revision 253328) +++ gcc/ggc-page.c (working copy) @@ -1500,6 +1500,11 @@ gt_ggc_m_S (const void *p) return; } +void +gt_ggc_m_S_nonconst (void *p) +{ + gt_ggc_m_S (CONST_CAST(const void *, p)); +} /* User-callable entry points for marking string X. */ Index: gcc/ggc.h =================================================================== --- gcc/ggc.h (revision 253328) +++ gcc/ggc.h (working copy) @@ -98,6 +98,8 @@ extern int ggc_marked_p (const void *); /* PCH and GGC handling for strings, mostly trivial. */ extern void gt_pch_n_S (const void *); extern void gt_ggc_m_S (const void *); +extern void gt_pch_n_S_nonconst (void *); +extern void gt_ggc_m_S_nonconst (void *); /* End of GTY machinery API. */ Index: gcc/passes.c =================================================================== --- gcc/passes.c (revision 253328) +++ gcc/passes.c (working copy) @@ -1694,7 +1694,8 @@ remove_cgraph_node_from_order (cgraph_node *node, call CALLBACK on the current function. This function is global so that plugins can use it. */ void -do_per_function_toporder (void (*callback) (function *, void *data), void *data) +do_per_function_toporder (void (*callback) (function *, opt_pass *data), + opt_pass *data) { int i; @@ -2932,9 +2933,7 @@ execute_ipa_pass_list (opt_pass *pass) if (pass->sub->type == GIMPLE_PASS) { invoke_plugin_callbacks (PLUGIN_EARLY_GIMPLE_PASSES_START, NULL); - do_per_function_toporder ((void (*)(function *, void *)) - execute_pass_list, - pass->sub); + do_per_function_toporder (execute_pass_list, pass->sub); invoke_plugin_callbacks (PLUGIN_EARLY_GIMPLE_PASSES_END, NULL); } else if (pass->sub->type == SIMPLE_IPA_PASS Index: gcc/recog.h =================================================================== --- gcc/recog.h (revision 253328) +++ gcc/recog.h (working copy) @@ -276,43 +276,60 @@ typedef const char * (*insn_output_fn) (rtx *, rtx struct insn_gen_fn { - typedef rtx_insn * (*f0) (void); - typedef rtx_insn * (*f1) (rtx); - typedef rtx_insn * (*f2) (rtx, rtx); - typedef rtx_insn * (*f3) (rtx, rtx, rtx); - typedef rtx_insn * (*f4) (rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f5) (rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f6) (rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f7) (rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f8) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f9) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f10) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f11) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f12) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f13) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f14) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f15) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef rtx_insn * (*f16) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f0) (void); + typedef rtx (*f1) (rtx); + typedef rtx (*f2) (rtx, rtx); + typedef rtx (*f3) (rtx, rtx, rtx); + typedef rtx (*f4) (rtx, rtx, rtx, rtx); + typedef rtx (*f5) (rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f6) (rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f7) (rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f8) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f9) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f10) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f11) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f12) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f13) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f14) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f15) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); + typedef rtx (*f16) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); - typedef f0 stored_funcptr; + typedef rtx (*stored_funcptr) (...); - rtx_insn * operator () (void) const { return ((f0)func) (); } - rtx_insn * operator () (rtx a0) const { return ((f1)func) (a0); } - rtx_insn * operator () (rtx a0, rtx a1) const { return ((f2)func) (a0, a1); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2) const { return ((f3)func) (a0, a1, a2); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3) const { return ((f4)func) (a0, a1, a2, a3); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4) const { return ((f5)func) (a0, a1, a2, a3, a4); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5) const { return ((f6)func) (a0, a1, a2, a3, a4, a5); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6) const { return ((f7)func) (a0, a1, a2, a3, a4, a5, a6); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7) const { return ((f8)func) (a0, a1, a2, a3, a4, a5, a6, a7); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8) const { return ((f9)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9) const { return ((f10)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10) const { return ((f11)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11) const { return ((f12)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12) const { return ((f13)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13) const { return ((f14)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13, rtx a14) const { return ((f15)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14); } - rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13, rtx a14, rtx a15) const { return ((f16)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15); } + rtx_insn * operator () (void) const + { return (rtx_insn *) ((f0)func) (); } + rtx_insn * operator () (rtx a0) const + { return (rtx_insn *) ((f1)func) (a0); } + rtx_insn * operator () (rtx a0, rtx a1) const + { return (rtx_insn *) ((f2)func) (a0, a1); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2) const + { return (rtx_insn *) ((f3)func) (a0, a1, a2); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3) const + { return (rtx_insn *) ((f4)func) (a0, a1, a2, a3); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4) const + { return (rtx_insn *) ((f5)func) (a0, a1, a2, a3, a4); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5) const + { return (rtx_insn *) ((f6)func) (a0, a1, a2, a3, a4, a5); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6) const + { return (rtx_insn *) ((f7)func) (a0, a1, a2, a3, a4, a5, a6); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7) const + { return (rtx_insn *) ((f8)func) (a0, a1, a2, a3, a4, a5, a6, a7); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8) const + { return (rtx_insn *) ((f9)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9) const + { return (rtx_insn *) ((f10)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10) const + { return (rtx_insn *) ((f11)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11) const + { return (rtx_insn *)((f12)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12) const + { return (rtx_insn *) ((f13)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13) const + { return (rtx_insn *) ((f14)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13, rtx a14) const + { return (rtx_insn *) ((f15)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14); } + rtx_insn * operator () (rtx a0, rtx a1, rtx a2, rtx a3, rtx a4, rtx a5, rtx a6, rtx a7, rtx a8, rtx a9, rtx a10, rtx a11, rtx a12, rtx a13, rtx a14, rtx a15) const + { return (rtx_insn *) ((f16)func) (a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15); } // This is for compatibility of code that invokes functions like // (*funcptr) (arg) Index: gcc/stringpool.c =================================================================== --- gcc/stringpool.c (revision 253328) +++ gcc/stringpool.c (working copy) @@ -196,6 +196,11 @@ gt_pch_n_S (const void *x) >_pch_p_S); } +void +gt_pch_n_S_nonconst (void *x) +{ + gt_pch_n_S (CONST_CAST (const void *, x)); +} /* User-callable entry point for marking string X. */ Index: gcc/testsuite/c-c++-common/Wcast-function-type.c =================================================================== --- gcc/testsuite/c-c++-common/Wcast-function-type.c (revision 0) +++ gcc/testsuite/c-c++-common/Wcast-function-type.c (working copy) @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-options "-Wcast-function-type" } */ + +int f(long); + +typedef int (f1)(long); +typedef int (f2)(void*); +#ifdef __cplusplus +typedef int (f3)(...); +typedef void (f4)(...); +#else +typedef int (f3)(); +typedef void (f4)(); +#endif + +f1 *a; +f2 *b; +f3 *c; +f4 *d; + +void +foo (void) +{ + a = (f1 *) f; /* { dg-bogus "incompatible function types" } */ + b = (f2 *) f; /* { dg-warning "incompatible function types" } */ + c = (f3 *) f; /* { dg-bogus "incompatible function types" } */ + d = (f4 *) f; /* { dg-warning "incompatible function types" } */ +} Index: gcc/tree-dump.c =================================================================== --- gcc/tree-dump.c (revision 253328) +++ gcc/tree-dump.c (working copy) @@ -735,7 +735,8 @@ dump_node (const_tree t, dump_flags_t flags, FILE di.flags = flags; di.node = t; di.nodes = splay_tree_new (splay_tree_compare_pointers, 0, - (splay_tree_delete_value_fn) &free); + (splay_tree_delete_value_fn) + (uintptr_t) free); /* Queue up the first node. */ queue (&di, t, DUMP_NONE); Index: gcc/tree-pass.h =================================================================== --- gcc/tree-pass.h (revision 253328) +++ gcc/tree-pass.h (working copy) @@ -646,7 +646,8 @@ extern void register_one_dump_file (opt_pass *); extern bool function_called_by_processed_nodes_p (void); /* Declare for plugins. */ -extern void do_per_function_toporder (void (*) (function *, void *), void *); +extern void do_per_function_toporder (void (*) (function *, opt_pass *), + opt_pass *); extern void disable_pass (const char *); extern void enable_pass (const char *); Index: gcc/typed-splay-tree.h =================================================================== --- gcc/typed-splay-tree.h (revision 253328) +++ gcc/typed-splay-tree.h (working copy) @@ -75,9 +75,12 @@ inline typed_splay_tree<KEY_TYPE, VALUE_TYPE>:: delete_key_fn delete_key_fn, delete_value_fn delete_value_fn) { - m_inner = splay_tree_new ((splay_tree_compare_fn)compare_fn, - (splay_tree_delete_key_fn)delete_key_fn, - (splay_tree_delete_value_fn)delete_value_fn); + m_inner = splay_tree_new ((splay_tree_compare_fn) + (uintptr_t) compare_fn, + (splay_tree_delete_key_fn) + (uintptr_t) delete_key_fn, + (splay_tree_delete_value_fn) + (uintptr_t) delete_value_fn); } /* Destructor for typed_splay_tree <K, V>. */ Index: libcpp/identifiers.c =================================================================== --- libcpp/identifiers.c (revision 253328) +++ libcpp/identifiers.c (working copy) @@ -116,5 +116,5 @@ extern char proxy_assertion_broken[offsetof (struc void cpp_forall_identifiers (cpp_reader *pfile, cpp_cb cb, void *v) { - ht_forall (pfile->hash_table, (ht_cb) cb, v); + ht_forall_internal (pfile->hash_table, cb, v); } Index: libcpp/include/symtab.h =================================================================== --- libcpp/include/symtab.h (revision 253328) +++ libcpp/include/symtab.h (working copy) @@ -88,6 +88,11 @@ extern hashnode ht_lookup_with_hash (cpp_hash_tabl if the callback returns zero. */ typedef int (*ht_cb) (struct cpp_reader *, hashnode, const void *); extern void ht_forall (cpp_hash_table *, ht_cb, const void *); +struct cpp_hashnode; +extern void ht_forall_internal (cpp_hash_table *, + int (*)(struct cpp_reader *, + cpp_hashnode *, void *), + void *); /* For all nodes in TABLE, call the callback. If the callback returns a nonzero value, the node is removed from the table. */ Index: libcpp/internal.h =================================================================== --- libcpp/internal.h (revision 253328) +++ libcpp/internal.h (working copy) @@ -708,7 +708,7 @@ struct _cpp_dir_only_callbacks { /* Called to print a block of lines. */ void (*print_lines) (int, const void *, size_t); - void (*maybe_print_line) (source_location); + bool (*maybe_print_line) (source_location); }; extern void _cpp_preprocess_dir_only (cpp_reader *, Index: libcpp/symtab.c =================================================================== --- libcpp/symtab.c (revision 253328) +++ libcpp/symtab.c (working copy) @@ -236,6 +236,26 @@ ht_forall (cpp_hash_table *table, ht_cb cb, const while (++p < limit); } +/* Like ht_forall, but with different parameter types in the callback. */ +void +ht_forall_internal (cpp_hash_table *table, + int (*cb)(struct cpp_reader *, + cpp_hashnode *, void *), + void *v) +{ + hashnode *p, *limit; + + p = table->entries; + limit = p + table->nslots; + do + if (*p && *p != DELETED) + { + if ((*cb) (table->pfile, (cpp_hashnode *)*p, v) == 0) + break; + } + while (++p < limit); +} + /* Like ht_forall, but a nonzero return from the callback means that the entry should be removed from the table. */ void Index: libgo/runtime/runtime.h =================================================================== --- libgo/runtime/runtime.h (revision 253328) +++ libgo/runtime/runtime.h (working copy) @@ -94,7 +94,7 @@ struct String struct FuncVal { - void (*fn)(void); + void (*fn)(); // variable-size, fn-specific data here };