| Message ID | 20221224192751.810363-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arch: fix broken BuildID for arm64 and riscv | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be fixes |
| conchuod/fixes_present | success | Fixes tag present in non-next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 12 and now 12 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | fail | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | warning | CHECK: spaces preferred around that '-' (ctx:VxV) |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | Fixes tag looks correct |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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 >
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 > >
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 :)
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 --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/ : { \
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(-)