From: Indu Bhagat Date: Thu, 6 Feb 2025 21:38:04 +0000 (-0800) Subject: ld: bfd: sframe: fix incorrect r_offset in RELA entries X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bc5fb8f4c29b55283afee95c26558389840ebca8;p=thirdparty%2Fbinutils-gdb.git ld: bfd: sframe: fix incorrect r_offset in RELA entries PR/32666 Incorrect .rela.sframe when using ld -r Input SFrame sections are merged using _bfd_elf_merge_section_sframe (), which clubs all SFrame FDEs together in one blob and all SFrame FREs in another. This, of course, means the offset of an SFrame FDE in the output section cannot be simply derived from the output_offset of the sections. Fix this by providing _bfd_elf_sframe_section_offset () which returns the new offset of the SFrame FDE in the merged SFrame section. Unlike EH_Frame sections, which also use the _bfd_elf_section_offset (), to update the r_offset, SFrame sections have distinct merging semantics. In case of SFrame, the SFrame FDE will not simply sit at location "sec->output_offset + offset of SFrame FDE in sec". Recall that information layout in an SFrame section is as follows: SFrame Header SFrame FDE 1 SFrame FDE 2 ... SFrame FDEn SFrame FREs (Frame Row Entries) Note how the SFrame FDEs and SFrame FREs are clubber together in groups of their own. Next, also note how the elf_link_input_bfd () does a: irela->r_offset += o->output_offset; This, however, needs to be avoided for SFrame sections because the placement of all FDEs is at the beginning of the section. So, rather than conditionalizing this as follows: if (o->sec_info_type != SEC_INFO_TYPE_SFRAME) irela->r_offset += o->output_offset; the implementation in _bfd_elf_sframe_section_offset () does a reverse adjustment, so that the generic parts of the linking process in elf_link_input_bfd () are not made to do SFrame specific adjustments. Add a new enum to track the current state of the SFrame input section during the linking process (SFRAME_SEC_DECODED, SFRAME_SEC_MERGED) for each input SFrame section. This is then used to assert an assumption that _bfd_elf_sframe_section_offset () is being used on an input SFrame sections which have not been merged (via _bfd_elf_merge_section_sframe ()) yet. bfd/ * elf-bfd.h: New declaration. * elf-sframe.c (_bfd_elf_sframe_section_offset): New definition. * elf.c (_bfd_elf_section_offset): Adjust offset if SFrame section. ld/testsuite/ * ld-x86-64/x86-64.exp: New test. * ld-x86-64/sframe-reloc-1.d: New test. --- Notes: This patch was previously reviewed. https://inbox.sourceware.org/binutils/d914ea1f-6c11-4ef5-ac15-404fd7afd26d@suse.com/ There was a comment on moving this if (o->sec_info_type != SEC_INFO_TYPE_SFRAME) irela->r_offset += o->output_offset; to inside the _bfd_elf_sframe_section_offset () or keep it in elf_link_input_bfd () with some code comments. Although, I think keeping it deliberatly out of _bfd_elf_sframe_section_offset () was clearer as that helped keep the contract of _bfd_elf_section_offset () symmetrical across section types. That said, taking the overall opinions shared in the previous review, I have moved the stub inside _bfd_elf_sframe_section_offset (). But if opinions have changed, I am happy to bring the conditional back to elf_link_input_bfd (). [No changes in V4] [No changes in V3] [No changes in V2] --- diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h index 5903d857faa..f62570919d5 100644 --- a/bfd/elf-bfd.h +++ b/bfd/elf-bfd.h @@ -503,12 +503,21 @@ struct sframe_func_bfdinfo unsigned int func_reloc_index; }; +/* Link state information of the SFrame section. */ +enum sframe_sec_state +{ + SFRAME_SEC_DECODED = 1, + SFRAME_SEC_MERGED, +}; + /* SFrame decoder info. Contains all information for a decoded .sframe section. */ struct sframe_dec_info { /* Decoder context. */ struct sframe_decoder_ctx *sfd_ctx; + /* SFrame section state as it progresses through the link process. */ + enum sframe_sec_state sfd_state; /* Number of function descriptor entries in this .sframe. */ unsigned int sfd_fde_count; /* Additional information for linking. */ @@ -2540,6 +2549,8 @@ extern bool _bfd_elf_discard_section_sframe (asection *, bool (*) (bfd_vma, void *), struct elf_reloc_cookie *); extern bool _bfd_elf_merge_section_sframe (bfd *, struct bfd_link_info *, asection *, bfd_byte *); +extern bfd_vma _bfd_elf_sframe_section_offset + (bfd *, struct bfd_link_info *, asection *, bfd_vma); extern bool _bfd_elf_write_section_sframe (bfd *, struct bfd_link_info *); extern bool _bfd_elf_set_section_sframe (bfd *, struct bfd_link_info *); diff --git a/bfd/elf-sframe.c b/bfd/elf-sframe.c index 7a79ff78433..0a2c4695c21 100644 --- a/bfd/elf-sframe.c +++ b/bfd/elf-sframe.c @@ -212,10 +212,11 @@ _bfd_elf_parse_sframe (bfd *abfd, /* Decode the buffer and keep decoded contents for later use. Relocations are performed later, but are such that the section's size is unaffected. */ - sfd_info = bfd_alloc (abfd, sizeof (*sfd_info)); + sfd_info = bfd_zalloc (abfd, sizeof (*sfd_info)); sf_size = sec->size; sfd_info->sfd_ctx = sframe_decode ((const char*)sfbuf, sf_size, &decerr); + sfd_info->sfd_state = SFRAME_SEC_DECODED; sfd_ctx = sfd_info->sfd_ctx; if (!sfd_ctx) /* Free'ing up any memory held by decoder context is done by @@ -540,12 +541,98 @@ _bfd_elf_merge_section_sframe (bfd *abfd, } } } + sfd_info->sfd_state = SFRAME_SEC_MERGED; /* Free the SFrame decoder context. */ sframe_decoder_free (&sfd_ctx); return true; } +/* Adjust an address in the .sframe section. Given OFFSET within + SEC, this returns the new offset in the merged .sframe section, + or -1 if the address refers to an FDE which has been removed. + + PS: This function assumes that _bfd_elf_merge_section_sframe has + not been called on the input section SEC yet. Note how it uses + sframe_encoder_get_num_fidx () to figure out the offset of FDE + in the output section. */ + +bfd_vma +_bfd_elf_sframe_section_offset (bfd *output_bfd ATTRIBUTE_UNUSED, + struct bfd_link_info *info, + asection *sec, + bfd_vma offset) +{ + struct sframe_dec_info *sfd_info; + struct sframe_enc_info *sfe_info; + sframe_decoder_ctx *sfd_ctx; + sframe_encoder_ctx *sfe_ctx; + struct elf_link_hash_table *htab; + + unsigned int sec_fde_idx, out_num_fdes; + unsigned int sfd_num_fdes, sfe_num_fdes; + uint32_t sfd_fde_offset; + bfd_vma new_offset; + + if (sec->sec_info_type != SEC_INFO_TYPE_SFRAME) + return offset; + + sfd_info = (struct sframe_dec_info *) elf_section_data (sec)->sec_info; + sfd_ctx = sfd_info->sfd_ctx; + sfd_num_fdes = sframe_decoder_get_num_fidx (sfd_ctx); + + BFD_ASSERT (sfd_info->sfd_state == SFRAME_SEC_DECODED); + + htab = elf_hash_table (info); + sfe_info = &(htab->sfe_info); + sfe_ctx = sfe_info->sfe_ctx; + sfe_num_fdes = sframe_encoder_get_num_fidx (sfe_ctx); + + /* The index of this FDE in the output section depends on number of deleted + functions (between index 0 and sec_fde_idx), if any. */ + out_num_fdes = 0; + sec_fde_idx = 0; + for (unsigned int i = 0; i < sfd_num_fdes; i++) + { + sfd_fde_offset = sframe_decoder_get_offsetof_fde_start_addr (sfd_ctx, + i, NULL); + if (!sframe_decoder_func_deleted_p (sfd_info, i)) + out_num_fdes++; + + if (sfd_fde_offset == offset) + { + /* Found the index of the FDE (at OFFSET) in the input section. */ + sec_fde_idx = i; + break; + } + } + + if (sframe_decoder_func_deleted_p (sfd_info, sec_fde_idx)) + return (bfd_vma) -1; + + /* The number of FDEs in the output SFrame section. Note that the output + index of the FDE of interest will be (out_num_fdes - 1). */ + out_num_fdes += sfe_num_fdes; + + new_offset = sframe_decoder_get_offsetof_fde_start_addr (sfd_ctx, + out_num_fdes - 1, + NULL); + /* Recall that SFrame section merging has distinct requirements: All SFrame + FDEs from input sections are clubbed together in the beginning of the + output section. So, at this point in the current function, the new_offset + is the correct offset in the merged output SFrame section. Note, however, + that the default mechanism in the _elf_link_input_bfd will do the + following adjustment: + irela->r_offset += o->output_offset; + for all section types. However, such an adjustment in the RELA offset is + _not_ needed for SFrame sections. Perform the reverse adjustment here so + that the default mechanism does not need additional SFrame specific + checks. */ + new_offset -= sec->output_offset; + + return new_offset; +} + /* Write out the .sframe section. This must be called after _bfd_elf_merge_section_sframe has been called on all input .sframe sections. */ diff --git a/bfd/elf.c b/bfd/elf.c index ee894eb05f2..cf0f1a128ba 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -13546,6 +13546,9 @@ _bfd_elf_section_offset (bfd *abfd, case SEC_INFO_TYPE_EH_FRAME: return _bfd_elf_eh_frame_section_offset (abfd, info, sec, offset); + case SEC_INFO_TYPE_SFRAME: + return _bfd_elf_sframe_section_offset (abfd, info, sec, offset); + default: if ((sec->flags & SEC_ELF_REVERSE_COPY) != 0) { diff --git a/ld/testsuite/ld-x86-64/sframe-reloc-1.d b/ld/testsuite/ld-x86-64/sframe-reloc-1.d new file mode 100644 index 00000000000..1deb9a2576a --- /dev/null +++ b/ld/testsuite/ld-x86-64/sframe-reloc-1.d @@ -0,0 +1,35 @@ +#as: --gsframe +#source: sframe-foo.s +#source: sframe-bar.s +#objdump: --sframe=.sframe +#ld: -r --no-rosegment +#name: SFrame simple link - relocatable + +.*: +file format .* + +Contents of the SFrame section .sframe: + Header : + + Version: SFRAME_VERSION_2 + Flags: SFRAME_F_FDE_SORTED, + SFRAME_F_FDE_FUNC_START_ADDR_PCREL + CFA fixed RA offset: \-8 + Num FDEs: 2 + Num FREs: 8 + + Function Index : + + + func idx \[0\]: pc = 0x0, size = 53 bytes + STARTPC +CFA +FP +RA + + 0+0000 +sp\+8 +u +f + + 0+0001 +sp\+16 +c-16 +f + + 0+0004 +fp\+16 +c-16 +f + + 0+0034 +sp\+8 +c-16 +f + + + func idx \[1\]: pc = 0x35, size = 37 bytes + STARTPC +CFA +FP +RA + + 0+0035 +sp\+8 +u +f + + 0+0036 +sp\+16 +c-16 +f + + 0+0039 +fp\+16 +c-16 +f + + 0+0059 +sp\+8 +c-16 +f + diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp index 3bb88ff1c49..a682b1374fa 100644 --- a/ld/testsuite/ld-x86-64/x86-64.exp +++ b/ld/testsuite/ld-x86-64/x86-64.exp @@ -567,6 +567,7 @@ run_dump_test "pr32809" if { ![skip_sframe_tests] } { run_dump_test "sframe-simple-1" + run_dump_test "sframe-reloc-1" run_dump_test "sframe-plt-1" run_dump_test "sframe-ibt-plt-1" run_dump_test "sframe-pltgot-1"