zdtm: fix building uptime_grow.c on s390

Submitted by Michael Holzheu on July 20, 2017, 5:34 p.m.

Details

Message ID 20170720193433.639b9803@TP-holzheu
State New
Series "zdtm: fix building uptime_grow.c on s390"
Headers show

Commit Message

Michael Holzheu July 20, 2017, 5:34 p.m.
Am Thu, 20 Jul 2017 18:42:33 +0200
schrieb Adrian Reber <areber@redhat.com>:

> On Thu, Jul 20, 2017 at 06:13:02PM +0200, Michael Holzheu wrote:
> > Am Wed, 19 Jul 2017 21:08:02 +0200
> > schrieb Adrian Reber <areber@redhat.com>:
> > 
> > > On Wed, Jul 19, 2017 at 07:04:16PM +0200, Michael Holzheu wrote:
> > > > Am Wed, 19 Jul 2017 13:40:36 +0000
> > > > schrieb Adrian Reber <adrian@lisas.de>:
> > > > 
> > > > > From: Adrian Reber <areber@redhat.com>
> > > > > 
> > > > > Building the uptime_grow test case fails on s390 with:
> > > > > 
> > > > > 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'
> > > > 
> > > > These look like soft-float symbols. Normally you should not need to
> > > > include <math.h> in this case.
> > > > 
> > > > Could you please send the V=1 compile output?
> > > 
> > > gcc -O2 -g  -Wall -Wformat-security -Werror -DCONFIG_S390 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DCONFIG_HAS_SELINUX -iquote include/ -DCONFIG_HAS_SELINUX -msoft-float -fno-optimize-sibling-calls -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -MM -MP -o uptime_grow.d uptime_grow.c
> > > gcc -O2 -g  -Wall -Wformat-security -Werror -DCONFIG_S390 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DCONFIG_HAS_SELINUX -iquote include/ -DCONFIG_HAS_SELINUX -msoft-float -fno-optimize-sibling-calls -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -o uptime_grow.o uptime_grow.c
> > 
> > The testsuite should *not* be compiled with the "-msoft-float" option.
> > 
> > When I do "make V=1 -C test/zdtm" on the current "criu-dev" branch I get the following:
> > 
> > # gcc -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -o uptime_grow.o uptime_grow.c
> > 
> > So no "-msoft-float" ...
> > 
> > The question is: What is different on your system?
> 
> So, this one is a bit strange. If I checkout criu-dev and run make and
> make zdtm (to just build all test cases) I get the error above.
> 
> If I do make; cd test; ./zdtm.py run -t zdtm/static/uptime_grow
> I see
> 
>  CC        uptime_grow.o
>  LINK      uptime_grow
> 
> No errors, it just builds and works. So building test cases from the top
> level directory is different than build test cases from the test
> directory:
> 
> # make -C zdtm/static/ uptime_grow V=1
> make: Entering directory `/tmp/2/criu/test/zdtm/static'
> make -C ../lib
> make[1]: Entering directory `/tmp/2/criu/test/zdtm/lib'
> make[1]: Leaving directory `/tmp/2/criu/test/zdtm/lib'
> gcc -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -o uptime_grow.o uptime_grow.c
> gcc  uptime_grow.o ../lib/libzdtmtst.a ../lib/libzdtmtst.a -lrt -pthread -o uptime_grow
> make: Leaving directory `/tmp/2/criu/test/zdtm/static'
> 
> 
> And my patch does not actually work. To test it I just rebuild the test
> case from the test directory and that works with and without the <math.h>
> include.
> 
> So the patch is wrong and unnecessary and the buildsystem needs a fix.

Ok got it.

The problem is that for "make zdtm" we use the toplevel Makefile
which includes Makefile.compel which defines -msoft-float in CFLAGS.

@Dmitry:

Perhaps we should not have moved -msoft-float to Makefile.compel:

https://lists.openvz.org/pipermail/criu/2017-June/038590.html

What do you think - should we move it back with the following patch?
---
Subject: [CRIU][PATCH] s390: Move -msoft-float into compel 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.

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

Dmitry Safonov July 20, 2017, 6:03 p.m.
2017-07-20 20:34 GMT+03:00 Michael Holzheu <holzheu@linux.vnet.ibm.com>:
> Am Thu, 20 Jul 2017 18:42:33 +0200
> schrieb Adrian Reber <areber@redhat.com>:
>
>> On Thu, Jul 20, 2017 at 06:13:02PM +0200, Michael Holzheu wrote:
>> > Am Wed, 19 Jul 2017 21:08:02 +0200
>> > schrieb Adrian Reber <areber@redhat.com>:
>> >
>> > > On Wed, Jul 19, 2017 at 07:04:16PM +0200, Michael Holzheu wrote:
>> > > > Am Wed, 19 Jul 2017 13:40:36 +0000
>> > > > schrieb Adrian Reber <adrian@lisas.de>:
>> > > >
>> > > > > From: Adrian Reber <areber@redhat.com>
>> > > > >
>> > > > > Building the uptime_grow test case fails on s390 with:
>> > > > >
>> > > > > 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'
>> > > >
>> > > > These look like soft-float symbols. Normally you should not need to
>> > > > include <math.h> in this case.
>> > > >
>> > > > Could you please send the V=1 compile output?
>> > >
>> > > gcc -O2 -g  -Wall -Wformat-security -Werror -DCONFIG_S390 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DCONFIG_HAS_SELINUX -iquote include/ -DCONFIG_HAS_SELINUX -msoft-float -fno-optimize-sibling-calls -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -MM -MP -o uptime_grow.d uptime_grow.c
>> > > gcc -O2 -g  -Wall -Wformat-security -Werror -DCONFIG_S390 -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DCONFIG_HAS_SELINUX -iquote include/ -DCONFIG_HAS_SELINUX -msoft-float -fno-optimize-sibling-calls -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -o uptime_grow.o uptime_grow.c
>> >
>> > The testsuite should *not* be compiled with the "-msoft-float" option.
>> >
>> > When I do "make V=1 -C test/zdtm" on the current "criu-dev" branch I get the following:
>> >
>> > # gcc -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -o uptime_grow.o uptime_grow.c
>> >
>> > So no "-msoft-float" ...
>> >
>> > The question is: What is different on your system?
>>
>> So, this one is a bit strange. If I checkout criu-dev and run make and
>> make zdtm (to just build all test cases) I get the error above.
>>
>> If I do make; cd test; ./zdtm.py run -t zdtm/static/uptime_grow
>> I see
>>
>>  CC        uptime_grow.o
>>  LINK      uptime_grow
>>
>> No errors, it just builds and works. So building test cases from the top
>> level directory is different than build test cases from the test
>> directory:
>>
>> # make -C zdtm/static/ uptime_grow V=1
>> make: Entering directory `/tmp/2/criu/test/zdtm/static'
>> make -C ../lib
>> make[1]: Entering directory `/tmp/2/criu/test/zdtm/lib'
>> make[1]: Leaving directory `/tmp/2/criu/test/zdtm/lib'
>> gcc -g -O2 -Wall -Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0  -D_GNU_SOURCE -I../lib -iquote ../lib/arch/s390/include  -c -o uptime_grow.o uptime_grow.c
>> gcc  uptime_grow.o ../lib/libzdtmtst.a ../lib/libzdtmtst.a -lrt -pthread -o uptime_grow
>> make: Leaving directory `/tmp/2/criu/test/zdtm/static'
>>
>>
>> And my patch does not actually work. To test it I just rebuild the test
>> case from the test directory and that works with and without the <math.h>
>> include.
>>
>> So the patch is wrong and unnecessary and the buildsystem needs a fix.
>
> Ok got it.
>
> The problem is that for "make zdtm" we use the toplevel Makefile
> which includes Makefile.compel which defines -msoft-float in CFLAGS.
>
> @Dmitry:
>
> Perhaps we should not have moved -msoft-float to Makefile.compel:
>
> https://lists.openvz.org/pipermail/criu/2017-June/038590.html
>
> What do you think - should we move it back with the following patch?

Ok, convinced.

Please, add also to the description the above, something like:

> ---
> Subject: [CRIU][PATCH] s390: Move -msoft-float into compel Makefiles
>

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.

>
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>

Reviewed-by: Dmitry Safonov <dsafonov@virtuozzo.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
>