All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Nick Desaulniers <ndesaulniers@google.com>,
	Nicholas Piggin <npiggin@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	clang-built-linux@googlegroups.com,
	Kees Cook <keescook@chromium.org>,
	Fangrui Song <maskray@google.com>,
	Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
Date: Wed, 02 Sep 2020 22:14:05 +1000	[thread overview]
Message-ID: <87blio1ilu.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20200901222523.1941988-2-ndesaulniers@google.com>

Nick Desaulniers <ndesaulniers@google.com> 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_bS92duihhywLJQ@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  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

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Nick Desaulniers <ndesaulniers@google.com>,
	Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Kees Cook <keescook@chromium.org>,
	Fangrui Song <maskray@google.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
Date: Wed, 02 Sep 2020 22:14:05 +1000	[thread overview]
Message-ID: <87blio1ilu.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20200901222523.1941988-2-ndesaulniers@google.com>

Nick Desaulniers <ndesaulniers@google.com> 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_bS92duihhywLJQ@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  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

  reply	other threads:[~2020-09-02 12:14 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 22:25 [PATCH 0/2] link vdso with linker Nick Desaulniers
2020-09-01 22:25 ` Nick Desaulniers
2020-09-01 22:25 ` [PATCH 1/2] powerpc/vdso64: link vdso64 " Nick Desaulniers
2020-09-01 22:25   ` Nick Desaulniers
2020-09-02 12:14   ` Michael Ellerman [this message]
2020-09-02 12:14     ` Michael Ellerman
2020-09-02 17:41     ` Nick Desaulniers
2020-09-02 17:41       ` Nick Desaulniers
2020-09-02 18:02       ` Christophe Leroy
2021-04-22 22:44         ` Nick Desaulniers
2021-04-22 22:44           ` Nick Desaulniers
2021-04-23  7:40           ` Christophe Leroy
2021-04-23  7:40             ` Christophe Leroy
2021-04-23 17:18           ` Christophe Leroy
2021-04-23 17:18             ` Christophe Leroy
2022-03-02 16:48   ` Christophe Leroy
2020-09-01 22:25 ` [PATCH 2/2] powerpc/vdso32: " Nick Desaulniers
2020-09-01 22:25   ` Nick Desaulniers
2020-09-02  2:58   ` Kees Cook
2020-09-02  2:58     ` Kees Cook
2020-09-02  6:46   ` Christophe Leroy
2020-09-02  6:46     ` Christophe Leroy
2020-09-02 14:14     ` Segher Boessenkool
2020-09-02 14:14       ` Segher Boessenkool
2020-09-02 15:43       ` Christophe Leroy
2020-09-02 15:43         ` Christophe Leroy
2020-09-02 16:57         ` Segher Boessenkool
2020-09-02 16:57           ` Segher Boessenkool
2020-09-02  7:56   ` Christophe Leroy
2020-09-02  7:56     ` Christophe Leroy
2020-09-02 10:16   ` Christophe Leroy
2020-09-02 10:16     ` Christophe Leroy
2020-09-02  5:21 ` [PATCH 0/2] link vdso " Nathan Chancellor
2020-09-02  5:21   ` Nathan Chancellor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87blio1ilu.fsf@mpe.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@c-s.fr \
    --cc=clang-built-linux@googlegroups.com \
    --cc=joe.lawrence@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maskray@google.com \
    --cc=ndesaulniers@google.com \
    --cc=npiggin@gmail.com \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.