s390: Move -msoft-float into compel Makefiles

Submitted by Michael Holzheu on July 20, 2017, 6:36 p.m.

Details

Message ID 20170720203650.7b2af87c@TP-holzheu
State New
Headers show

Commit Message

Michael Holzheu July 20, 2017, 6:36 p.m.
We currently define CFLAGS+=-msoft-float in Makefile.compel.

Makefile.compel is included into the toplevel Makefile and so changes
CFLAGS which are exported to all criu and zdtm Makefiles.

We must not use -msoft-float outside the compel files. E.g. otherwise for
zdtm we get the following build error:

uptime_grow.o: In function `main':
/tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__floatdidf'
/tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__muldf3'
/tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__floatdidf'
/tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__adddf3

Fix this and move the CFLAGS definition to the compel Makefile.

As compel/plugins/Makefile and compel/Makefile are called recursively,
changed CFLAGS will not be exported back to the top Makefile.

Reported-by: Adrian Reber <areber@redhat.com>
Reviewed-by: Dmitry Safonov <dsafonov@virtuozzo.com>
Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 Makefile.compel         | 10 ----------
 compel/Makefile         | 14 ++++++++++++++
 compel/plugins/Makefile | 14 ++++++++++++++
 3 files changed, 28 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Makefile.compel b/Makefile.compel
index 5c854e3..1ef7f8c 100644
--- a/Makefile.compel
+++ b/Makefile.compel
@@ -70,13 +70,3 @@  compel/$(LIBCOMPEL_SO): compel/$(LIBCOMPEL_A)
 compel-install-targets	+= compel/$(LIBCOMPEL_SO)
 compel-install-targets	+= compel/compel
 compel-install-targets	+= $(compel-plugins)
-
-#
-# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
-# (Global Offset Table) relocations with gcc compilers that don't have
-# commit "S/390: Fix 64 bit sibcall".
-#
-ifeq ($(ARCH),s390)
-CFLAGS += -msoft-float -fno-optimize-sibling-calls
-HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls
-endif
diff --git a/compel/Makefile b/compel/Makefile
index 43d27f5..81b9614 100644
--- a/compel/Makefile
+++ b/compel/Makefile
@@ -34,6 +34,20 @@  CFLAGS			+= -DNO_RELOCS
 HOSTCFLAGS		+= -DNO_RELOCS
 endif
 
+#
+# We assume that compel code does not change floating point registers.
+# On s390 gcc uses fprs to cache gprs. Therefore disable floating point
+# with -msoft-float.
+#
+# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
+# (Global Offset Table) relocations with gcc compilers that don't have
+# commit "S/390: Fix 64 bit sibcall".
+#
+ifeq ($(ARCH),s390)
+CFLAGS += -msoft-float -fno-optimize-sibling-calls
+HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls
+endif
+
 obj-y			+= src/main.o
 obj-y			+= arch/$(ARCH)/src/lib/handle-elf.o
 obj-y			+= src/lib/handle-elf.o
diff --git a/compel/plugins/Makefile b/compel/plugins/Makefile
index 7127425..948348f 100644
--- a/compel/plugins/Makefile
+++ b/compel/plugins/Makefile
@@ -8,6 +8,20 @@  PLUGIN_ARCH_DIR		:= compel/arch/$(ARCH)/plugins
 # CFLAGS, ASFLAGS, LDFLAGS
 
 #
+# We assume that compel code does not change floating point registers.
+# On s390 gcc uses fprs to cache gprs. Therefore disable floating point
+# with -msoft-float.
+#
+# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
+# (Global Offset Table) relocations with gcc compilers that don't have
+# commit "S/390: Fix 64 bit sibcall".
+#
+ifeq ($(ARCH),s390)
+CFLAGS += -msoft-float -fno-optimize-sibling-calls
+HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls
+endif
+
+#
 # UAPI inclusion, referred as <compel/...>
 ccflags-y		+= -I compel/include/uapi
 asflags-y		+= -I compel/include/uapi

Comments

Adrian Reber July 21, 2017, 9:08 a.m.
Sorry, but that does not work for me:

gcc -c -O2 -g -Wall -Wformat-security -Werror -DCONFIG_S390 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DCONFIG_HAS_SELINUX -iquote include/ -DCONFIG_HAS_SELINUX -Wstrict-prototypes -fno-stack-protector -nostdlib -fomit-frame-pointer -fpie -I ./compel/include/uapi -fno-strict-aliasing -iquote criu/include -iquote include -iquote images -iquote criu/arch/s390/include -iquote . -I/usr/include/libnl3 -I ./compel/include/uapi -DGLOBAL_CONFIG_DIR='"/etc/criu/"' -DDEFAULT_CONFIG_FILENAME='"default.conf"' -DUSER_CONFIG_DIR='".criu/"' -I ./compel/include/uapi -DCR_NOGLIBC -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=0 -msoft-float criu/arch/s390/restorer.c -o criu/arch/s390/restorer.o
gcc -c -O2 -g -Wall -Wformat-security -Werror -DCONFIG_S390 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DCONFIG_HAS_SELINUX -iquote include/ -DCONFIG_HAS_SELINUX -Wstrict-prototypes -fno-stack-protector -nostdlib -fomit-frame-pointer -fpie -I ./compel/include/uapi -fno-strict-aliasing -iquote criu/include -iquote include -iquote images -iquote criu/arch/s390/include -iquote . -I/usr/include/libnl3 -I ./compel/include/uapi -DGLOBAL_CONFIG_DIR='"/etc/criu/"' -DDEFAULT_CONFIG_FILENAME='"default.conf"' -DUSER_CONFIG_DIR='".criu/"' -I ./compel/include/uapi -DCR_NOGLIBC -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=0 -msoft-float criu/pie/restorer.c -o criu/pie/restorer.o
ld  -r -z noexecstack -T ./compel/arch/s390/scripts/compel-pack.lds.S  -o criu/pie/restorer.built-in.o  ./criu/arch/s390/restorer.o criu/pie/restorer.o criu/pie/pie.lib.a ./compel/plugins/std.lib.a
./compel/compel-host hgen -f criu/pie/restorer.built-in.o -o criu/pie/restorer-blob.h
Error (compel/src/lib/handle-elf-host.c:572): Unsupported relocation of type 26
make[2]: *** [criu/pie/restorer-blob.h] Error 255
make[1]: *** [pie] Error 2
make: *** [criu] Error 2

		Adrian

On Thu, Jul 20, 2017 at 08:36:50PM +0200, Michael Holzheu wrote:
> We currently define CFLAGS+=-msoft-float in Makefile.compel.
> 
> Makefile.compel is included into the toplevel Makefile and so changes
> CFLAGS which are exported to all criu and zdtm Makefiles.
> 
> We must not use -msoft-float outside the compel files. E.g. otherwise for
> zdtm we get the following build error:
> 
> uptime_grow.o: In function `main':
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__floatdidf'
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__muldf3'
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__floatdidf'
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__adddf3
> 
> Fix this and move the CFLAGS definition to the compel Makefile.
> 
> As compel/plugins/Makefile and compel/Makefile are called recursively,
> changed CFLAGS will not be exported back to the top Makefile.
> 
> Reported-by: Adrian Reber <areber@redhat.com>
> Reviewed-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  Makefile.compel         | 10 ----------
>  compel/Makefile         | 14 ++++++++++++++
>  compel/plugins/Makefile | 14 ++++++++++++++
>  3 files changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/Makefile.compel b/Makefile.compel
> index 5c854e3..1ef7f8c 100644
> --- a/Makefile.compel
> +++ b/Makefile.compel
> @@ -70,13 +70,3 @@ compel/$(LIBCOMPEL_SO): compel/$(LIBCOMPEL_A)
>  compel-install-targets	+= compel/$(LIBCOMPEL_SO)
>  compel-install-targets	+= compel/compel
>  compel-install-targets	+= $(compel-plugins)
> -
> -#
> -# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
> -# (Global Offset Table) relocations with gcc compilers that don't have
> -# commit "S/390: Fix 64 bit sibcall".
> -#
> -ifeq ($(ARCH),s390)
> -CFLAGS += -msoft-float -fno-optimize-sibling-calls
> -HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls
> -endif
> diff --git a/compel/Makefile b/compel/Makefile
> index 43d27f5..81b9614 100644
> --- a/compel/Makefile
> +++ b/compel/Makefile
> @@ -34,6 +34,20 @@ CFLAGS			+= -DNO_RELOCS
>  HOSTCFLAGS		+= -DNO_RELOCS
>  endif
>  
> +#
> +# We assume that compel code does not change floating point registers.
> +# On s390 gcc uses fprs to cache gprs. Therefore disable floating point
> +# with -msoft-float.
> +#
> +# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
> +# (Global Offset Table) relocations with gcc compilers that don't have
> +# commit "S/390: Fix 64 bit sibcall".
> +#
> +ifeq ($(ARCH),s390)
> +CFLAGS += -msoft-float -fno-optimize-sibling-calls
> +HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls
> +endif
> +
>  obj-y			+= src/main.o
>  obj-y			+= arch/$(ARCH)/src/lib/handle-elf.o
>  obj-y			+= src/lib/handle-elf.o
> diff --git a/compel/plugins/Makefile b/compel/plugins/Makefile
> index 7127425..948348f 100644
> --- a/compel/plugins/Makefile
> +++ b/compel/plugins/Makefile
> @@ -8,6 +8,20 @@ PLUGIN_ARCH_DIR		:= compel/arch/$(ARCH)/plugins
>  # CFLAGS, ASFLAGS, LDFLAGS
>  
>  #
> +# We assume that compel code does not change floating point registers.
> +# On s390 gcc uses fprs to cache gprs. Therefore disable floating point
> +# with -msoft-float.
> +#
> +# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
> +# (Global Offset Table) relocations with gcc compilers that don't have
> +# commit "S/390: Fix 64 bit sibcall".
> +#
> +ifeq ($(ARCH),s390)
> +CFLAGS += -msoft-float -fno-optimize-sibling-calls
> +HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls
> +endif
> +
> +#
>  # UAPI inclusion, referred as <compel/...>
>  ccflags-y		+= -I compel/include/uapi
>  asflags-y		+= -I compel/include/uapi
> -- 
> 2.7.4
>
Michael Holzheu July 21, 2017, 9:29 a.m.
Am Thu, 20 Jul 2017 23:52:00 -0000
schrieb Patchwork <criupatchwork@gmail.com>:

> == Series Details ==
> 
> Series: s390: Move -msoft-float into compel Makefiles
> URL   : https://patchwork.criu.org/series/1809/
> State : failure
> 
> == Logs ==
> 
> For more details see: https://travis-ci.org/criupatchwork/criu/builds/255781476?utm_source=github_status&utm_medium=notification
> 

At least travis seems to work now most of the time for s390 :)

   GEN      criu/pie/restorer-blob.h

 Error (compel/src/lib/handle-elf-host.c:333): Unexpected undefined symbol: `_GLOBAL_OFFSET_TABLE_'. External symbol in PIE?

 criu/pie/Makefile:56: recipe for target 'criu/pie/restorer-blob.h' failed

I missed to add -fno-optimize-sibling-calls to:

 - criu/pie/Makefile
 - criu/pie/Makefile.library

Sorry for that - I will send a v2 patch.

Michael