[PATCH v5 09/13] LoongArch: Linux ABI -- __ifunc_arg_t

[caiyinyu]

Fixed. Thanks.

 >>>>>>>>>>>>>

diff --git a/sysdeps/loongarch/dl-irel.h b/sysdeps/loongarch/dl-irel.h
index 0dfe78c217..4440453f06 100644
--- a/sysdeps/loongarch/dl-irel.h
+++ b/sysdeps/loongarch/dl-irel.h
@@ -21,13 +21,19 @@

  #include <stdio.h>
  #include <unistd.h>
+#include <sys/ifunc.h>

  #define ELF_MACHINE_IRELA 1

  static inline ElfW (Addr) __attribute ((always_inline))
  elf_ifunc_invoke (ElfW (Addr) addr)
  {
-  return ((ElfW (Addr) (*) (void)) (addr)) ();
+  __ifunc_arg_t arg =
+  {
+    ._size = sizeof (__ifunc_arg_t),
+    ._hwcap = GLRO(dl_hwcap),
+  };
+  return ((ElfW(Addr) (*) (const __ifunc_arg_t *)) (addr)) (&arg);
  }

<<<<<<<<<<<<<<<<


在 2022/6/8 下午9:16, Adhemerval Zanella 写道:
>
> On 08/06/2022 03:01, caiyinyu wrote:
>> I made some changes:
>>
>> static inline ElfW (Addr) __attribute ((always_inline))
>> elf_ifunc_invoke (ElfW (Addr) addr)
>> {
>>    __ifunc_arg_t arg =
>>    {
>>      ._size = sizeof (__ifunc_arg_t),
>>      ._hwcap = GLRO(dl_hwcap),
>>    };
>>    return ((ElfW(Addr) (*) *(const __ifunc_arg_t *, void *)*) (addr))
>>           *(&arg, NULL)*;
>> }
>>
> Why would you need the extra argument if now you are passing a struct?
> The idea is if you need extra space (for instance to pack another
> hwcap or any other arch-specific information) you define a new
> __ifunc_arg_t with a different name.  The resolver function will then
> check the size before accessing the correct expected struct.
>
>> otherwise:
>>
>> static inline ElfW (Addr) __attribute ((always_inline))
>> elf_ifunc_invoke (ElfW (Addr) addr)
>> {
>>    __ifunc_arg_t arg =
>>    {
>>      ._size = sizeof (__ifunc_arg_t),
>>      ._hwcap = GLRO(dl_hwcap),
>>    };
>>    return ((ElfW(Addr) (*) *(uint64_t, void *)*) (addr))
>>           *((uint64_t) &arg, NULL)*;
>> }
>>
> I would prefer to avoid alising violations if possible (and uint64_t is
> not usually the correct type for pointer to integer conversion).
>
>> THANKS.
>>
>>
>> diff --git a/sysdeps/loongarch/sys/ifunc.h b/sysdeps/loongarch/sys/ifunc.h
>> new file mode 100644
>> index 0000000000..461df20c96
>> --- /dev/null
>> +++ b/sysdeps/loongarch/sys/ifunc.h
>> @@ -0,0 +1,30 @@
>> +/* Definitions used by LoongArch indirect function resolvers.
>> +   Copyright (C) 2022 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <https://www.gnu.org/licenses/>.  */
>> +
>> +#ifndef _SYS_IFUNC_H
>> +#define _SYS_IFUNC_H
>> +
>> +struct __ifunc_arg_t
>> +{
>> +  unsigned long _size; /* Size of the struct, so it can grow.  */
>> +  unsigned long _hwcap;
>> +};
>> +
>> +typedef struct __ifunc_arg_t __ifunc_arg_t;
>> +
>> +#endif
>>
>> <<<<<<<<<<<<<<
>>
>>
>> 在 2022/6/7 下午9:56, Adhemerval Zanella 写道:
>>> On 07/06/2022 06:32, caiyinyu wrote:
>>>> +static inline ElfW (Addr) __attribute ((always_inline))
>>>> +elf_ifunc_invoke (ElfW (Addr) addr)
>>>> +{
>>>> +  return ((ElfW (Addr) (*) (void)) (addr)) ();
>>>>
>>>> At least for RISCV, sparc, aarch64, powerpc, arm; the ifunc resolver expects
>>>> a unsigned long int begin the hardware capability from kernelk (AT_HWCAP).
>>>>
>>>> AArch64 also extends it by passing both uint64_t and a struct with both
>>>> AT_HWCAP and AT_HWCAP2.  I am not sure if loongarch will ever use more
>>>> than the AT_HWCAP.
>>>> *Currently ifuncs (like __memchr_ifunc, __memcpy_ifunc ...) are not used in loongarch, and we will add these in future.*
>>>> *or we can add the following patch (now **AT_HWCAP only) though not woking: ****>>>>>>>>>>>*
>>>>
>>>> diff --git a/sysdeps/loongarch/dl-irel.h b/sysdeps/loongarch/dl-irel.h
>>>> index 0dfe78c217..ef248095b9 100644
>>>> --- a/sysdeps/loongarch/dl-irel.h
>>>> +++ b/sysdeps/loongarch/dl-irel.h
>>>> @@ -21,13 +21,18 @@
>>>>   
>>>>   #include <stdio.h>
>>>>   #include <unistd.h>
>>>> +#include <ldsodefs.h>
>>>> +#include <sysdep.h>
>>>>   
>>>>   #define ELF_MACHINE_IRELA 1
>>>>   
>>>>   static inline ElfW (Addr) __attribute ((always_inline))
>>>>   elf_ifunc_invoke (ElfW (Addr) addr)
>>>>   {
>>>> -  return ((ElfW (Addr) (*) (void)) (addr)) ();
>>>> +  /* The second argument is a void pointer to preserve the extension
>>>> +     fexibility.  */
>>>> +  return ((ElfW(Addr) (*) (uint64_t, void *)) (addr))
>>>> +        (GLRO(dl_hwcap), NULL);
>>>>   }
>>>>   
>>>>   static inline void __attribute ((always_inline))
>>>>
>>>> *<<<<<<<<<<<<<<<<<<*
>>> AArch64 added the extra argument to preserve backwards compatibility, which
>>> is not the case here.  Since ifunc is also used outside glibc, maybe it would
>>> be better to  use the extendable struct as default:
>>>
>>>
>>>   struct __ifunc_arg_t
>>>   {
>>>     unsigned long int _size; /* Size of the struct, so it can grow.  */
>>>     unsigned long int _hwcap;
>>>   };
>>>
>>>   static inline ElfW (Addr) __attribute ((always_inline))
>>>   elf_ifunc_invoke (ElfW (Addr) addr)
>>>   {
>>>     __ifunc_arg_t arg =
>>>     {
>>>       ._size = sizeof (__ifunc_arg_t),
>>>       ._hwcap = GLRO(dl_hwcap),
>>>     }
>>>     return ((ElfW(Addr) (*) (uint64_t, void *)) (addr)) (&arg);
>>>   }
>>>
>>> And then export __ifunc_arg_t on the sys/ifunc.h header like aarch64.