* [PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final @ 2021-10-01 15:35 Lewis Revill 2021-10-02 2:54 ` Nelson Chu 0 siblings, 1 reply; 4+ messages in thread From: Lewis Revill @ 2021-10-01 15:35 UTC (permalink / raw) To: binutils 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final 2021-10-01 15:35 [PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final Lewis Revill @ 2021-10-02 2:54 ` Nelson Chu 2021-10-05 14:36 ` Nick Clifton 0 siblings, 1 reply; 4+ messages in thread From: Nelson Chu @ 2021-10-02 2:54 UTC (permalink / raw) To: Lewis Revill, Nick Clifton, Alan Modra, Jim Wilson, Palmer Dabbelt, Andrew Waterman Cc: Binutils 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final 2021-10-02 2:54 ` Nelson Chu @ 2021-10-05 14:36 ` Nick Clifton 2021-10-06 4:41 ` Nelson Chu 0 siblings, 1 reply; 4+ messages in thread From: Nick Clifton @ 2021-10-05 14:36 UTC (permalink / raw) To: Nelson Chu, Lewis Revill, Alan Modra, Jim Wilson, Palmer Dabbelt, Andrew Waterman Cc: Binutils Hi Nelson, >> To fix this, this patch adds the delay_region_check flag to >> bfd_link_info, I dislike changing generic code just to fix a problem with one target. If there is a method that only involves changing risc-v specific code then I would prefer that. (This is option 3 in PR 28410, right ?) Cheers Nick ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final 2021-10-05 14:36 ` Nick Clifton @ 2021-10-06 4:41 ` Nelson Chu 0 siblings, 0 replies; 4+ messages in thread From: Nelson Chu @ 2021-10-06 4:41 UTC (permalink / raw) To: Nick Clifton Cc: Lewis Revill, Alan Modra, Jim Wilson, Palmer Dabbelt, Andrew Waterman, Binutils Hi Nick, On Tue, Oct 5, 2021 at 10:36 PM Nick Clifton <nickc@redhat.com> wrote: > > Hi Nelson, > > >> To fix this, this patch adds the delay_region_check flag to > >> bfd_link_info, > > I dislike changing generic code just to fix a problem with one target. > If there is a method that only involves changing risc-v specific code > then I would prefer that. (This is option 3 in PR 28410, right ?) Yes, that's option 3 in PR 28410, it is more complicated, but we don't need to modify any generic code. Thanks for the reply, this allows us to know more clearly that we should try option 3 at first, as possible as we can. Thanks Nelson ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-06 4:41 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-01 15:35 [PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final Lewis Revill 2021-10-02 2:54 ` Nelson Chu 2021-10-05 14:36 ` Nick Clifton 2021-10-06 4:41 ` Nelson Chu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).