Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker

2022-03-02 Thread Christophe Leroy




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

2021-04-23 Thread Christophe Leroy




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

2021-04-23 Thread Christophe Leroy




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

2021-04-22 Thread Nick Desaulniers
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

2020-09-02 Thread Christophe Leroy




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

2020-09-02 Thread Michael Ellerman
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