[PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

[PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

[Masahiro Yamada]

We use $(LD) to link vmlinux, modules, decompressors, etc.

VDSO is the only exceptional case where $(CC) is used as the linker
driver, but I do not know why we need to do so. VDSO uses a special
linker script, and does not link standard libraries at all.

I changed the Makefile to use $(LD) rather than $(CC). I tested this,
and VDSO worked for me.

Users will be able to use their favorite linker (e.g. lld instead of
of bfd) by passing LD= from the command line.

My plan is to rewrite all VDSO Makefiles to use $(LD), then delete
cc-ldoption.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm64/kernel/vdso/Makefile | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index a0af6bf6c11b..744b9dbaba03 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -12,17 +12,12 @@ obj-vdso := gettimeofday.o note.o sigreturn.o
 targets := $(obj-vdso) vdso.so vdso.so.dbg
 obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
 
-ccflags-y := -shared -fno-common -fno-builtin
-ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
-		$(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
+ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 \
+		$(call ld-option, --hash-style=sysv) -n -T
 
 # Disable gcov profiling for VDSO code
 GCOV_PROFILE := n
 
-# Workaround for bare-metal (ELF) toolchains that neglect to pass -shared
-# down to collect2, resulting in silent corruption of the vDSO image.
-ccflags-y += -Wl,-shared
-
 obj-y += vdso.o
 extra-y += vdso.lds
 CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
@@ -32,7 +27,7 @@ $(obj)/vdso.o : $(obj)/vdso.so
 
 # Link rule for the .so file, .lds has to be first
 $(obj)/vdso.so.dbg: $(obj)/vdso.lds $(obj-vdso) FORCE
-	$(call if_changed,vdsold)
+	$(call if_changed,ld)
 
 # Strip rule for the .so file
 $(obj)/%.so: OBJCOPYFLAGS := -S
@@ -52,8 +47,6 @@ $(obj-vdso): %.o: %.S FORCE
 	$(call if_changed_dep,vdsoas)
 
 # Actual build commands
-quiet_cmd_vdsold = VDSOL   $@
-      cmd_vdsold = $(CC) $(c_flags) -Wl,-n -Wl,-T $(real-prereqs) -o $@
 quiet_cmd_vdsoas = VDSOA   $@
       cmd_vdsoas = $(CC) $(a_flags) -c -o $@ $<
 
-- 
2.17.1

Re: [PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

[Nick Desaulniers]

On Thu, Apr 11, 2019 at 2:30 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> We use $(LD) to link vmlinux, modules, decompressors, etc.
>
> VDSO is the only exceptional case where $(CC) is used as the linker
> driver, but I do not know why we need to do so. VDSO uses a special
> linker script, and does not link standard libraries at all.
>
> I changed the Makefile to use $(LD) rather than $(CC). I tested this,
> and VDSO worked for me.
>
> Users will be able to use their favorite linker (e.g. lld instead of
> of bfd) by passing LD= from the command line.
>
> My plan is to rewrite all VDSO Makefiles to use $(LD), then delete
> cc-ldoption.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  arch/arm64/kernel/vdso/Makefile | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index a0af6bf6c11b..744b9dbaba03 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -12,17 +12,12 @@ obj-vdso := gettimeofday.o note.o sigreturn.o
>  targets := $(obj-vdso) vdso.so vdso.so.dbg
>  obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
>
> -ccflags-y := -shared -fno-common -fno-builtin

Thanks for the patch; in general LGTM. Just some small questions:

Looks like -shared and -fno-common are linker flags forwarded along,
but -fno-builtin is meant for the compiler, right? Do we want to keep
that in ccflags?  I'm not sure why it would have been added
intentionally; maybe Will knows?  Maybe it's ok to drop.

> -ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
> -               $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
> +ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 \
> +               $(call ld-option, --hash-style=sysv) -n -T

Forgive my ignorance, but the man page for ld makes it looks like the
-T argument requires a subsequent argument `scriptfile`.  Is that what
`$(real-prereqs)` was doing below? If so, is dropping it in this patch
intentional?

>
>  # Disable gcov profiling for VDSO code
>  GCOV_PROFILE := n
>
> -# Workaround for bare-metal (ELF) toolchains that neglect to pass -shared
> -# down to collect2, resulting in silent corruption of the vDSO image.
> -ccflags-y += -Wl,-shared
> -
>  obj-y += vdso.o
>  extra-y += vdso.lds
>  CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
> @@ -32,7 +27,7 @@ $(obj)/vdso.o : $(obj)/vdso.so
>
>  # Link rule for the .so file, .lds has to be first
>  $(obj)/vdso.so.dbg: $(obj)/vdso.lds $(obj-vdso) FORCE
> -       $(call if_changed,vdsold)
> +       $(call if_changed,ld)
>
>  # Strip rule for the .so file
>  $(obj)/%.so: OBJCOPYFLAGS := -S
> @@ -52,8 +47,6 @@ $(obj-vdso): %.o: %.S FORCE
>         $(call if_changed_dep,vdsoas)
>
>  # Actual build commands
> -quiet_cmd_vdsold = VDSOL   $@
> -      cmd_vdsold = $(CC) $(c_flags) -Wl,-n -Wl,-T $(real-prereqs) -o $@
>  quiet_cmd_vdsoas = VDSOA   $@
>        cmd_vdsoas = $(CC) $(a_flags) -c -o $@ $<
>
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

[Masahiro Yamada]

Hi Nick,

On Fri, Apr 12, 2019 at 3:20 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Apr 11, 2019 at 2:30 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > We use $(LD) to link vmlinux, modules, decompressors, etc.
> >
> > VDSO is the only exceptional case where $(CC) is used as the linker
> > driver, but I do not know why we need to do so. VDSO uses a special
> > linker script, and does not link standard libraries at all.
> >
> > I changed the Makefile to use $(LD) rather than $(CC). I tested this,
> > and VDSO worked for me.
> >
> > Users will be able to use their favorite linker (e.g. lld instead of
> > of bfd) by passing LD= from the command line.
> >
> > My plan is to rewrite all VDSO Makefiles to use $(LD), then delete
> > cc-ldoption.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> >  arch/arm64/kernel/vdso/Makefile | 13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> > index a0af6bf6c11b..744b9dbaba03 100644
> > --- a/arch/arm64/kernel/vdso/Makefile
> > +++ b/arch/arm64/kernel/vdso/Makefile
> > @@ -12,17 +12,12 @@ obj-vdso := gettimeofday.o note.o sigreturn.o
> >  targets := $(obj-vdso) vdso.so vdso.so.dbg
> >  obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
> >
> > -ccflags-y := -shared -fno-common -fno-builtin
>
> Thanks for the patch; in general LGTM. Just some small questions:
>
> Looks like -shared and -fno-common are linker flags forwarded along,
> but -fno-builtin is meant for the compiler, right?

Definitely, -shared must be passed to the linker
to create a shared object.
It is listed in the linker option list:
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Link-Options.html#Link-Options



I think -fno-common is a compiler flag instead of a linker one
as far as I understand this:
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Code-Gen-Options.html#Code-Gen-Options


-fno-builtin is a compiler flag:
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/C-Dialect-Options.html#C-Dialect-Options



> Do we want to keep
> that in ccflags?

All source files in arch/arm64/kernel/vdso/
are written in assembly (gettimeofday.S note.S sigreturn.S).

So, -fno-common -fno-builtin for ccflags-y
are not used.

That's why I dropped them.

If we want to keep them just in case,
it is fine with me too.


>  I'm not sure why it would have been added
> intentionally; maybe Will knows?  Maybe it's ok to drop.

They have been here since the initial VDSO support
by commit 9031fefde6f2a.

Will, do you remember why you added these flags?




> > -ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
> > -               $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
> > +ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 \
> > +               $(call ld-option, --hash-style=sysv) -n -T
>
> Forgive my ignorance, but the man page for ld makes it looks like the
> -T argument requires a subsequent argument `scriptfile`.  Is that what
> `$(real-prereqs)` was doing below? If so, is dropping it in this patch
> intentional?

I just re-used cmd_ld in scripts/Makefile.lib


It will be expanded like follows:

cmd_ld = $(LD) $(ld_flags) $(real-prereqs) -o $@

 ->

 $(LD) $(KBUILD_LDFLAGS) $(ldflags-y) $(LDFLAGS_$(@F)) $(real-prereqs) -o $@

 ->

$(LD) -shared -nostdlib -soname=linux-vdso.so.1 \
     --hash-style=sysv -n -T \
    $(obj)/vdso.lds $(obj-vdso) -o $(obj)/vdso.so.dbg



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

[Will Deacon]

Hi all,

On Fri, Apr 12, 2019 at 10:21:38AM +0900, Masahiro Yamada wrote:
> On Fri, Apr 12, 2019 at 3:20 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> > On Thu, Apr 11, 2019 at 2:30 AM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> > >
> > > We use $(LD) to link vmlinux, modules, decompressors, etc.
> > >
> > > VDSO is the only exceptional case where $(CC) is used as the linker
> > > driver, but I do not know why we need to do so. VDSO uses a special
> > > linker script, and does not link standard libraries at all.
> > >
> > > I changed the Makefile to use $(LD) rather than $(CC). I tested this,
> > > and VDSO worked for me.
> > >
> > > Users will be able to use their favorite linker (e.g. lld instead of
> > > of bfd) by passing LD= from the command line.
> > >
> > > My plan is to rewrite all VDSO Makefiles to use $(LD), then delete
> > > cc-ldoption.
> > >
> > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > ---
> > >
> > >  arch/arm64/kernel/vdso/Makefile | 13 +++----------
> > >  1 file changed, 3 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> > > index a0af6bf6c11b..744b9dbaba03 100644
> > > --- a/arch/arm64/kernel/vdso/Makefile
> > > +++ b/arch/arm64/kernel/vdso/Makefile
> > > @@ -12,17 +12,12 @@ obj-vdso := gettimeofday.o note.o sigreturn.o
> > >  targets := $(obj-vdso) vdso.so vdso.so.dbg
> > >  obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
> > >
> > > -ccflags-y := -shared -fno-common -fno-builtin
> >
> > Thanks for the patch; in general LGTM. Just some small questions:
> >
> > Looks like -shared and -fno-common are linker flags forwarded along,
> > but -fno-builtin is meant for the compiler, right?
> 
> Definitely, -shared must be passed to the linker
> to create a shared object.
> It is listed in the linker option list:
> https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Link-Options.html#Link-Options
> 
> 
> 
> I think -fno-common is a compiler flag instead of a linker one
> as far as I understand this:
> https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Code-Gen-Options.html#Code-Gen-Options
> 
> 
> -fno-builtin is a compiler flag:
> https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/C-Dialect-Options.html#C-Dialect-Options
> 
> 
> 
> > Do we want to keep
> > that in ccflags?
> 
> All source files in arch/arm64/kernel/vdso/
> are written in assembly (gettimeofday.S note.S sigreturn.S).
> 
> So, -fno-common -fno-builtin for ccflags-y
> are not used.
> 
> That's why I dropped them.
> 
> If we want to keep them just in case,
> it is fine with me too.

I think dropping this is fine for now, but be aware that there is ongoing
work to rewrite the vdso in C, in which case we may want that back. Much
of the Makefile will probably get reworked when that happens anyway, so
no need to worry.

> >  I'm not sure why it would have been added
> > intentionally; maybe Will knows?  Maybe it's ok to drop.
> 
> They have been here since the initial VDSO support
> by commit 9031fefde6f2a.
> 
> Will, do you remember why you added these flags?

Shamelessly lifted from ppc :)

> > > -ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
> > > -               $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
> > > +ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 \
> > > +               $(call ld-option, --hash-style=sysv) -n -T
> >
> > Forgive my ignorance, but the man page for ld makes it looks like the
> > -T argument requires a subsequent argument `scriptfile`.  Is that what
> > `$(real-prereqs)` was doing below? If so, is dropping it in this patch
> > intentional?
> 
> I just re-used cmd_ld in scripts/Makefile.lib
> 
> 
> It will be expanded like follows:
> 
> cmd_ld = $(LD) $(ld_flags) $(real-prereqs) -o $@
> 
>  ->
> 
>  $(LD) $(KBUILD_LDFLAGS) $(ldflags-y) $(LDFLAGS_$(@F)) $(real-prereqs) -o $@
> 
>  ->
> 
> $(LD) -shared -nostdlib -soname=linux-vdso.so.1 \
>      --hash-style=sysv -n -T \
>     $(obj)/vdso.lds $(obj-vdso) -o $(obj)/vdso.so.dbg

Yeah, this still works, but boy is it ugly to have that dangling -T and
then grabbing the linker script from extra-y! That's also my fault though,
so I think your patch is ok.

Will

Re: [PATCH] arm64: vdso: use $(LD) instead of $(CC) to link VDSO

[Nick Desaulniers]

On Fri, Apr 12, 2019 at 1:50 AM Will Deacon <will.deacon@arm.com> wrote:
>
> Hi all,
>
> On Fri, Apr 12, 2019 at 10:21:38AM +0900, Masahiro Yamada wrote:
> > On Fri, Apr 12, 2019 at 3:20 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > > On Thu, Apr 11, 2019 at 2:30 AM Masahiro Yamada
> > > <yamada.masahiro@socionext.com> wrote:
> > > >
> > > > We use $(LD) to link vmlinux, modules, decompressors, etc.
> > > >
> > > > VDSO is the only exceptional case where $(CC) is used as the linker
> > > > driver, but I do not know why we need to do so. VDSO uses a special
> > > > linker script, and does not link standard libraries at all.
> > > >
> > > > I changed the Makefile to use $(LD) rather than $(CC). I tested this,
> > > > and VDSO worked for me.
> > > >
> > > > Users will be able to use their favorite linker (e.g. lld instead of
> > > > of bfd) by passing LD= from the command line.
> > > >
> > > > My plan is to rewrite all VDSO Makefiles to use $(LD), then delete
> > > > cc-ldoption.
> > > >
> > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > > ---
> > > >
> > > >  arch/arm64/kernel/vdso/Makefile | 13 +++----------
> > > >  1 file changed, 3 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> > > > index a0af6bf6c11b..744b9dbaba03 100644
> > > > --- a/arch/arm64/kernel/vdso/Makefile
> > > > +++ b/arch/arm64/kernel/vdso/Makefile
> > > > @@ -12,17 +12,12 @@ obj-vdso := gettimeofday.o note.o sigreturn.o
> > > >  targets := $(obj-vdso) vdso.so vdso.so.dbg
> > > >  obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
> > > >
> > > > -ccflags-y := -shared -fno-common -fno-builtin
> > >
> > > Thanks for the patch; in general LGTM. Just some small questions:
> > >
> > > Looks like -shared and -fno-common are linker flags forwarded along,
> > > but -fno-builtin is meant for the compiler, right?
> >
> > Definitely, -shared must be passed to the linker
> > to create a shared object.
> > It is listed in the linker option list:
> > https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Link-Options.html#Link-Options
> >
> >
> >
> > I think -fno-common is a compiler flag instead of a linker one
> > as far as I understand this:
> > https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Code-Gen-Options.html#Code-Gen-Options
> >
> >
> > -fno-builtin is a compiler flag:
> > https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/C-Dialect-Options.html#C-Dialect-Options
> >
> >
> >
> > > Do we want to keep
> > > that in ccflags?
> >
> > All source files in arch/arm64/kernel/vdso/
> > are written in assembly (gettimeofday.S note.S sigreturn.S).
> >
> > So, -fno-common -fno-builtin for ccflags-y
> > are not used.
> >
> > That's why I dropped them.
> >
> > If we want to keep them just in case,
> > it is fine with me too.
>
> I think dropping this is fine for now, but be aware that there is ongoing
> work to rewrite the vdso in C, in which case we may want that back. Much
> of the Makefile will probably get reworked when that happens anyway, so
> no need to worry.
>
> > >  I'm not sure why it would have been added
> > > intentionally; maybe Will knows?  Maybe it's ok to drop.
> >
> > They have been here since the initial VDSO support
> > by commit 9031fefde6f2a.
> >
> > Will, do you remember why you added these flags?
>
> Shamelessly lifted from ppc :)
>
> > > > -ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
> > > > -               $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
> > > > +ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 \
> > > > +               $(call ld-option, --hash-style=sysv) -n -T
> > >
> > > Forgive my ignorance, but the man page for ld makes it looks like the
> > > -T argument requires a subsequent argument `scriptfile`.  Is that what
> > > `$(real-prereqs)` was doing below? If so, is dropping it in this patch
> > > intentional?
> >
> > I just re-used cmd_ld in scripts/Makefile.lib
> >
> >
> > It will be expanded like follows:
> >
> > cmd_ld = $(LD) $(ld_flags) $(real-prereqs) -o $@
> >
> >  ->
> >
> >  $(LD) $(KBUILD_LDFLAGS) $(ldflags-y) $(LDFLAGS_$(@F)) $(real-prereqs) -o $@
> >
> >  ->
> >
> > $(LD) -shared -nostdlib -soname=linux-vdso.so.1 \
> >      --hash-style=sysv -n -T \
> >     $(obj)/vdso.lds $(obj-vdso) -o $(obj)/vdso.so.dbg
>
> Yeah, this still works, but boy is it ugly to have that dangling -T and
> then grabbing the linker script from extra-y! That's also my fault though,
> so I think your patch is ok.
>
> Will

Will, thanks for the additional context.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers