compile error in master, restrict is missing

compile error in master, restrict is missing

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

I understand that grub.git#master does (most likely) compile for everyone else:

[  135s] + env CFLAGS=-Wall CXXFLAGS=-Wall FFLAGS=-Wall ../grub2-src/configure --target=i386-suse-linux --with-platform=pc --prefix=/usr/lib64/grub2-chainloader --disable-boot-time --disable-cache-stats --disable-device-mapper --disable-efiemu --disable-grub-emu-pci --disable-grub-emu-sdl --disable-grub-mkfont --disable-grub-mount --disable-grub-themes --disable-liblzma --disable-libzfs --enable-mm-debug --disable-nls --disable-werror --enable-rpath --with-bootdir=/chainloader/grub2-chainloader --enable-grub-mkfont --enable-grub-themes --with-grubdir=grub2
...
[  207s] In file included from ../grub2-src/util/grub-mkfont.c:21:0:
[  207s] ../grub2-src/include/grub/misc.h:264:64: error: expected ';', ',' or ')' before 'str'
[  207s]  unsigned long EXPORT_FUNC(grub_strtoul) (const char * restrict str, const char ** const restrict end, int base);
[  207s]                                                                 ^

Where is 'restrict' supposed to come from?

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: compile error in master, restrict is missing

[Daniel Kiper]

Hi Olaf,

CC-ing Peter...

On Wed, Mar 11, 2020 at 08:44:33AM +0100, Olaf Hering wrote:
> I understand that grub.git#master does (most likely) compile for everyone else:
>
> [  135s] + env CFLAGS=-Wall CXXFLAGS=-Wall FFLAGS=-Wall ../grub2-src/configure --target=i386-suse-linux --with-platform=pc --prefix=/usr/lib64/grub2-chainloader --disable-boot-time --disable-cache-stats --disable-device-mapper --disable-efiemu --disable-grub-emu-pci --disable-grub-emu-sdl --disable-grub-mkfont --disable-grub-mount --disable-grub-themes --disable-liblzma --disable-libzfs --enable-mm-debug --disable-nls --disable-werror --enable-rpath --with-bootdir=/chainloader/grub2-chainloader --enable-grub-mkfont --enable-grub-themes --with-grubdir=grub2
> ...
> [  207s] In file included from ../grub2-src/util/grub-mkfont.c:21:0:
> [  207s] ../grub2-src/include/grub/misc.h:264:64: error: expected ';', ',' or ')' before 'str'
> [  207s]  unsigned long EXPORT_FUNC(grub_strtoul) (const char * restrict str, const char ** const restrict end, int base);
> [  207s]                                                                 ^

Yeah, I added tons of patches yesterday evening and build tested.
Nothing suspicious pooped out. I expect that the issues you are
hitting are related to set of configure flags which you are using.
Anyway, it has to be fixed...

> Where is 'restrict' supposed to come from?

Peter, could you take a look at this?

Daniel

Re: compile error in master, restrict is missing

[John Paul Adrian Glaubitz]

On 3/11/20 11:39 AM, Daniel Kiper wrote:
>> [  135s] + env CFLAGS=-Wall CXXFLAGS=-Wall FFLAGS=-Wall ../grub2-src/configure --target=i386-suse-linux

Which version of SUSE Linux is that, in case I want to reproduce that?

FWIW, I work at SUSE, so I have access to all versions :).

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: compile error in master, restrict is missing

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

Am Wed, 11 Mar 2020 11:56:27 +0100
schrieb John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>:

> Which version of SUSE Linux is that, in case I want to reproduce that?

SLE_12 and Leap 42.3, everything with gcc-4.8. I may be able to work around it by using a newer compiler.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: compile error in master, restrict is missing

[John Paul Adrian Glaubitz]

On 3/11/20 12:01 PM, Olaf Hering wrote:
> Am Wed, 11 Mar 2020 11:56:27 +0100
> schrieb John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>:
> 
>> Which version of SUSE Linux is that, in case I want to reproduce that?
> 
> SLE_12 and Leap 42.3, everything with gcc-4.8. I may be able to work around it by using a newer compiler.

Ok, thanks. FWIW, Leap 42.3 is no longer supported by us.

I happen to have a SLE-12 cloud instance running anyway now so I can help
debug if necessary.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: compile error in master, restrict is missing

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Am Wed, 11 Mar 2020 11:39:08 +0100
schrieb Daniel Kiper <dkiper@net-space.pl>:

> Nothing suspicious pooped out. I expect that the issues you are
> hitting are related to set of configure flags which you are using.

It turned out --disable-mm-debug does for some reason fix the build with gcc7. But building with gcc48 does still trigger the failure. 

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: compile error in master, restrict is missing

[Javier Martinez Canillas]

Hello Olaf,

On 3/11/20 11:59 AM, Olaf Hering wrote:
> Am Wed, 11 Mar 2020 11:39:08 +0100
> schrieb Daniel Kiper <dkiper@net-space.pl>:
> 
>> Nothing suspicious pooped out. I expect that the issues you are
>> hitting are related to set of configure flags which you are using.
> 
> It turned out --disable-mm-debug does for some reason fix the build with gcc7. But building with gcc48 does still trigger the failure. 
> 

That's expected because unless specified, the default for GCC 4.8 is -std=gnu90
(C90 with GNU extensions) but the restrict type qualifier was introduced in C99.

