From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x30.google.com (mail-oa1-x30.google.com [IPv6:2001:4860:4864:20::30]) by sourceware.org (Postfix) with ESMTPS id 3595F3852C51 for ; Fri, 18 Nov 2022 01:43:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3595F3852C51 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-oa1-x30.google.com with SMTP id 586e51a60fabf-142612a5454so3748641fac.2 for ; Thu, 17 Nov 2022 17:43:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; 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=SvphLG90wDX+AqEQwwW3tRhbt8DA22lUAO+WgRgHvY8=; b=bjzYDd7x1phFNTeJdM+D0SwC/nc84QzdodvMm++ec1C5tt7ExwkWGiQ/8Du15qzPdr ROrR8mpS82NNsKu+g0kgBwTTynA7AsUTYa3B9mNa/7RvdPUnQnEuoKL6rFXqFZwAsjJC dzmXK4EU+o6KMnvV8L5GGbg8Tv+Dvsz907Ff4bHht+yKO1zZCJ3aNu0So5Tj03DAV3KF nQR8r7k/vOYNe5hRUBN+BuCvSLrDta9pBQUteiPkdEOzWTaZO0PXXitqVaeDbsAIT+QV kk+8MasP1bOxm5Xky70mZp9BoXcxAkB2vnqd0UbiZs2MFKCOgDJoTxXMPMagSuMryGBb Mdeg== 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=SvphLG90wDX+AqEQwwW3tRhbt8DA22lUAO+WgRgHvY8=; b=sFzh0mHISf8irAvXxRGSuZ+CakUW+J0L3mMZfhRxnIDno8hquXHnI/7mU80A7wPKJO yxmBy73tU+WfnooyGaPcDrURuR6cTkNZvjECSoT3lS6HWkaisEQWhI1YZ+0CKRL1GS0G bLTdUCPyTHM3iHR277f64ZdWcK8cdnX2K6r7CLgkRsYbsIE53zxi8SSGUJYgJ3d/8jn7 sDoLeKt8qjUn3D+i3z+4t5dp/cdrbIwcD+ntfqMhHgypxrIBT3A2+/urcQopcFMkRrrl UZ3hO1pdjNrnBEfL0T6PBbG8z5hkbYyayr0E4BonfiKCU2YwXvyQPgCkAOXr3kbqJXrg uuow== X-Gm-Message-State: ANoB5pnd9mZctHy6Fl+w5ESDmzxXYmSYLett7vjOW2lDrUSh3R2alrYH IkjzP2qXcT/M4ivU85CZ762mdpYlnUnArpVxJ7LJew== X-Google-Smtp-Source: AA0mqf4PLUrjNx+eTzCoqGywPEu4bYK0Q0fBgSakBsK9nW4BgsFluTZH6Hj5ZVQCqjTwV6cA0MNyYJsklacx9eJGxqU= X-Received: by 2002:a05:6870:788f:b0:13b:8ec6:be96 with SMTP id hc15-20020a056870788f00b0013b8ec6be96mr2797270oab.107.1668735838406; Thu, 17 Nov 2022 17:43:58 -0800 (PST) MIME-Version: 1.0 References: <20221006114628.304185-1-chigot@adacore.com> In-Reply-To: From: Nelson Chu Date: Fri, 18 Nov 2022 09:43:46 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: fix linker message when relaxation deletes bytes To: =?UTF-8?Q?Cl=C3=A9ment_Chigot?= Cc: binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,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: On Fri, Oct 14, 2022 at 3:24 PM 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' > ? The error message doesn't seem wrong without your patch, just don't report "in function `_start':". The comment in your testcase said the offset should be removed after handling alignment, but I get the same zero offset with or without your patch. It would be great if you can provide other cases to show how the offset reported by error message will be wrong. Thanks Nelson > Thanks, > Cl=C3=A9ment