diff mbox series

arch: fix broken BuildID for arm64 and riscv

Message ID 20221224192751.810363-1-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series arch: fix broken BuildID for arm64 and riscv | expand

Commit Message

Masahiro Yamada Dec. 24, 2022, 7:27 p.m. UTC 
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(-)

Comments

Conor Dooley Dec. 25, 2022, 3:18 p.m. UTC | #1 
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
>
Masahiro Yamada Dec. 26, 2022, 7:06 p.m. UTC | #2 
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
> >
Conor Dooley Dec. 26, 2022, 10:02 p.m. UTC | #3 
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 :)
Masahiro Yamada Dec. 27, 2022, 10:45 a.m. UTC | #4 
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 :)
>
diff mbox series

Patch

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/ : {							\