Re: [PATCH] arch: fix broken BuildID for arm64 and riscv

From: Masahiro Yamada <masahiroy@kernel.org>
To: Conor Dooley <conor@kernel.org>
Cc: linux-arch@vger.kernel.org, linux-kbuild@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>,
	Thorsten Leemhuis <regressions@leemhuis.info>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, Dennis Gilmore <dennis@ausil.us>,
	Albert Ou <aou@eecs.berkeley.edu>, Arnd Bergmann <arnd@arndb.de>,
	Jisheng Zhang <jszhang@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>
Subject: Re: [PATCH] arch: fix broken BuildID for arm64 and riscv
Date: Tue, 27 Dec 2022 04:06:35 +0900	[thread overview]
Message-ID: <CAK7LNAT39aZEw=0209ovYZ2kxtOaA2a51=XD9=LqYHjkTOEK4g@mail.gmail.com> (raw)
In-Reply-To: <Y6hptEk8FmISixLS@spud>

On Mon, Dec 26, 2022 at 12:18 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Sun, Dec 25, 2022 at 04:27:51AM +0900, Masahiro Yamada wrote:
> > Dennis Gilmore reports that the BuildID is missing in the arm64 vmlinux
> > since commit 994b7ac1697b ("arm64: remove special treatment for the
> > link order of head.o").
> >
> > The issue is that the type of .notes section, which contains the BuildID,
> > changed from NOTES to PROGBITS.
> >
> > Ard Biesheuvel figured out that whichever object gets linked first gets
> > to decide the type of a section, and the PROGBITS type is the result of
> > the compiler emitting .note.GNU-stack as PROGBITS rather than NOTE.
> >
> > While Ard provided a fix for arm64, I want to fix this globally because
> > the same issue is happening on riscv since commit 2348e6bf4421 ("riscv:
> > remove special treatment for the link order of head.o"). This problem
> > will happen in general for other architectures if they start to drop
> > unneeded entries from scripts/head-object-list.txt.
> >
> > Discard .note.GNU-stack in include/asm-generic/vmlinux.lds.h.
> >
> > riscv needs to change its linker script so that DISCARDS comes before
> > the .notes section.
>
> Hey Mashiro,
>
> No idea why I decided to look at patchwork today, but this seems to
> break the build on RISC-V, there's a whole load of the following in the
> output:
> `.LPFE4' referenced in section `__patchable_function_entries' of kernel/trace/trace_selftest_dynamic.o: defined in discarded section `.text.exit' of kernel/trace/trace_selftest_dynamic.o
>
> I assume that's what's doing it, but given the day that's in it - I
> haven't looked into this any further, nor gone and fished the logs out of
> the builder.

arch/riscv/kernel/vmlinux.lds.S clearly says:
/* we have to discard exit text and such at runtime, not link time */

riscv already relies on the linker not discarding EXIT_{TEXT,DATA}
so riscv should define RUNTIME_DISCARD_EXIT like x86, arm64.

Anyway, I came up with a simpler patch, so I do not need to
touch around arch linker scripts.

I sent v2.
https://lore.kernel.org/lkml/20221226184537.744960-1-masahiroy@kernel.org/T/#u

Thanks for the report.

>
> Thanks,
> Conor.
>
> >
> > Link: https://lore.kernel.org/lkml/CAABkxwuQoz1CTbyb57n0ZX65eSYiTonFCU8-LCQc=74D=xE=rA@mail.gmail.com/
> > Fixes: 994b7ac1697b ("arm64: remove special treatment for the link order of head.o")
> > Fixes: 2348e6bf4421 ("riscv: remove special treatment for the link order of head.o")
> > Reported-by: Dennis Gilmore <dennis@ausil.us>
> > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  arch/riscv/kernel/vmlinux.lds.S   | 4 ++--
> >  include/asm-generic/vmlinux.lds.h | 1 +
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index 4e6c88aa4d87..1865a258e560 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -31,6 +31,8 @@ PECOFF_FILE_ALIGNMENT = 0x200;
> >
> >  SECTIONS
> >  {
> > +     DISCARDS
> > +
> >       /* Beginning of code and text segment */
> >       . = LOAD_OFFSET;
> >       _start = .;
> > @@ -141,7 +143,5 @@ SECTIONS
> >       STABS_DEBUG
> >       DWARF_DEBUG
> >       ELF_DETAILS
> > -
> > -     DISCARDS
> >  }
> >  #endif /* CONFIG_XIP_KERNEL */
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index a94219e9916f..2993b790fe98 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -1007,6 +1007,7 @@
> >       *(.modinfo)                                                     \
> >       /* ld.bfd warns about .gnu.version* even when not emitted */    \
> >       *(.gnu.version*)                                                \
> > +     *(.note.GNU-stack)      /* emitted as PROGBITS */
> >
> >  #define DISCARDS                                                     \
> >       /DISCARD/ : {                                                   \
> > --
> > 2.34.1
> >

-- 
Best Regards
Masahiro Yamada