From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id E45163856099 for ; Thu, 9 Jun 2022 04:01:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E45163856099 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn Received: from [10.20.4.187] (unknown [10.20.4.187]) by mail.loongson.cn (Coremail) with SMTP id AQAAf9Dx30+ocKFi3igzAA--.17106S3; Thu, 09 Jun 2022 12:01:45 +0800 (CST) Subject: Re: [PATCH v5 09/13] LoongArch: Linux ABI -- __ifunc_arg_t To: Adhemerval Zanella , libc-alpha@sourceware.org Cc: joseph_myers@mentor.com, xuchenghua@loongson.cn References: <20220601021836.1082160-1-caiyinyu@loongson.cn> <20220601021836.1082160-10-caiyinyu@loongson.cn> <81157172-4f55-3987-d6e0-968fc8a13c4f@linaro.org> <4b4ed912-244f-5a3e-38bd-574190a44154@loongson.cn> <6ce5fddd-969d-d509-65c4-a257a000bad8@linaro.org> <8a744969-518e-f6ea-e72c-ec6d35cdffcf@loongson.cn> <6b1867a7-33e4-fa76-2dd8-46137d1680f8@linaro.org> From: caiyinyu Message-ID: <94238960-b1ab-b2c7-6efd-1c6fc8491ae0@loongson.cn> Date: Thu, 9 Jun 2022 12:01:44 +0800 User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <6b1867a7-33e4-fa76-2dd8-46137d1680f8@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-CM-TRANSID: AQAAf9Dx30+ocKFi3igzAA--.17106S3 X-Coremail-Antispam: 1UD129KBjvJXoW3AFyDKr13uw45XrWfKryrtFb_yoW7KryrpF y5AFWUCFs7tayxGr92gr13Z3Wrtr1fJFy7ZF15Xa4qyrsxtry0qrWa9ryq9a48JrW8Kr4Y qrW5u34fAanrJaDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUvS14x267AKxVWUJVW8JwAFc2x0x2IEx4CE42xK8VAvwI8IcIk0 rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2ocxC64kIII0Yj41l84x0c7CEw4AK67xGY2AK02 1l84ACjcxK6xIIjxv20xvE14v26ryj6F1UM28EF7xvwVC0I7IYx2IY6xkF7I0E14v26F4j 6r4UJwA2z4x0Y4vEx4A2jsIE14v26F4UJVW0owA2z4x0Y4vEx4A2jsIEc7CjxVAFwI0_Gc CE3s1le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqx4xG64xvF2IEw4CE5I8CrVC2j2WlYx0E 2Ix0cI8IcVAFwI0_Jr0_Jr4lYx0Ex4A2jsIE14v26r1j6r4UMcvjeVCFs4IE7xkEbVWUJV W8JwACjcxG0xvEwIxGrwACjI8F5VA0II8E6IAqYI8I648v4I1lc7I2V7IY0VAS07AlzVAY IcxG8wCY02Avz4vE-syl42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2 IqxVAqx4xG67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v2 6r126r1DMIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2 IY6xkF7I0E14v26r1j6r4UMIIF0xvE42xK8VAvwI8IcIk0rVWrZr1j6s0DMIIF0xvEx4A2 jsIE14v26r1j6r4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Jr0_GrUvcSsGvfC2KfnxnUUI43 ZEXa7VUbrMaUUUUUU== X-CM-SenderInfo: 5fdl5xhq1xqz5rrqw2lrqou0/ X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, BODY_8BITS, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jun 2022 04:01:52 -0000 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  #include +#include  #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 >> +   .  */ >> + >> +#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 >>>> #include >>>> +#include >>>> +#include >>>> >>>> #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.