From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vs1-xe36.google.com (mail-vs1-xe36.google.com [IPv6:2607:f8b0:4864:20::e36]) by sourceware.org (Postfix) with ESMTPS id 74AC23858417 for ; Sat, 2 Oct 2021 02:54:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 74AC23858417 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sifive.com Received: by mail-vs1-xe36.google.com with SMTP id f18so13546727vsp.2 for ; Fri, 01 Oct 2021 19:54:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Y+jZEyzQWfWZkl5q/5ydD1jkB0gA8Y62Q7WSkQYCWUA=; b=Fm3FXZG73Mk1virReQXAOcHxsals1qwpwLoQyPf0xy0JcpFzC4mSUHlE1R7abqKNia C387YRvVzGA6pzWOKH8ZJfzMADOqOLPGQ/zKdIZxSPqEsKwgV5sqxcfyKA/GBYXCDVv5 p/C9JJ3mSJuKsil+yGasVMlzqwzo94UXlFLLMTqOCdlEUuiQK5qpMT9FQhuCKW6tvfqY nEOU6C/rCdEiSPFUKYdFmqfvnjVCpjhdyPGe/fNka9YGbMdcjSKr4r6McNn4rKBxJR+l FEqaFDPJj8G7Z9UYdnVPddrWj+BEPz6dGJ9YtUPMXMLtiFSJOycd2JkfdVvMveyXXOeT RQAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Y+jZEyzQWfWZkl5q/5ydD1jkB0gA8Y62Q7WSkQYCWUA=; b=f2U/nXKBGhuLTwFEtRo5KIWQ6PXVSXsx7SqwksMvbi8Zh0GXXn8uXMWC6t0Rwcdacw iN3k/Q1Z7LCGvAFcgpRwRzG9JcmeBgV/jzOCJHK/jRAjKy8oi6n00/bt7ce44qrnT58l bgZOpQhifQUhI1dIk2yWG6a0XrpMuDafeK+5ptNRvUziXcFuaSQEOyj/6gtq20lW20s/ Myq/GsRwQKRwkZdBxRngOl6nTKZUDg7cqkCoMDIrOX4IAKp1bx4VUrs92KKC/JC3T1U0 epftDIkV/wRubKsssUc6dBfwOzT6WHxmVQijYLSV6/i2JrIjIpE9/sBtkjSTpAMECWaC FuEA== X-Gm-Message-State: AOAM533aYqAnmKvlWKAVrgthdBsjTmqQFIX89iyozP/CPzr8oujU2iE1 qGVKLefPR32sgyhiFfgHrEsorhUlIbBmzNIKq+xAnQ== X-Google-Smtp-Source: ABdhPJwJxUeP3wNQ4UVCrH0bSvGSYd+ipJ/uIb5BK5pVKK3h2safjzTC8NAdSRIWy3+XJrOA+yukJDEGhujBiPrFYUk= X-Received: by 2002:a67:2c57:: with SMTP id s84mr7087878vss.3.1633143276369; Fri, 01 Oct 2021 19:54:36 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nelson Chu Date: Sat, 2 Oct 2021 10:54:27 +0800 Message-ID: Subject: Re: [PATCH] bfd/RISC-V: Prevent region check failures when relaxation is not final To: Lewis Revill , Nick Clifton , Alan Modra , Jim Wilson , Palmer Dabbelt , Andrew Waterman Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_MANYTO, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 02 Oct 2021 02:54:39 -0000 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 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 > + > + * elfnn-riscv.c (_bfd_riscv_relax_section): Set flag to delay > + region check when restarting relaxation. > + > 2021-09-27 Nick Alcock > > * 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 > + > + * ldlang.c (lang_relax_sections): Account for delay_region_check > + during final sizing of sections. > + > 2021-09-30 Dimitar Dimitrov > > * 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