Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
Le 02/09/2020 à 00:25, Nick Desaulniers a écrit : Rather than invoke the compiler as the driver, use the linker. That way we can check --orphan-handling=warn support correctly, as cc-ldoption was removed in commit 055efab3120b ("kbuild: drop support for cc-ldoption"). Painstakingly compared the output between `objdump -a` before and after this change. Now function symbols have the correct type of FUNC rather than NONE, and the entry is slightly different (which doesn't matter for the vdso). Binary size is the same. Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections") Link: https://lore.kernel.org/lkml/cakwvodnn3wxydjomvnveyd_njwrku3fabwt_bs92duihhyw...@mail.gmail.com/ Signed-off-by: Nick Desaulniers Is this change still necessary ? If so please rebase as we have changed the structure of VDSO source files (Only one directory common to 32 and 64). Christophe --- arch/powerpc/include/asm/vdso.h | 17 ++--- arch/powerpc/kernel/vdso64/Makefile | 8 ++-- arch/powerpc/kernel/vdso64/vdso64.lds.S | 1 - 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h index 2ff884853f97..11b2ecf49f79 100644 --- a/arch/powerpc/include/asm/vdso.h +++ b/arch/powerpc/include/asm/vdso.h @@ -24,19 +24,7 @@ int vdso_getcpu_init(void); #else /* __ASSEMBLY__ */ -#ifdef __VDSO64__ -#define V_FUNCTION_BEGIN(name) \ - .globl name;\ - name: \ - -#define V_FUNCTION_END(name) \ - .size name,.-name; - -#define V_LOCAL_FUNC(name) (name) -#endif /* __VDSO64__ */ - -#ifdef __VDSO32__ - +#if defined(__VDSO32__) || defined (__VDSO64__) #define V_FUNCTION_BEGIN(name)\ .globl name;\ .type name,@function; \ @@ -46,8 +34,7 @@ int vdso_getcpu_init(void); .size name,.-name; #define V_LOCAL_FUNC(name) (name) - -#endif /* __VDSO32__ */ +#endif /* __VDSO{32|64}__ */ #endif /* __ASSEMBLY__ */ diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile index 38c317f25141..7ea3ce537d0a 100644 --- a/arch/powerpc/kernel/vdso64/Makefile +++ b/arch/powerpc/kernel/vdso64/Makefile @@ -32,9 +32,13 @@ $(obj)/%.so: OBJCOPYFLAGS := -S $(obj)/%.so: $(obj)/%.so.dbg FORCE $(call if_changed,objcopy) +ldflags-y := -shared -soname linux-vdso64.so.1 \ + $(call ld-option, --eh-frame-hdr) \ + $(call ld-option, --orphan-handling=warn) -T + # actual build commands -quiet_cmd_vdso64ld = VDSO64L $@ - cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) $(call cc-ldoption, -Wl$(comma)--orphan-handling=warn) +quiet_cmd_vdso64ld = LD $@ + cmd_vdso64ld = $(cmd_ld) # install commands for the unstripped file quiet_cmd_vdso_install = INSTALL $@ diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S index 4e3a8d4ee614..58c33b704b6a 100644 --- a/arch/powerpc/kernel/vdso64/vdso64.lds.S +++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S @@ -11,7 +11,6 @@ OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") #endif OUTPUT_ARCH(powerpc:common64) -ENTRY(_start) SECTIONS {
Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
Le 23/04/2021 à 00:44, Nick Desaulniers a écrit : On Wed, Sep 2, 2020 at 11:02 AM Christophe Leroy wrote: Le 02/09/2020 à 19:41, Nick Desaulniers a écrit : On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman wrote: Nick Desaulniers writes: Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections") I think I'll just revert that for v5.9 ? SGTM; you'll probably still want these changes with some modifications at some point; vdso32 did have at least one orphaned section, and will be important for hermetic builds. Seeing crashes in supported versions of the tools ties our hands at the moment. Keeping the tool problem aside with binutils 2.26, do you have a way to really link an elf32ppc object when building vdso32 for PPC64 ? Sorry, I'm doing a bug scrub and found https://github.com/ClangBuiltLinux/linux/issues/774 still open (and my reply to this thread still in Drafts; never sent). With my patches rebased: $ file arch/powerpc/kernel/vdso32/vdso32.so arch/powerpc/kernel/vdso32/vdso32.so: ELF 32-bit MSB shared object, PowerPC or cisco 4500, version 1 (SYSV), dynamically linked, stripped Are you still using 2.26? I'm not able to repro Nathan's reported issue from https://lore.kernel.org/lkml/20200902052123.GA2687902@ubuntu-n2-xlarge-x86/, so I'm curious if I should resend the rebased patches as v2? One comment on your rebased patch: > diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h > index 8542e9bbeead..0bd06ec06aaa 100644 > --- a/arch/powerpc/include/asm/vdso.h > +++ b/arch/powerpc/include/asm/vdso.h > @@ -25,19 +25,7 @@ int vdso_getcpu_init(void); > > #else /* __ASSEMBLY__ */ > > -#ifdef __VDSO64__ > -#define V_FUNCTION_BEGIN(name)\ > - .globl name;\ > - name: \ > - > -#define V_FUNCTION_END(name) \ > - .size name,.-name; > - > -#define V_LOCAL_FUNC(name) (name) > -#endif /* __VDSO64__ */ > - > -#ifdef __VDSO32__ > - > +#if defined(__VDSO32__) || defined (__VDSO64__) You always have either __VDSO32__ or __VDSO64__ so this #if is pointless > #define V_FUNCTION_BEGIN(name) \ >.globl name;\ >.type name,@function; \ > @@ -47,8 +35,7 @@ int vdso_getcpu_init(void); >.size name,.-name; > > #define V_LOCAL_FUNC(name) (name) > - > -#endif /* __VDSO32__ */ > +#endif /* __VDSO{32|64}__ */ > > #endif /* __ASSEMBLY__ */ > Christophe
Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
Le 23/04/2021 à 00:44, Nick Desaulniers a écrit : On Wed, Sep 2, 2020 at 11:02 AM Christophe Leroy wrote: Le 02/09/2020 à 19:41, Nick Desaulniers a écrit : On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman wrote: Nick Desaulniers writes: Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections") I think I'll just revert that for v5.9 ? SGTM; you'll probably still want these changes with some modifications at some point; vdso32 did have at least one orphaned section, and will be important for hermetic builds. Seeing crashes in supported versions of the tools ties our hands at the moment. Keeping the tool problem aside with binutils 2.26, do you have a way to really link an elf32ppc object when building vdso32 for PPC64 ? Sorry, I'm doing a bug scrub and found https://github.com/ClangBuiltLinux/linux/issues/774 still open (and my reply to this thread still in Drafts; never sent). With my patches rebased: $ file arch/powerpc/kernel/vdso32/vdso32.so arch/powerpc/kernel/vdso32/vdso32.so: ELF 32-bit MSB shared object, PowerPC or cisco 4500, version 1 (SYSV), dynamically linked, stripped Are you still using 2.26? Yes, our production kernels and applications are built with gcc 5.5 and binutils 2.26 I'm not able to repro Nathan's reported issue from https://lore.kernel.org/lkml/20200902052123.GA2687902@ubuntu-n2-xlarge-x86/, so I'm curious if I should resend the rebased patches as v2? I can't remember what was all this discussion about. I gave a try to your rebased patches. Still an issue with binutils 2.26: VDSO32L arch/powerpc/kernel/vdso32/vdso32.so.dbg ppc-linux-ld: warning: orphan section `.rela.got' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn'. ppc-linux-ld: warning: orphan section `.rela.plt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn'. ppc-linux-ld: warning: orphan section `.glink' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.glink'. ppc-linux-ld: warning: orphan section `.iplt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.iplt'. ppc-linux-ld: warning: orphan section `.rela.iplt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn'. ppc-linux-ld: warning: orphan section `.rela.text' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn'. /bin/sh: line 1: 7850 Segmentation fault (core dumped) ppc-linux-ld -EB -m elf32ppc -shared -soname linux-vdso32.so.1 --eh-frame-hdr --orphan-handling=warn -T arch/powerpc/kernel/vdso32/vdso32.lds arch/powerpc/kernel/vdso32/sigtramp.o arch/powerpc/kernel/vdso32/gettimeofday.o arch/powerpc/kernel/vdso32/datapage.o arch/powerpc/kernel/vdso32/cacheflush.o arch/powerpc/kernel/vdso32/note.o arch/powerpc/kernel/vdso32/getcpu.o arch/powerpc/kernel/vdso32/vgettimeofday.o -o arch/powerpc/kernel/vdso32/vdso32.so.dbg make[2]: *** [arch/powerpc/kernel/vdso32/vdso32.so.dbg] Error 139 make[2]: *** Deleting file `arch/powerpc/kernel/vdso32/vdso32.so.dbg' With gcc 10.1 and binutils 2.34 I get: PPC32 build: VDSO32L arch/powerpc/kernel/vdso32/vdso32.so.dbg powerpc64-linux-ld: warning: orphan section `.rela.got' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' powerpc64-linux-ld: warning: orphan section `.rela.plt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' powerpc64-linux-ld: warning: orphan section `.glink' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.glink' powerpc64-linux-ld: warning: orphan section `.iplt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.iplt' powerpc64-linux-ld: warning: orphan section `.rela.iplt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' powerpc64-linux-ld: warning: orphan section `.rela.branch_lt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' powerpc64-linux-ld: warning: orphan section `.rela.text' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' PPC64 build: VDSO32L arch/powerpc/kernel/vdso32/vdso32.so.dbg powerpc64-linux-ld: warning: orphan section `.rela.got' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' powerpc64-linux-ld: warning: orphan section `.rela.plt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' powerpc64-linux-ld: warning: orphan section `.glink' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.glink' powerpc64-linux-ld: warning: orphan section `.iplt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.iplt' powerpc64-linux-ld: warning: orphan section `.rela.iplt' from `arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.rela.dyn' powerpc64-linux-ld: warning: orphan section `.rela.branch_lt' from
Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
On Wed, Sep 2, 2020 at 11:02 AM Christophe Leroy wrote: > > > > Le 02/09/2020 à 19:41, Nick Desaulniers a écrit : > > On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman wrote: > >> > >> Nick Desaulniers writes: > >>> Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for > >>> orphan sections") > >> > >> I think I'll just revert that for v5.9 ? > > > > SGTM; you'll probably still want these changes with some modifications > > at some point; vdso32 did have at least one orphaned section, and will > > be important for hermetic builds. Seeing crashes in supported > > versions of the tools ties our hands at the moment. > > > > Keeping the tool problem aside with binutils 2.26, do you have a way to > really link an elf32ppc object when building vdso32 for PPC64 ? Sorry, I'm doing a bug scrub and found https://github.com/ClangBuiltLinux/linux/issues/774 still open (and my reply to this thread still in Drafts; never sent). With my patches rebased: $ file arch/powerpc/kernel/vdso32/vdso32.so arch/powerpc/kernel/vdso32/vdso32.so: ELF 32-bit MSB shared object, PowerPC or cisco 4500, version 1 (SYSV), dynamically linked, stripped Are you still using 2.26? I'm not able to repro Nathan's reported issue from https://lore.kernel.org/lkml/20200902052123.GA2687902@ubuntu-n2-xlarge-x86/, so I'm curious if I should resend the rebased patches as v2? -- Thanks, ~Nick Desaulniers
Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
Le 02/09/2020 à 19:41, Nick Desaulniers a écrit : On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman wrote: Nick Desaulniers writes: Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan sections") I think I'll just revert that for v5.9 ? SGTM; you'll probably still want these changes with some modifications at some point; vdso32 did have at least one orphaned section, and will be important for hermetic builds. Seeing crashes in supported versions of the tools ties our hands at the moment. Keeping the tool problem aside with binutils 2.26, do you have a way to really link an elf32ppc object when building vdso32 for PPC64 ? Christophe
Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
Nick Desaulniers writes: > Rather than invoke the compiler as the driver, use the linker. That way > we can check --orphan-handling=warn support correctly, as cc-ldoption > was removed in > commit 055efab3120b ("kbuild: drop support for cc-ldoption"). Ouch. Seems make is quite happy to $(call deadbeef, ...) and not print a warning, which I guess is probably a feature. > Painstakingly compared the output between `objdump -a` before and after > this change. Now function symbols have the correct type of FUNC rather > than NONE, and the entry is slightly different (which doesn't matter for > the vdso). Binary size is the same. > > Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan > sections") I think I'll just revert that for v5.9 ? cheers > Link: > https://lore.kernel.org/lkml/cakwvodnn3wxydjomvnveyd_njwrku3fabwt_bs92duihhyw...@mail.gmail.com/ > Signed-off-by: Nick Desaulniers > --- > arch/powerpc/include/asm/vdso.h | 17 ++--- > arch/powerpc/kernel/vdso64/Makefile | 8 ++-- > arch/powerpc/kernel/vdso64/vdso64.lds.S | 1 - > 3 files changed, 8 insertions(+), 18 deletions(-) > > diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h > index 2ff884853f97..11b2ecf49f79 100644 > --- a/arch/powerpc/include/asm/vdso.h > +++ b/arch/powerpc/include/asm/vdso.h > @@ -24,19 +24,7 @@ int vdso_getcpu_init(void); > > #else /* __ASSEMBLY__ */ > > -#ifdef __VDSO64__ > -#define V_FUNCTION_BEGIN(name) \ > - .globl name;\ > - name: \ > - > -#define V_FUNCTION_END(name) \ > - .size name,.-name; > - > -#define V_LOCAL_FUNC(name) (name) > -#endif /* __VDSO64__ */ > - > -#ifdef __VDSO32__ > - > +#if defined(__VDSO32__) || defined (__VDSO64__) > #define V_FUNCTION_BEGIN(name) \ > .globl name;\ > .type name,@function; \ > @@ -46,8 +34,7 @@ int vdso_getcpu_init(void); > .size name,.-name; > > #define V_LOCAL_FUNC(name) (name) > - > -#endif /* __VDSO32__ */ > +#endif /* __VDSO{32|64}__ */ > > #endif /* __ASSEMBLY__ */ > > diff --git a/arch/powerpc/kernel/vdso64/Makefile > b/arch/powerpc/kernel/vdso64/Makefile > index 38c317f25141..7ea3ce537d0a 100644 > --- a/arch/powerpc/kernel/vdso64/Makefile > +++ b/arch/powerpc/kernel/vdso64/Makefile > @@ -32,9 +32,13 @@ $(obj)/%.so: OBJCOPYFLAGS := -S > $(obj)/%.so: $(obj)/%.so.dbg FORCE > $(call if_changed,objcopy) > > +ldflags-y := -shared -soname linux-vdso64.so.1 \ > + $(call ld-option, --eh-frame-hdr) \ > + $(call ld-option, --orphan-handling=warn) -T > + > # actual build commands > -quiet_cmd_vdso64ld = VDSO64L $@ > - cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) > $(filter %.o,$^) $(call cc-ldoption, -Wl$(comma)--orphan-handling=warn) > +quiet_cmd_vdso64ld = LD $@ > + cmd_vdso64ld = $(cmd_ld) > > # install commands for the unstripped file > quiet_cmd_vdso_install = INSTALL $@ > diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S > b/arch/powerpc/kernel/vdso64/vdso64.lds.S > index 4e3a8d4ee614..58c33b704b6a 100644 > --- a/arch/powerpc/kernel/vdso64/vdso64.lds.S > +++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S > @@ -11,7 +11,6 @@ OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", > "elf64-powerpcle") > OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") > #endif > OUTPUT_ARCH(powerpc:common64) > -ENTRY(_start) > > SECTIONS > { > -- > 2.28.0.402.g5ffc5be6b7-goog