linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
@ 2019-03-15 19:54 Matthias Kaehlcke
  2019-03-15 21:31 ` Nick Desaulniers
  2019-03-20 13:55 ` Masahiro Yamada
  0 siblings, 2 replies; 12+ messages in thread
From: Matthias Kaehlcke @ 2019-03-15 19:54 UTC (permalink / raw)
  To: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H . Peter Anvin
  Cc: x86, linux-kernel, Nick Desaulniers, Manoj Gupta, Tiancong Wang,
	Stephen Hines, clang-built-linux, Matthias Kaehlcke

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.

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)
+
 $(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>
 #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
-- 
2.21.0.360.g471c308f928-goog

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-15 19:54 [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO Matthias Kaehlcke
@ 2019-03-15 21:31 ` Nick Desaulniers
  2019-03-15 22:29   ` Matthias Kaehlcke
  2019-03-20 14:06   ` Masahiro Yamada
  2019-03-20 13:55 ` Masahiro Yamada
  1 sibling, 2 replies; 12+ messages in thread
From: Nick Desaulniers @ 2019-03-15 21:31 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H . Peter Anvin, x86, LKML, Manoj Gupta, Tiancong Wang,
	Stephen Hines, clang-built-linux, Masahiro Yamada

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?

Is there a config I can set to reproduce this, in order to help test?

>
> 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).

> +
>  $(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?

>  #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
> --
> 2.21.0.360.g471c308f928-goog

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,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-15 21:31 ` Nick Desaulniers
@ 2019-03-15 22:29   ` Matthias Kaehlcke
  2019-03-15 23:18     ` hpa
  2019-03-18 17:09     ` Nick Desaulniers
  2019-03-20 14:06   ` Masahiro Yamada
  1 sibling, 2 replies; 12+ messages in thread
From: Matthias Kaehlcke @ 2019-03-15 22:29 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H . Peter Anvin, x86, LKML, Manoj Gupta, Tiancong Wang,
	Stephen Hines, clang-built-linux, Masahiro Yamada

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-15 22:29   ` Matthias Kaehlcke
@ 2019-03-15 23:18     ` hpa
  2019-03-18  8:59       ` Peter Zijlstra
  2019-03-18 17:09     ` Nick Desaulniers
  1 sibling, 1 reply; 12+ messages in thread
From: hpa @ 2019-03-15 23:18 UTC (permalink / raw)
  To: Matthias Kaehlcke, Nick Desaulniers
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	x86, LKML, Manoj Gupta, Tiancong Wang, Stephen Hines,
	clang-built-linux, Masahiro Yamada

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-15 23:18     ` hpa
@ 2019-03-18  8:59       ` Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2019-03-18  8:59 UTC (permalink / raw)
  To: hpa
  Cc: Matthias Kaehlcke, Nick Desaulniers, Andy Lutomirski,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, LKML,
	Manoj Gupta, Tiancong Wang, Stephen Hines, clang-built-linux,
	Masahiro Yamada

On Fri, Mar 15, 2019 at 04:18:31PM -0700, hpa@zytor.com wrote:
> Note: it is also probably no reason to use -Os/-Oz for the vdso.

Is anyone actually using -Os and CONFIG_CC_OPTIMIZE_FOR_SIZE ? I've been
staring at compiler output a lot lately and -Os really generates
atrocious crap.

That is, should we just get rid of it?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-15 22:29   ` Matthias Kaehlcke
  2019-03-15 23:18     ` hpa
@ 2019-03-18 17:09     ` Nick Desaulniers
  2019-03-18 17:16       ` Peter Zijlstra
  1 sibling, 1 reply; 12+ messages in thread
From: Nick Desaulniers @ 2019-03-18 17:09 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H . Peter Anvin, x86, LKML, Manoj Gupta, Tiancong Wang,
	Stephen Hines, clang-built-linux, Masahiro Yamada

On Fri, Mar 15, 2019 at 3:29 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> On Fri, Mar 15, 2019 at 02:31:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> > 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.

defconfig selects CONFIG_OPTIMIZE_FOR_PERFORMANCE=y.  I needed to
select CONFIG_CC_OPTIMIZE_FOR_SIZE=y to repro.  My CI is simply
running defconfigs, which is why I didn't catch this.

> Note that you probably need a development version
> of clang to reproduce this.

Yep.  Thanks for the patch.

Tested-by: Nick Desaulniers <ndesaulniers@google.com>
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-18 17:09     ` Nick Desaulniers
@ 2019-03-18 17:16       ` Peter Zijlstra
  2019-03-18 20:44         ` Matthias Kaehlcke
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2019-03-18 17:16 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Matthias Kaehlcke, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H . Peter Anvin, x86, LKML, Manoj Gupta,
	Tiancong Wang, Stephen Hines, clang-built-linux, Masahiro Yamada

On Mon, Mar 18, 2019 at 10:09:37AM -0700, Nick Desaulniers wrote:
> On Fri, Mar 15, 2019 at 3:29 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> > On Fri, Mar 15, 2019 at 02:31:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> > > 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.
> 
> defconfig selects CONFIG_OPTIMIZE_FOR_PERFORMANCE=y.  I needed to
> select CONFIG_CC_OPTIMIZE_FOR_SIZE=y to repro.  My CI is simply
> running defconfigs, which is why I didn't catch this.

I suspect the right fix is to make clang use -Os and abolish that -Oz
madness.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-18 17:16       ` Peter Zijlstra
@ 2019-03-18 20:44         ` Matthias Kaehlcke
  2019-03-18 23:03           ` Peter Zijlstra
  0 siblings, 1 reply; 12+ messages in thread
From: Matthias Kaehlcke @ 2019-03-18 20:44 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H . Peter Anvin, x86, LKML, Manoj Gupta,
	Tiancong Wang, Stephen Hines, clang-built-linux, Masahiro Yamada

Hi Peter,

On Mon, Mar 18, 2019 at 06:16:04PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 18, 2019 at 10:09:37AM -0700, Nick Desaulniers wrote:
> > On Fri, Mar 15, 2019 at 3:29 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> > > On Fri, Mar 15, 2019 at 02:31:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> > > > 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.
> > 
> > defconfig selects CONFIG_OPTIMIZE_FOR_PERFORMANCE=y.  I needed to
> > select CONFIG_CC_OPTIMIZE_FOR_SIZE=y to repro.  My CI is simply
> > running defconfigs, which is why I didn't catch this.
> 
> I suspect the right fix is to make clang use -Os and abolish that -Oz
> madness.

You have a point, enabling -Oz (aggressive optimization for size)
doesn't seem a good choice for the entire kernel.

I'll send a patch to change it (back) to -Os.

Matthias

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-18 20:44         ` Matthias Kaehlcke
@ 2019-03-18 23:03           ` Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2019-03-18 23:03 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Nick Desaulniers, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H . Peter Anvin, x86, LKML, Manoj Gupta,
	Tiancong Wang, Stephen Hines, clang-built-linux, Masahiro Yamada

On Mon, Mar 18, 2019 at 01:44:35PM -0700, Matthias Kaehlcke wrote:
> You have a point, enabling -Oz (aggressive optimization for size)
> doesn't seem a good choice for the entire kernel.

Note that in this instance, -Os actually generates _smaller_ code than
-Oz, now that's mostly because of stupid register shuffling, but still.

> I'll send a patch to change it (back) to -Os.

Thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-15 19:54 [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO Matthias Kaehlcke
  2019-03-15 21:31 ` Nick Desaulniers
@ 2019-03-20 13:55 ` Masahiro Yamada
  2019-03-20 15:45   ` Matthias Kaehlcke
  1 sibling, 1 reply; 12+ messages in thread
From: Masahiro Yamada @ 2019-03-20 13:55 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H . Peter Anvin, X86 ML, Linux Kernel Mailing List,
	Nick Desaulniers, Manoj Gupta, Tiancong Wang, Stephen Hines,
	clang-built-linux

On Sat, Mar 16, 2019 at 4:55 AM 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.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> 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>
>  #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


I picked up
https://patchwork.kernel.org/patch/10858565/



I do not know if this patch is still needed.

FYI, just a tip to turn EXPORT_SYMBOL into no-op cleanly.

You can use __DISABLE_EXPORTS
as drivers/firmware/efi/libstub/Makefile does.

See commit f922c4abdf76.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-15 21:31 ` Nick Desaulniers
  2019-03-15 22:29   ` Matthias Kaehlcke
@ 2019-03-20 14:06   ` Masahiro Yamada
  1 sibling, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2019-03-20 14:06 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Matthias Kaehlcke, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H . Peter Anvin, X86 ML, LKML, Manoj Gupta,
	Tiancong Wang, Stephen Hines, clang-built-linux

On Sat, Mar 16, 2019 at 6:32 AM Nick Desaulniers
<ndesaulniers@google.com> 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?
>
> Is there a config I can set to reproduce this, in order to help test?
>
> >
> > 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).


This is a copy from scripts/Makefile.build,
which is globally used to compile %.c -> %.o



--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO
  2019-03-20 13:55 ` Masahiro Yamada
@ 2019-03-20 15:45   ` Matthias Kaehlcke
  0 siblings, 0 replies; 12+ messages in thread
From: Matthias Kaehlcke @ 2019-03-20 15:45 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H . Peter Anvin, X86 ML, Linux Kernel Mailing List,
	Nick Desaulniers, Manoj Gupta, Tiancong Wang, Stephen Hines,
	clang-built-linux

On Wed, Mar 20, 2019 at 10:55:06PM +0900, Masahiro Yamada wrote:
> On Sat, Mar 16, 2019 at 4:55 AM 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.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > 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>
> >  #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
> 
> 
> I picked up
> https://patchwork.kernel.org/patch/10858565/

Great, thanks!

> I do not know if this patch is still needed.

no, with the change to -Os the patch isn't needed anymore.

> FYI, just a tip to turn EXPORT_SYMBOL into no-op cleanly.
> 
> You can use __DISABLE_EXPORTS
> as drivers/firmware/efi/libstub/Makefile does.
> 
> See commit f922c4abdf76.

Good to know, thanks for the pointer!

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-03-20 15:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-15 19:54 [PATCH] x86/vdso: include generic __lshrdi3 in 32-bit vDSO Matthias Kaehlcke
2019-03-15 21:31 ` Nick Desaulniers
2019-03-15 22:29   ` Matthias Kaehlcke
2019-03-15 23:18     ` hpa
2019-03-18  8:59       ` Peter Zijlstra
2019-03-18 17:09     ` Nick Desaulniers
2019-03-18 17:16       ` Peter Zijlstra
2019-03-18 20:44         ` Matthias Kaehlcke
2019-03-18 23:03           ` Peter Zijlstra
2019-03-20 14:06   ` Masahiro Yamada
2019-03-20 13:55 ` Masahiro Yamada
2019-03-20 15:45   ` Matthias Kaehlcke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).