diff mbox

have ifunc resolver's return type match target

Message ID f967b8f5-6802-f848-ba68-b1df8069d379@gmail.com
State New, archived
Headers

Commit Message

Martin Sebor Aug. 20, 2017, 10:30 p.m. UTC 
  The following GCC patch has been submitted for review.  It
helps detect mismatches between the type of an ifunc or alias
declaration and the type of the resolver or alias.

   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01103.html

To let Glibc take advantage of this type checking and avoid
warnings when using the patched GCC when the change above is
committed, the patch below adjusts the Glibc __ifunc_resolver
macro to declare the ifunc resolver so that its return type
matches that of the target.  (I was going to wait to submit it
until after the GCC patch has been accepted but per Joseph's
suggestion I'm posting it here ahead of time.)

The patch has been tested on its own with the system GCC 6.3
and with the patched GCC on x86_64-linux with no regressions.

Martin

      __typeof (type_name) *res = expr;                                  \

Comments

Joseph Myers Aug. 21, 2017, 1:26 p.m. UTC | #1 
On Sun, 20 Aug 2017, Martin Sebor wrote:

> To let Glibc take advantage of this type checking and avoid
> warnings when using the patched GCC when the change above is
> committed, the patch below adjusts the Glibc __ifunc_resolver
> macro to declare the ifunc resolver so that its return type
> matches that of the target.  (I was going to wait to submit it
> until after the GCC patch has been accepted but per Joseph's
> suggestion I'm posting it here ahead of time.)
> 
> The patch has been tested on its own with the system GCC 6.3
> and with the patched GCC on x86_64-linux with no regressions.

OK (with a ChangeLog entry, of course).
Florian Weimer Aug. 23, 2017, 9:28 a.m. UTC | #2 
On 08/21/2017 12:30 AM, Martin Sebor wrote:
> The following GCC patch has been submitted for review.  It
> helps detect mismatches between the type of an ifunc or alias
> declaration and the type of the resolver or alias.
> 
>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01103.html
> 
> To let Glibc take advantage of this type checking and avoid
> warnings when using the patched GCC when the change above is
> committed, the patch below adjusts the Glibc __ifunc_resolver
> macro to declare the ifunc resolver so that its return type
> matches that of the target.  (I was going to wait to submit it
> until after the GCC patch has been accepted but per Joseph's
> suggestion I'm posting it here ahead of time.)

Do we have to backport both patches to older releases, too, so that they
keep building with a newer GCC?

Thanks,
Florian
Martin Sebor Aug. 23, 2017, 2:48 p.m. UTC | #3 
On 08/23/2017 03:28 AM, Florian Weimer wrote:
> On 08/21/2017 12:30 AM, Martin Sebor wrote:
>> The following GCC patch has been submitted for review.  It
>> helps detect mismatches between the type of an ifunc or alias
>> declaration and the type of the resolver or alias.
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01103.html
>>
>> To let Glibc take advantage of this type checking and avoid
>> warnings when using the patched GCC when the change above is
>> committed, the patch below adjusts the Glibc __ifunc_resolver
>> macro to declare the ifunc resolver so that its return type
>> matches that of the target.  (I was going to wait to submit it
>> until after the GCC patch has been accepted but per Joseph's
>> suggestion I'm posting it here ahead of time.)
>
> Do we have to backport both patches to older releases, too, so that they
> keep building with a newer GCC?

It would make sense to me if that's how Glibc usually deals with
these sorts of things (i.e., changing code to avoid new warnings).
The other (obvious) alternative is for people to suppress the
warnings when using the new compiler.

Martin
Florian Weimer Sept. 19, 2017, 5:07 p.m. UTC | #4 
On 08/23/2017 04:48 PM, Martin Sebor wrote:
> On 08/23/2017 03:28 AM, Florian Weimer wrote:
>> On 08/21/2017 12:30 AM, Martin Sebor wrote:
>>> The following GCC patch has been submitted for review.  It
>>> helps detect mismatches between the type of an ifunc or alias
>>> declaration and the type of the resolver or alias.
>>>
>>>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01103.html
>>>
>>> To let Glibc take advantage of this type checking and avoid
>>> warnings when using the patched GCC when the change above is
>>> committed, the patch below adjusts the Glibc __ifunc_resolver
>>> macro to declare the ifunc resolver so that its return type
>>> matches that of the target.  (I was going to wait to submit it
>>> until after the GCC patch has been accepted but per Joseph's
>>> suggestion I'm posting it here ahead of time.)
>>
>> Do we have to backport both patches to older releases, too, so that they
>> keep building with a newer GCC?
> 
> It would make sense to me if that's how Glibc usually deals with
> these sorts of things (i.e., changing code to avoid new warnings).
> The other (obvious) alternative is for people to suppress the
> warnings when using the new compiler.

Okay, I backported a few toolchain enablement patches to the 2.26 branch.

Thanks,
Florian
diff mbox

Patch

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index fe3ab81..5413e56 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -790,7 +790,8 @@  for linking")

  /* Helper / base  macros for indirect function symbols.  */
  #define __ifunc_resolver(type_name, name, expr, arg, init, classifier) \
-  classifier inhibit_stack_protector void *name##_ifunc (arg) 
              \
+  classifier inhibit_stack_protector                                   \
+  __typeof (type_name) *name##_ifunc (arg)                             \
    {                                                                    \
      init ();                                                           \