From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id EC1C03858C74 for ; Fri, 29 Dec 2023 10:24:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EC1C03858C74 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn ARC-Filter: OpenARC Filter v1.0.0 sourceware.org EC1C03858C74 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=114.242.206.163 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703845502; cv=none; b=oWyifLCoaIwQmAS6WFIkwtLKamko7ql4xqWIs1HQsbKj87vBcRRy+pcdOakkaSJsdwfJ46uJTVzJnc7YMB+XTtizcO5ZUBD11wrNcLFAKzZMs2IhPmlQ5KoX/844pWBVAFfcmGT+PQPc0hi2Kuwr3CDaCPIb23qDy/MVE/5kWMc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703845502; c=relaxed/simple; bh=VJP4wvZDsU24/GY+38IP0hfOlJekEGrgM4pdN3HfTWI=; h=Subject:To:From:Message-ID:Date:MIME-Version; b=TuITxxehz8mQeS65pU7A4IrC8u6jHCabPo4fPFg6mruZr9SAknpALphX9imEiczKzSQhz/9ctSfc22zogL2LAZq5Yh/2/9MKEZw6tWi4mN4z7KWLOJeivn4kHHCAdNz6DcpJknT2dXaBZSnYIxKqa9ROJGT31VU7eYuN85Oh3gY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from loongson.cn (unknown [10.20.4.103]) by gateway (Coremail) with SMTP id _____8Dx++p4no5lc24AAA--.1883S3; Fri, 29 Dec 2023 18:24:56 +0800 (CST) Received: from [10.20.4.103] (unknown [10.20.4.103]) by localhost.localdomain (Coremail) with SMTP id AQAAf8Cx2r13no5lwM8PAA--.24078S3; Fri, 29 Dec 2023 18:24:55 +0800 (CST) Subject: Re: [PATCH v5 2/5] LoongArch: Add support for TLSDESC in ld. To: Tatsuyuki Ishi Cc: binutils@sourceware.org, xuchenghua@loongson.cn, chenglulu@loongson.cn, liuzhensong@loongson.cn, mengqinggang@loongson.cn, xry111@xry111.site, i.swmail@xen0n.name, maskray@google.com, luweining@loongson.cn, wanglei@loongson.cn, hejinyang@loongson.cn References: <20231222114243.1836112-1-cailulu@loongson.cn> <20231222114243.1836112-3-cailulu@loongson.cn> <7DE21EC8-207D-41AA-B82E-0950D482F07B@gmail.com> From: Lulu Cai Message-ID: <31b541b7-40ef-0c1c-34ae-d023c7842d7e@loongson.cn> Date: Fri, 29 Dec 2023 18:24:55 +0800 User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <7DE21EC8-207D-41AA-B82E-0950D482F07B@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-CM-TRANSID:AQAAf8Cx2r13no5lwM8PAA--.24078S3 X-CM-SenderInfo: xfdlz3tox6z05rqj20fqof0/1tbiAQAAB2WOLK8I9AABsT X-Coremail-Antispam: 1Uk129KBj9fXoW3Zw4DGw47Kr1UWryUCFWkZrc_yoW8Jry3Zo WFqF1xAw4kJFWfArZ8AasrWFyxG395GrW5Aw1Yvwnaqay29FyY9rWUAryUXayfWFW2y3W8 CFy5WayDAF9xGF13l-sFpf9Il3svdjkaLaAFLSUrUUUUjb8apTn2vfkv8UJUUUU8wcxFpf 9Il3svdxBIdaVrn0xqx4xG64xvF2IEw4CE5I8CrVC2j2Jv73VFW2AGmfu7bjvjm3AaLaJ3 UjIYCTnIWjp_UUUYx7kC6x804xWl14x267AKxVWUJVW8JwAFc2x0x2IEx4CE42xK8VAvwI 8IcIk0rVWrJVCq3wAFIxvE14AKwVWUXVWUAwA2ocxC64kIII0Yj41l84x0c7CEw4AK67xG Y2AK021l84ACjcxK6xIIjxv20xvE14v26r1j6r1xM28EF7xvwVC0I7IYx2IY6xkF7I0E14 v26r1j6r4UM28EF7xvwVC2z280aVAFwI0_Gr1j6F4UJwA2z4x0Y4vEx4A2jsIEc7CjxVAF wI0_Gr1j6F4UJwAS0I0E0xvYzxvE52x082IY62kv0487Mc804VCY07AIYIkI8VC2zVCFFI 0UMc02F40EFcxC0VAKzVAqx4xG6I80ewAv7VC0I7IYx2IY67AKxVWUGVWUXwAv7VC2z280 aVAFwI0_Jr0_Gr1lOx8S6xCaFVCjc4AY6r1j6r4UM4x0Y48IcVAKI48JMxk0xIA0c2IEe2 xFo4CEbIxvr21l42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2IqxVAq x4xG67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v26r1q6r 43MIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2IY6xkF 7I0E14v26r1j6r4UMIIF0xvE42xK8VAvwI8IcIk0rVWUJVWUCwCI42IY6I8E87Iv67AKxV WUJVW8JwCI42IY6I8E87Iv6xkF7I0E14v26r1j6r4UYxBIdaVFxhVjvjDU0xZFpf9x07jU sqXUUUUU= X-Spam-Status: No, score=-14.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,NICE_REPLY_A,SPF_HELO_NONE,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 List-Id: On 2023/12/28 at 10:54 PM, Tatsuyuki Ishi Wrote: > Hi, author of RISC-V TLSDESC patches here. I recently learned that LoongArch is working on a similar TLSDESC extension too. I’ll leave some of my thoughts on the patch set. Thank you for your suggestions, I will add a reply after these suggestions. > >> On Dec 22, 2023, at 20:42, Lulu Cai wrote: >> >> 1.The linker for each DESC generates a R_LARCH_TLS_DESC64 dynamic >> relocation, which relocation is placed at .rela.dyn. >> TLSDESC always allocates two GOT slots and one dynamic relocation >> space to TLSDESC. >> 2. When using multiple ways to access the same TLS variable, a >> maximum of 5 GOT slots are used. For example, using GD, TLSDESC, >> and IE to access the same TLS variable, GD always uses the first >> two of the five GOT, TLSDESC uses the third and fourth, and IE >> uses the last. >> --- >> bfd/elfnn-loongarch.c | 168 ++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 146 insertions(+), 22 deletions(-) >> >> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c >> index faffe27294a..95a39148f73 100644 >> --- a/bfd/elfnn-loongarch.c >> +++ b/bfd/elfnn-loongarch.c >> @@ -48,6 +48,12 @@ struct loongarch_elf_link_hash_entry >> #define GOT_TLS_GD 2 >> #define GOT_TLS_IE 4 >> #define GOT_TLS_LE 8 >> +#define GOT_TLS_GDESC 16 >> + >> +#define GOT_TLS_GD_BOTH_P(tls_type) \ >> + ((tls_type & GOT_TLS_GD) && (tls_type & GOT_TLS_GDESC)) >> +#define GOT_TLS_GD_ANY_P(tls_type) \ >> + ((tls_type & GOT_TLS_GD) || (tls_type & GOT_TLS_GDESC)) >> char tls_type; >> }; >> >> @@ -563,6 +569,7 @@ loongarch_elf_record_tls_and_got_reference (bfd *abfd, >> case GOT_NORMAL: >> case GOT_TLS_GD: >> case GOT_TLS_IE: >> + case GOT_TLS_GDESC: >> /* Need GOT. */ >> if (htab->elf.sgot == NULL >> && !loongarch_elf_create_got_section (htab->elf.dynobj, info)) >> @@ -750,6 +757,14 @@ loongarch_elf_check_relocs (bfd *abfd, struct bfd_link_info *info, >> return false; >> break; >> >> + case R_LARCH_TLS_DESC_PC_HI20: >> + case R_LARCH_TLS_DESC_HI20: >> + if (!loongarch_elf_record_tls_and_got_reference (abfd, info, h, >> + r_symndx, >> + GOT_TLS_GDESC)) >> + return false; >> + break; >> + >> case R_LARCH_ABS_HI20: >> case R_LARCH_SOP_PUSH_ABSOLUTE: >> if (h != NULL) >> @@ -1130,7 +1145,7 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf) >> >> s = htab->elf.sgot; >> h->got.offset = s->size; >> - if (tls_type & (GOT_TLS_GD | GOT_TLS_IE)) >> + if (tls_type & (GOT_TLS_GD | GOT_TLS_IE | GOT_TLS_GDESC)) >> { >> /* TLS_GD needs two dynamic relocs and two GOT slots. */ >> if (tls_type & GOT_TLS_GD) >> @@ -1167,7 +1182,15 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf) >> htab->elf.srelgot->size += sizeof (ElfNN_External_Rela); >> } >> } >> + >> + /* TLS_DESC needs one dynamic reloc and two GOT slot. */ >> + if (tls_type & GOT_TLS_GDESC) >> + { >> + s->size += GOT_ENTRY_SIZE * 2; >> + htab->elf.srelgot->size += sizeof (ElfNN_External_Rela); >> + } >> } >> + >> else >> { >> s->size += GOT_ENTRY_SIZE; >> @@ -1670,19 +1693,34 @@ loongarch_elf_size_dynamic_sections (bfd *output_bfd, >> if (0 < *local_got) >> { >> *local_got = s->size; >> + if (*local_tls_type & (GOT_TLS_GD | GOT_TLS_IE | GOT_TLS_GDESC)) >> + { >> + /* TLS gd use two got. */ >> + if (*local_tls_type & GOT_TLS_GD) >> + { >> + s->size += 2 * GOT_ENTRY_SIZE; >> + if (!bfd_link_executable (info)) >> + srel->size += sizeof (ElfNN_External_Rela); >> + } >> >> - /* TLS gd use two got. */ >> - if (*local_tls_type & GOT_TLS_GD) >> - s->size += GOT_ENTRY_SIZE * 2; >> - else >> - /* Normal got, tls ie/ld use one got. */ >> - s->size += GOT_ENTRY_SIZE; >> + /* TLS_DESC use two got. */ >> + if (*local_tls_type & GOT_TLS_GDESC) >> + { >> + s->size += 2 * GOT_ENTRY_SIZE; >> + srel->size += sizeof (ElfNN_External_Rela); >> + } >> >> - if (bfd_link_executable (info) >> - && (*local_tls_type & (GOT_TLS_GD| GOT_TLS_IE))) >> - ;/* Do nothing. */ >> + /* TLS ie and use one got. */ >> + if (*local_tls_type & GOT_TLS_IE) >> + { >> + s->size += GOT_ENTRY_SIZE; >> + if (!bfd_link_executable (info)) >> + srel->size += sizeof (ElfNN_External_Rela); >> + } >> + } >> else >> { >> + s->size += GOT_ENTRY_SIZE; >> srel->size += sizeof (ElfNN_External_Rela); >> } >> } >> @@ -2126,6 +2164,15 @@ perform_relocation (const Elf_Internal_Rela *rel, asection *input_section, >> case R_LARCH_TLS_GD_HI20: >> case R_LARCH_PCREL20_S2: >> case R_LARCH_CALL36: >> + case R_LARCH_TLS_DESC_PC_HI20: >> + case R_LARCH_TLS_DESC_PC_LO12: >> + case R_LARCH_TLS_DESC64_PC_LO20: >> + case R_LARCH_TLS_DESC64_PC_HI12: >> + case R_LARCH_TLS_DESC_HI20: >> + case R_LARCH_TLS_DESC_LO12: >> + case R_LARCH_TLS_DESC64_LO20: >> + case R_LARCH_TLS_DESC64_HI12: >> + >> r = loongarch_check_offset (rel, input_section); >> if (r != bfd_reloc_ok) >> break; >> @@ -2135,6 +2182,11 @@ perform_relocation (const Elf_Internal_Rela *rel, asection *input_section, >> contents, value); >> break; >> >> + case R_LARCH_TLS_DESC_LD: >> + case R_LARCH_TLS_DESC_CALL: >> + r = bfd_reloc_ok; >> + break; >> + >> case R_LARCH_RELAX: >> break; >> >> @@ -2383,10 +2435,10 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> struct elf_link_hash_entry *h = NULL; >> const char *name; >> bfd_reloc_status_type r = bfd_reloc_ok; >> - bool is_ie, is_undefweak, unresolved_reloc, defined_local; >> + bool is_ie, is_desc, is_undefweak, unresolved_reloc, defined_local; >> bool resolved_local, resolved_dynly, resolved_to_const; >> char tls_type; >> - bfd_vma relocation, off, ie_off; >> + bfd_vma relocation, off, ie_off, desc_off; >> int i, j; >> >> howto = loongarch_elf_rtype_to_howto (input_bfd, r_type); >> @@ -2515,6 +2567,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> >> BFD_ASSERT (!resolved_local || defined_local); >> >> + is_desc = false; >> is_ie = false; >> switch (r_type) >> { >> @@ -3405,6 +3458,8 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> case R_LARCH_TLS_LD_HI20: >> case R_LARCH_TLS_GD_PC_HI20: >> case R_LARCH_TLS_GD_HI20: >> + case R_LARCH_TLS_DESC_PC_HI20: >> + case R_LARCH_TLS_DESC_HI20: >> BFD_ASSERT (rel->r_addend == 0); >> unresolved_reloc = false; >> >> @@ -3412,6 +3467,10 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> || r_type == R_LARCH_TLS_IE_HI20) >> is_ie = true; >> >> + if (r_type == R_LARCH_TLS_DESC_PC_HI20 >> + || r_type == R_LARCH_TLS_DESC_HI20) >> + is_desc = true; >> + > I see that the got_off assignment logic is duplicated for R_LARCH_SOP_PUSH_TLS_{GOT, GD}, but that path was not modified in this patch. Is this OK? Ideally, the logic should not be duplicated at all. R_LARCH_SOP_PUSH_TLS_{GOT, GD} are relocations in the old toolchain that are no longer used by the new toolchain. So we are not going to modify this. > >> bfd_vma got_off = 0; >> if (h != NULL) >> { >> @@ -3426,9 +3485,19 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> >> BFD_ASSERT (got_off != MINUS_ONE); >> >> - ie_off = 0; >> tls_type = _bfd_loongarch_elf_tls_type (input_bfd, h, r_symndx); >> - if ((tls_type & GOT_TLS_GD) && (tls_type & GOT_TLS_IE)) >> + >> + /* If a tls variable is accessed in multiple ways, GD uses >> + the first two slots of GOT, desc follows with two slots, >> + and IE uses one slot at the end. */ >> + desc_off = 0; >> + if (GOT_TLS_GD_BOTH_P (tls_type)) >> + desc_off = 2 * GOT_ENTRY_SIZE; >> + >> + ie_off = 0; >> + if (GOT_TLS_GD_BOTH_P (tls_type) && (tls_type & GOT_TLS_IE)) >> + ie_off = 4 * GOT_ENTRY_SIZE; >> + else if (GOT_TLS_GD_ANY_P (tls_type) && (tls_type & GOT_TLS_IE)) > I think the calculation should be done additively, instead of checking for every possible combination. That is, > > off = 0; > if (tls gd) > off += gd size; > desc_off = off; > if (tls ie) > off += desc size; > ie_off = off; > > would be much simpler. Agreed, I will add these changes in a later version. >> ie_off = 2 * GOT_ENTRY_SIZE; >> >> if ((got_off & 1) == 0) >> @@ -3477,6 +3546,21 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> loongarch_elf_append_rela (output_bfd, relgot, &rela); >> } >> } >> + if (tls_type & GOT_TLS_GDESC) >> + { >> + /* Unless it is a static link, DESC always emits a >> + dynamic relocation. */ >> + int indx = h && h->dynindx != -1 ? h->dynindx : 0; >> + rela.r_offset = sec_addr (got) + got_off + desc_off; >> + rela.r_addend = 0; >> + if (indx == 0) >> + rela.r_addend = relocation - elf_hash_table (info)->tls_sec->vma; >> + >> + rela.r_info = ELFNN_R_INFO (indx, R_LARCH_TLS_DESCNN); >> + loongarch_elf_append_rela (output_bfd, relgot, &rela); >> + bfd_put_NN (output_bfd, 0, >> + got->contents + got_off + desc_off); >> + } >> if (tls_type & GOT_TLS_IE) >> { >> rela.r_offset = sec_addr (got) + got_off + ie_off; >> @@ -3504,16 +3588,52 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> } >> } >> } >> - relocation = (got_off & (~(bfd_vma)1)) + sec_addr (got) >> - + (is_ie ? ie_off : 0); >> + relocation = (got_off & (~(bfd_vma)1)) + sec_addr (got); >> + if (is_desc) >> + relocation += desc_off; >> + else if (is_ie) >> + relocation += ie_off; >> >> if (r_type == R_LARCH_TLS_LD_PC_HI20 >> || r_type == R_LARCH_TLS_GD_PC_HI20 >> - || r_type == R_LARCH_TLS_IE_PC_HI20) >> + || r_type == R_LARCH_TLS_IE_PC_HI20 >> + || r_type == R_LARCH_TLS_DESC_PC_HI20) >> RELOCATE_CALC_PC32_HI20 (relocation, pc); >> >> break; >> >> + case R_LARCH_TLS_DESC_PC_LO12: >> + case R_LARCH_TLS_DESC64_PC_LO20: >> + case R_LARCH_TLS_DESC64_PC_HI12: >> + case R_LARCH_TLS_DESC_LO12: >> + case R_LARCH_TLS_DESC64_LO20: >> + case R_LARCH_TLS_DESC64_HI12: >> + { >> + unresolved_reloc = false; >> + >> + if (h) >> + relocation = sec_addr (got) + (h->got.offset & (~(bfd_vma)1)); >> + else >> + relocation = sec_addr (got) >> + + (local_got_offsets[r_symndx] & (~(bfd_vma)1)); >> + >> + tls_type = _bfd_loongarch_elf_tls_type (input_bfd, h, r_symndx); >> + /* Use both TLS_GD and TLS_DESC. */ >> + if ((tls_type & GOT_TLS_GD) && (tls_type & GOT_TLS_GDESC)) >> + relocation += 2 * GOT_ENTRY_SIZE; > For simplicity, I think the ie_off and desc_off calculation should be moved out of the switch. Then you can just reuse the desc/ie_off calculated there. I will modify it in a later version. >> + } >> + >> + if (r_type == R_LARCH_TLS_DESC64_PC_LO20 >> + || r_type == R_LARCH_TLS_DESC64_PC_HI12) >> + RELOCATE_CALC_PC64_HI32 (relocation, pc); >> + >> + break; >> + >> + case R_LARCH_TLS_DESC_LD: >> + case R_LARCH_TLS_DESC_CALL: >> + unresolved_reloc = false; >> + break; >> + >> case R_LARCH_TLS_IE_PC_LO12: >> case R_LARCH_TLS_IE64_PC_LO20: >> case R_LARCH_TLS_IE64_PC_HI12: >> @@ -3523,14 +3643,17 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info, >> unresolved_reloc = false; >> >> if (h) >> - relocation = sec_addr (got) + (h->got.offset & (~(bfd_vma)3)); >> + relocation = sec_addr (got) + (h->got.offset & (~(bfd_vma)1)); >> else >> relocation = sec_addr (got) >> - + (local_got_offsets[r_symndx] & (~(bfd_vma)3)); >> + + (local_got_offsets[r_symndx] & (~(bfd_vma)1)); >> >> tls_type = _bfd_loongarch_elf_tls_type (input_bfd, h, r_symndx); >> - /* Use both TLS_GD and TLS_IE. */ >> - if ((tls_type & GOT_TLS_GD) && (tls_type & GOT_TLS_IE)) >> + /* Use TLS_GD TLS_DESC and TLS_IE. */ >> + if (GOT_TLS_GD_BOTH_P (tls_type) && (tls_type & GOT_TLS_IE)) >> + relocation += 4 * GOT_ENTRY_SIZE; >> + /* Use GOT_TLS_GD_ANY_P (tls_type) and TLS_IE. */ >> + else if (GOT_TLS_GD_ANY_P (tls_type) && (tls_type & GOT_TLS_IE)) >> relocation += 2 * GOT_ENTRY_SIZE; >> >> if (r_type == R_LARCH_TLS_IE64_PC_LO20 >> @@ -4174,7 +4297,8 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd, >> >> if (h->got.offset != MINUS_ONE >> /* TLS got entry have been handled in elf_relocate_section. */ >> - && !(loongarch_elf_hash_entry (h)->tls_type & (GOT_TLS_GD | GOT_TLS_IE)) >> + && !(loongarch_elf_hash_entry (h)->tls_type >> + & (GOT_TLS_GD | GOT_TLS_IE | GOT_TLS_GDESC)) >> /* Have allocated got entry but not allocated rela before. */ >> && !UNDEFWEAK_NO_DYNAMIC_RELOC (info, h)) >> { >> -- >> 2.43.0 >> >>