linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arch: fix broken BuildID for arm64 and riscv
@ 2022-12-24 19:27 Masahiro Yamada
  2022-12-25 15:18 ` Conor Dooley
  0 siblings, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2022-12-24 19:27 UTC (permalink / raw)
  To: linux-arch, linux-kbuild
  Cc: linux-kernel, Ard Biesheuvel, Thorsten Leemhuis, Catalin Marinas,
	Will Deacon, linux-arm-kernel, linux-riscv, Masahiro Yamada,
	Dennis Gilmore, Albert Ou, Arnd Bergmann, Jisheng Zhang,
	Nicolas Schier, Palmer Dabbelt, Paul Walmsley

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.

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] arch: fix broken BuildID for arm64 and riscv
  2022-12-24 19:27 [PATCH] arch: fix broken BuildID for arm64 and riscv Masahiro Yamada
@ 2022-12-25 15:18 ` Conor Dooley
  2022-12-26 19:06   ` Masahiro Yamada
  0 siblings, 1 reply; 5+ messages in thread
From: Conor Dooley @ 2022-12-25 15:18 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-arch, linux-kbuild, linux-kernel, Ard Biesheuvel,
	Thorsten Leemhuis, Catalin Marinas, Will Deacon,
	linux-arm-kernel, linux-riscv, Dennis Gilmore, Albert Ou,
	Arnd Bergmann, Jisheng Zhang, Nicolas Schier, Palmer Dabbelt,
	Paul Walmsley

[-- Attachment #1: Type: text/plain, Size: 3266 bytes --]

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.

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arch: fix broken BuildID for arm64 and riscv
  2022-12-25 15:18 ` Conor Dooley
@ 2022-12-26 19:06   ` Masahiro Yamada
  2022-12-26 22:02     ` Conor Dooley
  0 siblings, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2022-12-26 19:06 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-arch, linux-kbuild, linux-kernel, Ard Biesheuvel,
	Thorsten Leemhuis, Catalin Marinas, Will Deacon,
	linux-arm-kernel, linux-riscv, Dennis Gilmore, Albert Ou,
	Arnd Bergmann, Jisheng Zhang, Nicolas Schier, Palmer Dabbelt,
	Paul Walmsley

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arch: fix broken BuildID for arm64 and riscv
  2022-12-26 19:06   ` Masahiro Yamada
@ 2022-12-26 22:02     ` Conor Dooley
  2022-12-27 10:45       ` Masahiro Yamada
  0 siblings, 1 reply; 5+ messages in thread
From: Conor Dooley @ 2022-12-26 22:02 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-arch, linux-kbuild, linux-kernel, Ard Biesheuvel,
	Thorsten Leemhuis, Catalin Marinas, Will Deacon,
	linux-arm-kernel, linux-riscv, Dennis Gilmore, Albert Ou,
	Arnd Bergmann, Jisheng Zhang, Nicolas Schier, Palmer Dabbelt,
	Paul Walmsley

[-- Attachment #1: Type: text/plain, Size: 3150 bytes --]

Hey Masahiro,

On Tue, Dec 27, 2022 at 04:06:35AM +0900, Masahiro Yamada wrote:
> 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.
> >
> > 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.

Huh, fair enough. The diff for that appears to be trivial, but I was not
able to correctly determine a fixes tag. I may have erred in my
history-diving, but it's a wee bit hard to determine the correct fixes
tag. That comment about runtime discards appears to date back to
commit fbe934d69eb7 ("RISC-V: Build Infrastructure") in 2017 -
apparently pre-dating the addition of the define in the first place.

Commit 84d5f77fc2ee ("x86, vmlinux.lds: Add RUNTIME_DISCARD_EXIT to
generic DISCARDS") added it to x86 but not to arm64 - but it seems like
it was added to arm during a later reword. Does that make 84d5f77fc2ee
the correct one to mark it as a fix of & riscv was just overlooked when
the define was added?


> 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

Sweet, thanks. Hopefully the automation likes that version better :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arch: fix broken BuildID for arm64 and riscv
  2022-12-26 22:02     ` Conor Dooley
@ 2022-12-27 10:45       ` Masahiro Yamada
  0 siblings, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2022-12-27 10:45 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-arch, linux-kbuild, linux-kernel, Ard Biesheuvel,
	Thorsten Leemhuis, Catalin Marinas, Will Deacon,
	linux-arm-kernel, linux-riscv, Dennis Gilmore, Albert Ou,
	Arnd Bergmann, Jisheng Zhang, Nicolas Schier, Palmer Dabbelt,
	Paul Walmsley

On Tue, Dec 27, 2022 at 7:02 AM Conor Dooley <conor@kernel.org> wrote:
>
> Hey Masahiro,
>
> On Tue, Dec 27, 2022 at 04:06:35AM +0900, Masahiro Yamada wrote:
> > 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.
> > >
> > > 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.
>
> Huh, fair enough. The diff for that appears to be trivial, but I was not
> able to correctly determine a fixes tag. I may have erred in my
> history-diving, but it's a wee bit hard to determine the correct fixes
> tag. That comment about runtime discards appears to date back to
> commit fbe934d69eb7 ("RISC-V: Build Infrastructure") in 2017 -
> apparently pre-dating the addition of the define in the first place.
>
> Commit 84d5f77fc2ee ("x86, vmlinux.lds: Add RUNTIME_DISCARD_EXIT to
> generic DISCARDS") added it to x86 but not to arm64 - but it seems like
> it was added to arm during a later reword. Does that make 84d5f77fc2ee
> the correct one to mark it as a fix of & riscv was just overlooked when
> the define was added?
>


You do not need to add the Fixes tag.

Currently, it is working, but it turns out bad
only when somebody moves "DISCARDS" up in the linker script.







> > 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
>
> Sweet, thanks. Hopefully the automation likes that version better :)
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-12-27 10:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-24 19:27 [PATCH] arch: fix broken BuildID for arm64 and riscv Masahiro Yamada
2022-12-25 15:18 ` Conor Dooley
2022-12-26 19:06   ` Masahiro Yamada
2022-12-26 22:02     ` Conor Dooley
2022-12-27 10:45       ` Masahiro Yamada

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 NNTP newsgroup(s).