On Fri, Apr 12, 2019 at 1:50 AM Will Deacon <will.dea...@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 > > <ndesaulni...@google.com> wrote: > > > On Thu, Apr 11, 2019 at 2:30 AM Masahiro Yamada > > > <yamada.masah...@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.masah...@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 <ndesaulni...@google.com> -- Thanks, ~Nick Desaulniers
