Messages in this thread | | | Date | Fri, 15 Mar 2019 16:18:31 -0700 | Subject | Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO | From | hpa@zytor ... |
| |
On March 15, 2019 3:29:06 PM PDT, Matthias Kaehlcke <mka@chromium.org> wrote: >Hi Nick, > >On Fri, Mar 15, 2019 at 02:31:09PM -0700, 'Nick Desaulniers' via Clang >Built Linux wrote: >> On Fri, Mar 15, 2019 at 12:54 PM Matthias Kaehlcke <mka@chromium.org> >wrote: >> > >> > Building the 32-bit vDSO with a recent clang version fails due >> > to undefined symbols: >> > >> > arch/x86/entry/vdso/vdso32.so.dbg: undefined symbols found >> > >> > The undefined symbol in this case is __lshrdi3, which is part of >> > the compiler runtime library, however the vDSO isn't linked against >> > this library. >> > >> > Include the kernel version of __lshrdi3 in the 32-bit vDSO build. >> >> __lshrdi3 is used for "logical shift right double-word by int" (best >> guess), so anywhere there's a right shift of a u64. Looks like >> there's a few of these in arch/x86/entry/vdso/, so it's legal for the >> compiler to emit this libcall. Do you know which function >> specifically in the .so has a relocation referencing __lshrdi3 >> specifically? > >It's the right shifts in do_realtime() and do_monotonic(). > >> Is there a config I can set to reproduce this, in order to help >> test? > >I encountered it with a Chrome OS specific configuration, but a >defconfig should do. Note that you probably need a development version >of clang to reproduce this. > >> > >> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> >> > --- >> > arch/x86/entry/vdso/Makefile | 7 ++++++- >> > lib/lshrdi3.c | 4 +++- >> > 2 files changed, 9 insertions(+), 2 deletions(-) >> > >> > diff --git a/arch/x86/entry/vdso/Makefile >b/arch/x86/entry/vdso/Makefile >> > index 5bfe2243a08f..7517cd87e10b 100644 >> > --- a/arch/x86/entry/vdso/Makefile >> > +++ b/arch/x86/entry/vdso/Makefile >> > @@ -144,6 +144,7 @@ KBUILD_CFLAGS_32 += $(call cc-option, >-fno-stack-protector) >> > KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls) >> > KBUILD_CFLAGS_32 += -fno-omit-frame-pointer >> > KBUILD_CFLAGS_32 += -DDISABLE_BRANCH_PROFILING >> > +KBUILD_CFLAGS_32 += -DBUILD_VDSO >> > >> > ifdef CONFIG_RETPOLINE >> > ifneq ($(RETPOLINE_VDSO_CFLAGS),) >> > @@ -153,12 +154,16 @@ endif >> > >> > $(obj)/vdso32.so.dbg: KBUILD_CFLAGS = $(KBUILD_CFLAGS_32) >> > >> > +$(obj)/vdso32/lshrdi3.o: $(srctree)/lib/lshrdi3.c FORCE >> > + $(call if_changed_rule,cc_o_c) >> >> + Masahiro to help look at this part (I don't understand this part >> of kbuild). > >I bluntly stole that from arch/x86/purgatory/Makefile , which does >something similar. > >> >> > + >> > $(obj)/vdso32.so.dbg: FORCE \ >> > $(obj)/vdso32/vdso32.lds \ >> > $(obj)/vdso32/vclock_gettime.o \ >> > $(obj)/vdso32/note.o \ >> > $(obj)/vdso32/system_call.o \ >> > - $(obj)/vdso32/sigreturn.o >> > + $(obj)/vdso32/sigreturn.o \ >> > + $(obj)/vdso32/lshrdi3.o >> > $(call if_changed,vdso) >> > >> > # >> > diff --git a/lib/lshrdi3.c b/lib/lshrdi3.c >> > index 99cfa5721f2d..8a4fc6bcf3a4 100644 >> > --- a/lib/lshrdi3.c >> > +++ b/lib/lshrdi3.c >> > @@ -16,7 +16,7 @@ >> > * to the Free Software Foundation, Inc. >> > */ >> > >> > -#include <linux/module.h> >> > +#include <linux/export.h> >> >> Is this a simple cleanup, or? > >The vDSO build is unhappy when modules.h draws in a whole bunch of >other kernel headers and export.h is all that's need. It seemed >reasonable to do the 'cleanup' in this patch since we touch it anyway >to place EXPORT_SYMBOL within an #ifdef. > >> > #include <linux/libgcc.h> >> > >> > long long notrace __lshrdi3(long long u, word_type b) >> > @@ -42,4 +42,6 @@ long long notrace __lshrdi3(long long u, >word_type b) >> > >> > return w.ll; >> > } >> > +#ifndef BUILD_VDSO >> > EXPORT_SYMBOL(__lshrdi3); >> > +#endif >> >> Compilers (GCC and Clang) will always assume their runtime has these >> helper functions; whether or not they emit libcalls vs inline >routines >> is implementation defined. So I agree with this patch; I just would >> like to help confirm/test it. > >Thanks for your help! > >Matthias
Note: it is also probably no reason to use -Os/-Oz for the vdso. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
| |