Does the following patch solve the issue?

From 2e9879df9ddd4744f7b6c55bac9cf335e5832243 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@redhat.com>
Date: Wed, 11 Mar 2020 12:03:54 +0100
Subject: [PATCH] misc: Disable the restrict keyword when not building with C99

The restrict type qualifier was introduced in the C99 standard, so it will
lead to a compile error if GRUB is not built with -std=c99 or newer.

This check comes from grub-core/lib/zstd/xxhash.h file that does the same.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
 include/grub/misc.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/grub/misc.h b/include/grub/misc.h
index cd5a8b4ae82..878ac18da7e 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -27,6 +27,10 @@
 #include <grub/i18n.h>
 #include <grub/compiler.h>
 
+#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L))   /* ! C99 */
+#  define restrict   /* disable restrict */
+#endif
+
 #define ALIGN_UP(addr, align) \
 	((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align - 1))
 #define ALIGN_UP_OVERHEAD(addr, align) ((-(addr)) & ((typeof (addr)) (align) - 1))
-- 
2.24.1

Re: compile error in master, restrict is missing

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

Am Wed, 11 Mar 2020 12:30:01 +0100
schrieb Javier Martinez Canillas <fmartine@redhat.com>:

> +#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L))   

This does not help. I have seen similar code in one of the zstd headers.

Whatever the outcome will be, it should get a 'Fixes commit d5a32255d' tag.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: compile error in master, restrict is missing

[Javier Martinez Canillas]

On 3/11/20 4:41 PM, Olaf Hering wrote:
> Am Wed, 11 Mar 2020 12:30:01 +0100
> schrieb Javier Martinez Canillas <fmartine@redhat.com>:
> 
>> +#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L))   
>
> This does not help. I have seen similar code in one of the zstd headers.
> 

Yes, I mentioned in the commit message that it was taken from there...

I would had expected that change to avoid the build failure. I wonder
why it didn't help.

> Whatever the outcome will be, it should get a 'Fixes commit d5a32255d' tag.
>

Agreed.

Daniel said that instead he will add a -std=gnu99 option so the compiler
uses C99 even if is not its default.
 
> Olaf
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: compile error in master, restrict is missing

[Daniel Kiper]

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

On Wed, Mar 11, 2020 at 04:57:13PM +0100, Javier Martinez Canillas wrote:
> On 3/11/20 4:41 PM, Olaf Hering wrote:
> > Am Wed, 11 Mar 2020 12:30:01 +0100
> > schrieb Javier Martinez Canillas <fmartine@redhat.com>:
> >
> >> +#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L))
> >
> > This does not help. I have seen similar code in one of the zstd headers.
>
> Yes, I mentioned in the commit message that it was taken from there...
>
> I would had expected that change to avoid the build failure. I wonder
> why it didn't help.
>
> > Whatever the outcome will be, it should get a 'Fixes commit d5a32255d' tag.
>
> Agreed.
>
> Daniel said that instead he will add a -std=gnu99 option so the compiler
> uses C99 even if is not its default.

Olaf, could you try the patch in the attachment? Please rerun
./autogen.sh and ./configure after applying it.

Daniel

[-- Attachment #2: std.patch --]
[-- Type: text/x-diff, Size: 702 bytes --]

diff --git a/configure.ac b/configure.ac
index 88c0adbae..77175c94b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,7 +36,7 @@ AC_INIT([GRUB],[2.05],[bug-grub@gnu.org])
 AC_CONFIG_AUX_DIR([build-aux])
 
 # We don't want -g -O2 by default in CFLAGS
-: ${CFLAGS=""}
+: ${CFLAGS='-std=gnu99'}
 
 # Checks for build, host and target systems.
 AC_CANONICAL_BUILD
@@ -80,6 +80,12 @@ if test "x$TARGET_CFLAGS" = x; then
   TARGET_CFLAGS="$TARGET_CFLAGS -Os"
 fi
 
+TARGET_CFLAGS="$TARGET_CFLAGS -std=gnu99"
+
+BUILD_CFLAGS="$BUILD_CFLAGS -std=gnu99"
+
+HOST_CFLAGS="$HOST_CFLAGS -std=gnu99"
+
 # Default HOST_CPPFLAGS
 HOST_CPPFLAGS="$HOST_CPPFLAGS -Wall -W"
 HOST_CPPFLAGS="$HOST_CPPFLAGS -DGRUB_UTIL=1"

Re: compile error in master, restrict is missing

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 191 bytes --]

Am Thu, 12 Mar 2020 19:01:26 +0100
schrieb Daniel Kiper <dkiper@net-space.pl>:

> Olaf, could you try the patch in the attachment? 

Yes, this fixes the compile errors. Thanks.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: compile error in master, restrict is missing

[Daniel Kiper]

On Thu, Mar 12, 2020 at 07:20:22PM +0100, Olaf Hering wrote:
> Am Thu, 12 Mar 2020 19:01:26 +0100
> schrieb Daniel Kiper <dkiper@net-space.pl>:
>
> > Olaf, could you try the patch in the attachment?
>
> Yes, this fixes the compile errors. Thanks.

Great! So, I will post proper patch probably at the beginning of next
week. This one requires some polishing.

Olaf, in the mean time please fix your issue if possible.

Daniel