[PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final
Nelson Chu
nelson.chu@sifive.com
Sat Oct 2 02:54:27 GMT 2021
Hi Lewis,
Thanks for reporting this, and have a proposed solution. I cannot
find the bug report in the bugzilla, so I create one:
https://sourceware.org/bugzilla/show_bug.cgi?id=28410. I have some
comments, but not sure how to do, maybe we can discuss the details
there.
Hi Nick, Hi Alan, Hi Jim, Hi Palmer, Hi Andrew,
I need your suggestions and helps if you are free recently.
Thank you very much
Nelson
On Fri, Oct 1, 2021 at 11:35 PM Lewis Revill <lewis.revill@embecosm.com> wrote:
>
> In some circumstances for RISC-V, such as using a .align directive in a
> section with a very small output memory region, we currently fail to
> link. The following is an analysis of the problem:
>
> Since the restart_relax flag was implemented (commit
> ebdcad3fddf6ec21f6d4dcc702379a12718cf0c4), we have the capability to
> take another complete round trip through the lang_relax_sections process
> rather than simply using the `again` flag. Doing this, we delay the
> relaxation of alignment directives until no more round trips are
> required.
>
> However, the final part of the lang_relax_sections process also involves
> a sanity check that addresses fit within regions (os_region_check). In
> the case where alignment directives - implemented as nop sleds which can
> be reduced as needed - are not yet relaxed, these checks may fail
> prematurely.
>
> To fix this, this patch adds the delay_region_check flag to
> bfd_link_info, which allows lang_relax_sections to prevent the sanity
> check from occurring if set to true. Then, in _bfd_riscv_relax_section,
> instead of simply skipping the final phase of relaxation when we are
> restarting relaxation, we also set this flag to true, preventing
> premature errors.
>
> bfd/
>
> * elfnn-riscv.c (_bfd_riscv_relax_section): Set flag to delay
> region check when restarting relaxation.
>
> ld/
>
> * ldlang.c (lang_relax_sections): Account for delay_region_check
> during final sizing of sections.
> ---
> bfd/ChangeLog | 5 +++++
> bfd/elfnn-riscv.c | 13 +++++++++++--
> include/bfdlink.h | 6 ++++++
> ld/ChangeLog | 5 +++++
> ld/ldlang.c | 4 +++-
> ld/testsuite/ld-riscv-elf/align-small-region.d | 12 ++++++++++++
> ld/testsuite/ld-riscv-elf/align-small-region.ld | 12 ++++++++++++
> ld/testsuite/ld-riscv-elf/align-small-region.s | 7 +++++++
> 8 files changed, 61 insertions(+), 3 deletions(-)
> create mode 100644 ld/testsuite/ld-riscv-elf/align-small-region.d
> create mode 100644 ld/testsuite/ld-riscv-elf/align-small-region.ld
> create mode 100644 ld/testsuite/ld-riscv-elf/align-small-region.s
>
> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 2a08ff7cfb4..892eb7a322b 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,8 @@
> +2021-10-01 Lewis Revill <lewis.revill@embecosm.com>
> +
> + * elfnn-riscv.c (_bfd_riscv_relax_section): Set flag to delay
> + region check when restarting relaxation.
> +
> 2021-09-27 Nick Alcock <nick.alcock@oracle.com>
>
> * configure: Regenerate.
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 2e8df72fa2a..e3f247a1c39 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -4711,13 +4711,22 @@ _bfd_riscv_relax_section (bfd *abfd, asection *sec,
> || sec->reloc_count == 0
> || (info->disable_target_specific_optimizations
> && info->relax_pass < 2)
> - || (htab->restart_relax
> - && info->relax_pass == 3)
> /* The exp_seg_relro_adjust is enum phase_enum (0x4),
> and defined in ld/ldexp.h. */
> || *(htab->data_segment_phase) == 4)
> return true;
>
> + if (htab->restart_relax && info->relax_pass == 3)
> + {
> + /* We are restarting the entire process of mapping segments
> (including
> + relaxation passes) again. This process would usually include a
> sanity
> + check that addresses lie within their region, however in this
> case we
> + are not yet done with relaxation and are delaying relaxation of
> + alignment directives. As such we should also delay this check. */
> + info->delay_region_check = true;
> + return true;
> + }
> +
> riscv_init_pcgp_relocs (&pcgp_relocs);
>
> /* Read this BFD's relocs if we haven't done so already. */
> diff --git a/include/bfdlink.h b/include/bfdlink.h
> index 566529ee644..c392d3eb341 100644
> --- a/include/bfdlink.h
> +++ b/include/bfdlink.h
> @@ -625,6 +625,12 @@ struct bfd_link_info
> relaxation returning true in *AGAIN. */
> int relax_trip;
>
> + /* Whether to check that addresses fit within regions at the end of
> + the lang_relax_sections pass. Set to TRUE by bfd_relax_section if
> + section sizes are not necessarily final at the end of this
> + particular pass through lang_relax_sections. */
> + bool delay_region_check;
> +
> /* > 0 to treat protected data defined in the shared library as
> reference external. 0 to treat it as internal. -1 to let
> backend to decide. */
> diff --git a/ld/ChangeLog b/ld/ChangeLog
> index 808191bd14e..f8cf6b2de3a 100644
> --- a/ld/ChangeLog
> +++ b/ld/ChangeLog
> @@ -1,3 +1,8 @@
> +2021-10-01 Lewis Revill <lewis.revill@embecosm.com>
> +
> + * ldlang.c (lang_relax_sections): Account for delay_region_check
> + during final sizing of sections.
> +
> 2021-09-30 Dimitar Dimitrov <dimitar@dinux.eu>
>
> * scripttempl/pru.sc (.resource_table): Align the output
> diff --git a/ld/ldlang.c b/ld/ldlang.c
> index bc3f8b76d35..05ad2ea7b4b 100644
> --- a/ld/ldlang.c
> +++ b/ld/ldlang.c
> @@ -7681,6 +7681,8 @@ lang_find_relro_sections (void)
> void
> lang_relax_sections (bool need_layout)
> {
> + link_info.delay_region_check = false;
> +
> if (RELAXATION_ENABLED)
> {
> /* We may need more than one relaxation pass. */
> @@ -7728,7 +7730,7 @@ lang_relax_sections (bool need_layout)
> /* Final extra sizing to report errors. */
> lang_do_assignments (lang_assigning_phase_enum);
> lang_reset_memory_regions ();
> - lang_size_sections (NULL, true);
> + lang_size_sections (NULL, !link_info.delay_region_check);
> }
> }
>
> diff --git a/ld/testsuite/ld-riscv-elf/align-small-region.d
> b/ld/testsuite/ld-riscv-elf/align-small-region.d
> new file mode 100644
> index 00000000000..8b9a0ab47eb
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/align-small-region.d
> @@ -0,0 +1,12 @@
> +#source: align-small-region.d
> +#as: -march=rv32i
> +#ld: -melf32lriscv --relax -Talign-small-region.ld --defsym=_start=0x100
> +#objdump: -d
> +
> +.*: file format .*
> +
> +Disassembly of section \.entry:
> +
> +00000000 <_reset>:
> + 0: 6f 00 00 10 j 0x100 <_reset+0x100>
> +#pass
> diff --git a/ld/testsuite/ld-riscv-elf/align-small-region.ld
> b/ld/testsuite/ld-riscv-elf/align-small-region.ld
> new file mode 100644
> index 00000000000..6a3e6638b26
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/align-small-region.ld
> @@ -0,0 +1,12 @@
> +MEMORY
> +{
> + reset : ORIGIN = 0x0, LENGTH = 32
> +}
> +
> +SECTIONS
> +{
> + .entry :
> + {
> + KEEP (*(.entry))
> + } > reset
> +}
> diff --git a/ld/testsuite/ld-riscv-elf/align-small-region.s
> b/ld/testsuite/ld-riscv-elf/align-small-region.s
> new file mode 100644
> index 00000000000..ace81d640ed
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/align-small-region.s
> @@ -0,0 +1,7 @@
> + .section .entry, "xa"
> + .align 5
> + .globl _reset
> + .type _reset, @function
> +_reset:
> + tail _start
> + .size _reset, . - _reset
> --
> 2.25.1
More information about the Binutils
mailing list