[Bug ld/28410] Prevent region check failures when relaxation is not final

[nelsonc1225 at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28410

Nelson Chu <nelsonc1225 at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Nelson Chu <nelsonc1225 at sourceware dot org> ---
I forgot to add the "pr 28410" keywords in the patch comments, so added here,
and marked as resolved and fixed.

commit 9abcdc10b21ed9f98e218ead7ac9d91612f9ed0f
Refs: users/ARM/embedded-gdb-master-2018q4-11544-g9abcdc1
Author:     Lewis Revill <lewis.revill@embecosm.com>
AuthorDate: Thu Oct 21 11:15:47 2021 +0800
Commit:     Nelson Chu <nelson.chu@sifive.com>
CommitDate: Fri Oct 22 16:44:37 2021 +0800

    RISC-V: Don't separate pcgp relaxation to another relax pass.

    Commit abd20cb637008da9d32018b4b03973e119388a0a and
    ebdcad3fddf6ec21f6d4dcc702379a12718cf0c4 introduced additional
    complexity into the paths run by the RISC-V relaxation pass in order to
    resolve the issue of accurately keeping track of pcrel_hi and pcrel_lo
    pairs. The first commit split up relaxation of these relocs into a pass
    which occurred after other relaxations in order to prevent the situation
    where bytes were deleted in between a pcrel_lo/pcrel_hi pair, inhibiting
    our ability to find the corresponding pcrel_hi relocation from the
    address attached to the pcrel_lo.

    Since the relaxation was split into two passes the 'again' parameter
    could not be used to perform the entire relaxation process again and so
    the second commit added a way to restart ldelf_map_segments, thus
    starting the whole process again.

    Unfortunately this process could not account for the fact that we were
    not finished with the relaxation process so in some cases - such as the
    case where code would not fit in a memory region before the
    R_RISCV_ALIGN relocation was relaxed - sanity checks in generic code
    would fail.

    This patch fixes all three of these concerns by reverting back to a
    system of having only one target relax pass but updating entries in the
    table of pcrel_hi/pcrel_lo relocs every time any bytes are deleted. Thus
    we can keep track of the pairs accurately, and we can use the 'again'
    parameter to restart the entire target relax pass, behaving in the way
    that generic code expects. Unfortunately we must still have an
    additional pass to delay deleting AUIPC bytes to avoid ambiguity between
    pcrel_hi relocs stored in the table after deletion. This pass can only
    be run once so we may potentially miss out on relaxation opportunities
    but this is likely to be rare.

    https://sourceware.org/bugzilla/show_bug.cgi?id=28410

    bfd/
        * elfnn-riscv.c (riscv_elf_link_hash_table): Removed restart_relax.
        (riscv_elf_link_hash_table_create): Updated.
        (riscv_relax_delete_bytes): Moved after the riscv_update_pcgp_relocs.
        Update the pcgp_relocs table whenever bytes are deleted.
        (riscv_update_pcgp_relocs): Add function to update the section
        offset of pcrel_hi and pcrel_lo, and also update the symbol value
        of pcrel_hi.
        (_bfd_riscv_relax_call): Need to update the pcgp_relocs table
        when deleting codes.
        (_bfd_riscv_relax_lui): Likewise.
        (_bfd_riscv_relax_tls_le): Likewise.
        (_bfd_riscv_relax_align): Once we've handled an R_RISCV_ALIGN,
        we can't relax anything else, so set the sec->sec_flg0 to true.
        Besides, we don't need to update the pcgp_relocs table at this
        stage, so just pass NULL pointer as the pcgp_relocs table for
        riscv_relax_delete_bytes.
        (_bfd_riscv_relax_section): Use only one pass for all target
        relaxations.
        (_bfd_riscv_relax_delete): Likewise, we don't need to update
        the pcgp_relocs table at this stage, and don't need to set
        the `again' since restart_relax mechanism is abandoned.
        (bfd_elfNN_riscv_restart_relax_sections): Removed.
        (_bfd_riscv_relax_section): Updated.
        * elfxx-riscv.h (bfd_elf32_riscv_restart_relax_sections): Removed.
        (bfd_elf64_riscv_restart_relax_sections): Likewise.
    ld/
        * emultempl/riscvelf.em: Revert restart_relax changes and set
        relax_pass to 3.
        * testsuite/ld-riscv-elf/align-small-region.d: New testcase.
        * testsuite/ld-riscv-elf/align-small-region.ld: Likewise.
        * testsuite/ld-riscv-elf/align-small-region.s: Likewise.
        * testsuite/ld-riscv-elf/restart-relax.d: Removed sine the
        restart_relax mechanism is abandoned.
        * testsuite/ld-riscv-elf/restart-relax.s: Likewise.
        * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.

-- 
You are receiving this mail because:
You are on the CC list for the bug.