Messages in this thread | | | From | Nick Desaulniers <> | Date | Sat, 22 Feb 2020 15:33:20 -0800 | Subject | Re: [PATCH] x86/boot/compressed: Fix compressed kernel linking with lld |
| |
On Sat, Feb 22, 2020 at 1:01 PM 'Fangrui Song' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > On 2020-02-22, Arvind Sankar wrote: > >On Sat, Feb 22, 2020 at 10:58:06AM -0800, Fangrui Song wrote: > >> On 2020-02-22, Nathan Chancellor wrote: > >> >On Sat, Feb 22, 2020 at 12:18:59PM -0500, Arvind Sankar wrote: > >> >> Commit TBD ("x86/boot/compressed: Remove unnecessary sections from > >> >> bzImage") discarded unnecessary sections with *(*). While this works > >> >> fine with the bfd linker, lld tries to also discard essential sections > >> >> like .shstrtab, .symtab and .strtab, which results in the link failing > >> >> since .shstrtab is required by the ELF specification. .symtab and > >> >> .strtab are also necessary to generate the zoffset.h file for the > >> >> bzImage header. > >> >> > >> >> Since the only sizeable section that can be discarded is .eh_frame, > >> >> restrict the discard to only .eh_frame to be safe. > >> >> > >> >> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > >> >> --- > >> >> Sending as a fix on top of tip/x86/boot. > >> >> > >> >> arch/x86/boot/compressed/vmlinux.lds.S | 4 ++-- > >> >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S > >> >> index 12a20603d92e..469dcf800a2c 100644 > >> >> --- a/arch/x86/boot/compressed/vmlinux.lds.S > >> >> +++ b/arch/x86/boot/compressed/vmlinux.lds.S > >> >> @@ -74,8 +74,8 @@ SECTIONS > >> >> . = ALIGN(PAGE_SIZE); /* keep ZO size page aligned */ > >> >> _end = .; > >> >> > >> >> - /* Discard all remaining sections */ > >> >> + /* Discard .eh_frame to save some space */ > >> >> /DISCARD/ : { > >> >> - *(*) > >> >> + *(.eh_frame) > >> >> } > >> >> } > >> >> -- > >> >> 2.24.1 > >> >> > >> > > >> >FWIW: > >> > > >> >Tested-by: Nathan Chancellor <natechancellor@gmail.com> > >> > >> I am puzzled. Doesn't -fno-asynchronous-unwind-tables suppress > >> .eh_frame in the object files? Why are there still .eh_frame? > >> > >> Though, there is prior art: arch/s390/boot/compressed/vmlinux.lds.S also discards .eh_frame > > > >The compressed kernel doesn't use the regular flags and it seems it > >doesn't have that option. Maybe we should add it in to avoid generating > >those in the first place.
Ah, yikes. For reference, please see my commit:
commit b059f801a937 ("x86/purgatory: Use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS")
I'm of the conviction that reassigning KBUILD_CFLAGS via `:=`, as opposed to strictly filtering flags out of it or appending to it, is an antipattern. We very very carefully construct KBUILD_CFLAGS in top level and arch/ Makefiles, and it's very easy to miss a flag or to when you "reset" KBUILD_CFLAGS.
*Boom* Case in point.
I meant to audit the rest of the places we do this in the kernel, but haven't had the time to revisit arch/x86/boot/compressed/Makefile.
For now, I suggest: 1. revert `Commit TBD ("x86/boot/compressed: Remove unnecessary sections from bzImage")` as it runs afoul differences in `*` for `DISCARD` sections between linkers, as the intent was to remove .eh_frame, of which it's less work to not generate them in the first place via compiler flag, rather than generate then discard via linker. 2. simply add `KBUILD_CFLAGS += -fno-asynchronous-unwind-tables` to arch/x86/boot/compressed/Makefile with Fangrui's Sugguested-by tag. 3. Remind me to revisit my proposed cleanup of arch/x86/boot/compressed/Makefile (which eventually will undo #2). ;) 4. tglx to remind me that my compiler is broken and that I should fix it. :P
> > > >The .eh_frame discard in arch/x86/kernel/vmlinux.lds.S does seem > >superfluous though. > > Yes, please do that. I recommend suppressting unneeded sections at > compile time, instead of discarding them at link time. > > https://github.com/torvalds/linux/commit/83a092cf95f28696ddc36c8add0cf03ac034897f > added -Wl,--orphan-handling=warn to arch/powerpc/Makefile . > x86 can follow if that is appropriate.
Kees has patches for that; I recommend waiting on his series.
> I don't recommend -Wl,--orphan-handling=error, which can unnecessarily > break the builds.
Agreed. Until we have CI testing with LLD which is a WIP (action item on me). -- Thanks, ~Nick Desaulniers
| |