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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> ---
>
>  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