From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2e.google.com (mail-oa1-x2e.google.com [IPv6:2001:4860:4864:20::2e]) by sourceware.org (Postfix) with ESMTPS id 43E6B385C418 for ; Mon, 31 Oct 2022 09:58:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 43E6B385C418 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-oa1-x2e.google.com with SMTP id 586e51a60fabf-13ba9a4430cso12886982fac.11 for ; Mon, 31 Oct 2022 02:58:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=KL6TcxIJxwx2XnGCEO8zNh8NT/f5osYqfaN1qvfRtvw=; b=D14yMML7We7exUCJLiKzm6Pq+VdJ8xSznK2IsF0AWvVD5jKAxRgGqnucOHkz7avwE0 hXAhcoUVqN6oUdJKCwRpFYyHXQP801yheklSHW+eI75Jbdcn3Ww34fiMD3YjSVy2lxUV ri74Crsnwq48nDWrr0fSWyyAyZQmpTUjPRC1zTjE56DISWEIw68qd2PgI5sCQjB0I2jh s6zz9euhlt5tQfS+hds0dywviP6EfY8dS6MGOkv3eLRad5thS5by1Ell0755DNaDefWB b6umAWS0V/N8Z/7B6ZGoI8duskhrGx61F3nkA9QHeCxjzGyvWE5AI7GNb6sBr73MFrZE cCHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KL6TcxIJxwx2XnGCEO8zNh8NT/f5osYqfaN1qvfRtvw=; b=fD+f8MPL9YURIJwvTwX2URDxrqLHKoDFVvKOPLD1bQtnm2a57AXWYQz+hLQwc0o+zF msOxxteKiv0XtUaAbE6ai5fDK9HoEKaXEXaLUh41UCs7Q7caJzeWn+qDdwwSyes0Hm5N h//u4mhH0BxWF+ux5ZaJpeMxsDPc6f/iY07RlQLdyCp8HnCqhrQGiqmq8LItsAr7lYBP D75pr0p96HHUvUTKnDZyQwC0Ay1vHEtOocapf6f4/A+Y31gbwfvpHNBF4B2QJWBumeL8 7cEBhIcjjpTH3A7XehzhwqgK/b3qxuPgoCchAdvhi3WpFK4nH1g3yCs5q9U34tjrPgnB rwTA== X-Gm-Message-State: ACrzQf3xLyvCxbK5yHaqPp2z9IOpThqVNFt+Bz7uA4gRcO1VsrAMe/NY +TsQtgdNqWyxCjOstR7ZvVVVHMa70GHHoDMZUhslfBuUwQg= X-Google-Smtp-Source: AMsMyM6ODTO3PWGCaN/I4GWoJIDI/aWhMxHKQoN4G2bcvCYisgDWi4ttqLtlQ34W2JisEXFcbg8l+yp3ZF76Zr+R1rQ= X-Received: by 2002:a05:6870:56a9:b0:13b:d2ae:ed90 with SMTP id p41-20020a05687056a900b0013bd2aeed90mr16732992oao.198.1667210285450; Mon, 31 Oct 2022 02:58:05 -0700 (PDT) MIME-Version: 1.0 References: <20221006114628.304185-1-chigot@adacore.com> In-Reply-To: From: =?UTF-8?Q?Cl=C3=A9ment_Chigot?= Date: Mon, 31 Oct 2022 10:57:54 +0100 Message-ID: Subject: Re: [PATCH] RISC-V: fix linker message when relaxation deletes bytes To: Nelson Chu Cc: binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, Gentle ping On Fri, Oct 14, 2022 at 9:24 AM Cl=C3=A9ment Chigot wr= ote: > > On Fri, Oct 14, 2022 at 7:08 AM Nelson Chu wrote: > > > > Hi, > > > > On Thu, Oct 13, 2022 at 8:06 PM Cl=C3=A9ment Chigot via Binutils > > wrote: > > > > > > Gentle ping > > > > > > Thanks, > > > Cl=C3=A9ment > > > > > > On Thu, Oct 6, 2022 at 1:46 PM Cl=C3=A9ment Chigot wrote: > > > > > > > > The section relaxation can delete some bytes resulting in the symbo= ls' > > > > value being modified. > > > > As the linker messages retrieve a symbol information using the > > > > outsymbols field of abfd, it must be updated as well. > > > > Interesting, I never noticed this. If this is necessary, then other > > targets should also update the output symbols when relaxing (nds32 and > > sh), but it seems like they don't do that as well. > > Yeah, I agree there are probably other targets having a similar issue. > I don't know at what extend outsymbols is being used for. But the fact > that is being filled with "untouched" symbols in object files looks > weird to me. Maybe there is something to look into this direction. > For now, this patch aims just to fix ld-undefined on Risc-V targets. > > > > > bfd/ChangeLog: > > > > > > > > * elfnn-riscv.c (riscv_relax_delete_bytes): Update > > > > abfd->outsymbols. > > > > > > > > ld/ChangeLog: > > > > > > > > * testsuite/ld-riscv-elf/ld-riscv-elf.exp: New test. > > > > * testsuite/ld-riscv-elf/undefined-align.d: New test. > > > > * testsuite/ld-riscv-elf/undefined-align.s: New test. > > > > --- > > > > bfd/elfnn-riscv.c | 23 +++++++++++++++++= ++++ > > > > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + > > > > ld/testsuite/ld-riscv-elf/undefined-align.d | 5 +++++ > > > > ld/testsuite/ld-riscv-elf/undefined-align.s | 10 +++++++++ > > > > 4 files changed, 39 insertions(+) > > > > create mode 100644 ld/testsuite/ld-riscv-elf/undefined-align.d > > > > create mode 100644 ld/testsuite/ld-riscv-elf/undefined-align.s > > > > > > > > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > > > > index 3d2ddf4e651..7a6b66dcd8a 100644 > > > > --- a/bfd/elfnn-riscv.c > > > > +++ b/bfd/elfnn-riscv.c > > > > @@ -4060,6 +4060,7 @@ riscv_relax_delete_bytes (bfd *abfd, > > > > unsigned int sec_shndx =3D _bfd_elf_section_from_bfd_section (ab= fd, sec); > > > > struct bfd_elf_section_data *data =3D elf_section_data (sec); > > > > bfd_byte *contents =3D data->this_hdr.contents; > > > > + asymbol **outsyms =3D bfd_get_outsymbols (abfd); > > > > > > > > /* Actually delete the bytes. */ > > > > sec->size -=3D count; > > > > @@ -4158,6 +4159,28 @@ riscv_relax_delete_bytes (bfd *abfd, > > > > } > > > > } > > > > > > > > + /* As linker messages are getting symbols through outsymbols fie= ld of abfd, > > > > + it must be adjusted too. */ > > > > + if (outsyms =3D=3D NULL) > > > > + { > > > > + if (!bfd_generic_link_read_symbols (abfd)) > > > > + link_info->callbacks->einfo (_("%F%P: %pB: could not read s= ymbols: %E\n"), abfd); > > > > + outsyms =3D bfd_get_outsymbols (abfd); > > > > + } > > > > + > > > > + for (i =3D 0; i < bfd_get_symcount (abfd); i++) > > > > + { > > > > + asymbol *sym =3D outsyms[i]; > > > > + > > > > + if (sym->section !=3D sec) > > > > + continue; > > > > + > > > > + /* If the symbol is in the range of memory we just moved, we > > > > + have to adjust its value. */ > > > > + if (sym->value > addr && sym->value <=3D toaddr) > > > > + sym->value -=3D count; > > > > + } > > > > + > > > > return true; > > > > } > > > > > > > > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsu= ite/ld-riscv-elf/ld-riscv-elf.exp > > > > index df89e0ee68b..86f1d05bc08 100644 > > > > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > > > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > > > @@ -176,6 +176,7 @@ if [istarget "riscv*-*-*"] { > > > > [list "Weak reference 64" "-T weakref.ld -m[riscv_choose_lp= 64_emul]" "" \ > > > > "-march=3Drv64i -mabi=3Dlp64" {weakref64.s} \ > > > > {{objdump -d weakref64.d}} "weakref64"]] > > > > + run_dump_test "undefined-align" > > > > > > > > # The following tests require shared library support. > > > > if ![check_shared_lib_support] { > > > > diff --git a/ld/testsuite/ld-riscv-elf/undefined-align.d b/ld/tests= uite/ld-riscv-elf/undefined-align.d > > > > new file mode 100644 > > > > index 00000000000..c8cbb84ac7c > > > > --- /dev/null > > > > +++ b/ld/testsuite/ld-riscv-elf/undefined-align.d > > > > @@ -0,0 +1,5 @@ > > > > +#name: undefined with alignment > > > > +#source: undefined-align.s > > > > +#as: > > > > +#ld: --relax > > > > +#error: \A[^\n]*\.o: in function `_start':\n\(\.text\+0x0\): undef= ined reference to `foo'\Z > > > > diff --git a/ld/testsuite/ld-riscv-elf/undefined-align.s b/ld/tests= uite/ld-riscv-elf/undefined-align.s > > > > new file mode 100644 > > > > index 00000000000..577557c663a > > > > --- /dev/null > > > > +++ b/ld/testsuite/ld-riscv-elf/undefined-align.s > > > > @@ -0,0 +1,10 @@ > > > > +# Make sure that the linker messages take into account the modific= ation > > > > +# of a symbol's value made during the section relaxation. > > > > +# Here, "_start" will have an offset made by ".align 4" which will= be > > > > +# removed during the relaxation of R_RISCV_ALIGN. > > > > + .text > > > > + .align 4 > > > > + .globl _start > > > > + .type _start, @function > > > > +_start: > > > > + call foo > > > > Tested with undefined-align.s, I get the following error message > > before applying this patch, > > > > % riscv64-unknown-elf-ld tmp.o > > > > riscv64-unknown-elf-ld: tmp.o:(.text+0x0): undefined reference to `foo' > > > > And get the error after applying this patch, > > > > % riscv64-unknown-elf-ld tmp.o > > > > riscv64-unknown-elf-ld: tmp.o: in function `_start': > > > > (.text+0x0): undefined reference to `foo' > > > > It reports more detail, but I cannot see if the removed offset may > > cause the wrong message? > > I'm not sure I understand what you mean. > As in the object file, _start starts at 0xc, you mean that the error > message should be something like: > | riscv64-unknown-elf-ld: tmp.o: in function `_start': > | (.text+0xc): undefined reference to `foo' > ? > > Thanks, > Cl=C3=A9ment