[PATCH] tools/nolibc: validate C99 compatibility

[PATCH] tools/nolibc: validate C99 compatibility

[Thomas Weißschuh]

Most of the code was migrated to C99-conformant __asm__ statements
before. It seems string.h was missed.

Fix string.h and also validate during build that nolibc stays within
C99.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---

This patch is based on the "dev" branch of the RCU tree.
---
 tools/include/nolibc/string.h                | 4 ++--
 tools/testing/selftests/nolibc/Makefile      | 2 +-
 tools/testing/selftests/nolibc/nolibc-test.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index fffdaf6ff467..0c2e06c7c477 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -90,7 +90,7 @@ void *memset(void *dst, int b, size_t len)
 
 	while (len--) {
 		/* prevent gcc from recognizing memset() here */
-		asm volatile("");
+		__asm__ volatile("");
 		*(p++) = b;
 	}
 	return dst;
@@ -139,7 +139,7 @@ size_t strlen(const char *str)
 	size_t len;
 
 	for (len = 0; str[len]; len++)
-		asm("");
+		__asm__("");
 	return len;
 }
 
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index bbce57420465..55efcb1011cb 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -83,7 +83,7 @@ CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR)
 CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR)
 CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR)
 CFLAGS_s390 = -m64
-CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables \
+CFLAGS  ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c99 \
 		$(call cc-option,-fno-stack-protector) \
 		$(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH))
 LDFLAGS := -s
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 47013b78972e..932b2c7b0ce3 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -895,7 +895,7 @@ int main(int argc, char **argv, char **envp)
 #else
 		else if (ioperm(0x501, 1, 1) == 0)
 #endif
-			asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
+			__asm__ volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
 		/* if it does nothing, fall back to the regular panic */
 #endif
 	}

---
base-commit: a5333c037de823912dd20e933785c63de7679e64
change-id: 20230328-nolibc-c99-59f44ea45636

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

Re: [PATCH] tools/nolibc: validate C99 compatibility

[Willy Tarreau]

Hi Thomas,

On Tue, Mar 28, 2023 at 09:07:35PM +0000, Thomas Weißschuh wrote:
> Most of the code was migrated to C99-conformant __asm__ statements
> before. It seems string.h was missed.
> 
> Fix string.h and also validate during build that nolibc stays within
> C99.

I'm all for improving portability, however I have a concern with building
the test case with -std=c99 which is that it might hide some c99-only
stuff that we'd introduce by accident in the nolibc's code, and I'd
rather not do that because it will mean changing build options for some
external programs using it if it happens. However I totally agree with
you that we need to make sure that there's no build issues with c99
compilers. Modern compilers are c99-compatible but generally come with
GNU extensions and I understand why you're interested in switching to
std=c99 in order to drop some of these like "asm". Should we have two
build targets, the default one and a c99 one ? Maybe. The build is so
small and quick that nobody will care, so we could definitely imagine
building the two versions. Maybe you have a better idea ?

Thanks,
Willy

Re: [PATCH] tools/nolibc: validate C99 compatibility

[Thomas Weißschuh]

Hi Willy,

On 2023-03-29 07:16:11+0200, Willy Tarreau wrote:
> On Tue, Mar 28, 2023 at 09:07:35PM +0000, Thomas Weißschuh wrote:
> > Most of the code was migrated to C99-conformant __asm__ statements
> > before. It seems string.h was missed.
> > 
> > Fix string.h and also validate during build that nolibc stays within
> > C99.
> 
> I'm all for improving portability, however I have a concern with building
> the test case with -std=c99 which is that it might hide some c99-only
> stuff that we'd introduce by accident in the nolibc's code, and I'd
> rather not do that because it will mean changing build options for some
> external programs using it if it happens. However I totally agree with
> you that we need to make sure that there's no build issues with c99
> compilers. Modern compilers are c99-compatible but generally come with
> GNU extensions and I understand why you're interested in switching to
> std=c99 in order to drop some of these like "asm". Should we have two
> build targets, the default one and a c99 one ? Maybe. The build is so
> small and quick that nobody will care, so we could definitely imagine
> building the two versions. Maybe you have a better idea ?

I'm not sure I understand.
Do you want to stay compatible with c89/gnu89?

If so we could use that baseline standard instead of -std=c99.

Without specifying a standard we get whatever the compiler uses as
default which is probably much newer than c99.

Having two targets seems to be easy to do but I'm not sure what the
advantage would be over compiling once against the intended baseline
standard.

Regards,
Thomas

Re: [PATCH] tools/nolibc: validate C99 compatibility

[Willy Tarreau]

On Wed, Mar 29, 2023 at 05:35:33AM +0000, Thomas Weißschuh wrote:
> Hi Willy,
> 
> On 2023-03-29 07:16:11+0200, Willy Tarreau wrote:
> > On Tue, Mar 28, 2023 at 09:07:35PM +0000, Thomas Weißschuh wrote:
> > > Most of the code was migrated to C99-conformant __asm__ statements
> > > before. It seems string.h was missed.
> > > 
> > > Fix string.h and also validate during build that nolibc stays within
> > > C99.
> > 
> > I'm all for improving portability, however I have a concern with building
> > the test case with -std=c99 which is that it might hide some c99-only
> > stuff that we'd introduce by accident in the nolibc's code, and I'd
> > rather not do that because it will mean changing build options for some
> > external programs using it if it happens. However I totally agree with
> > you that we need to make sure that there's no build issues with c99
> > compilers. Modern compilers are c99-compatible but generally come with
> > GNU extensions and I understand why you're interested in switching to
> > std=c99 in order to drop some of these like "asm". Should we have two
> > build targets, the default one and a c99 one ? Maybe. The build is so
> > small and quick that nobody will care, so we could definitely imagine
> > building the two versions. Maybe you have a better idea ?
> 
> I'm not sure I understand.
> Do you want to stay compatible with c89/gnu89?

At least with gnu89, yes, since it's been used by default by a wide
range of compilers.

> If so we could use that baseline standard instead of -std=c99.

The only thing is that c99 is both more permissive and more restrictive
than gnu89 since (as you noticed) gnu89 allows for example "asm" instead
of "__asm__".

> Without specifying a standard we get whatever the compiler uses as
> default which is probably much newer than c99.

Yes but do we really care ? I think we want at least some gnuXX
(which gcc does by default) and some c99 for those who don't want to
depend on gnuXX. Diversity in tests provides faster reports than
forcing everyone to the same set. By keeping the default build option,
a backwards-compatibility test is just a matter of setting CC= with the
relevant compiler to confirm it's still OK, without being fooled by the
fact that a standard other than the default was used.

> Having two targets seems to be easy to do but I'm not sure what the
> advantage would be over compiling once against the intended baseline
> standard.

We're providing a set of includes to be used by userland so there isn't
a single intended baseline standard. I'm not advocating for supporting
everything on earth at all, but at least it should work with native
compilers currently found in distros or on the kernel.org crosstools,
and with some older toolchains that are used once in a while to rebuild
a few compact tools. For example I've used this codebase to build a
recovery kernel+tools in the past, which fits everything in a 1MB
binary, and that's the type of thing where you know that it's not always
easy nor relevant to port the code to newer compilers, so if it used to
work on gcc 4.7 you'll just reuse that one if you still have it. My
position regarding older tools is: we don't make particular efforts to
test them, but we at least do not try hard to evince them either as
long as it's not necessary.

Thanks,
Willy

Re: [PATCH] tools/nolibc: validate C99 compatibility

[Willy Tarreau]

Hi Thomas,

On Wed, Mar 29, 2023 at 08:11:58AM +0200, Willy Tarreau wrote:
> On Wed, Mar 29, 2023 at 05:35:33AM +0000, Thomas Weißschuh wrote:
> > Hi Willy,
> > 
> > On 2023-03-29 07:16:11+0200, Willy Tarreau wrote:
> > > On Tue, Mar 28, 2023 at 09:07:35PM +0000, Thomas Weißschuh wrote:
> > > > Most of the code was migrated to C99-conformant __asm__ statements
> > > > before. It seems string.h was missed.
> > > > 
> > > > Fix string.h and also validate during build that nolibc stays within
> > > > C99.
> > > 
> > > I'm all for improving portability, however I have a concern with building
> > > the test case with -std=c99 which is that it might hide some c99-only
> > > stuff that we'd introduce by accident in the nolibc's code, and I'd
> > > rather not do that because it will mean changing build options for some
> > > external programs using it if it happens. However I totally agree with
> > > you that we need to make sure that there's no build issues with c99
> > > compilers. Modern compilers are c99-compatible but generally come with
> > > GNU extensions and I understand why you're interested in switching to
> > > std=c99 in order to drop some of these like "asm". Should we have two
> > > build targets, the default one and a c99 one ? Maybe. The build is so
> > > small and quick that nobody will care, so we could definitely imagine
> > > building the two versions. Maybe you have a better idea ?
> > 
> > I'm not sure I understand.
> > Do you want to stay compatible with c89/gnu89?
> 
> At least with gnu89, yes, since it's been used by default by a wide
> range of compilers.
> 
> > If so we could use that baseline standard instead of -std=c99.
> 
> The only thing is that c99 is both more permissive and more restrictive
> than gnu89 since (as you noticed) gnu89 allows for example "asm" instead
> of "__asm__".
> 
> > Without specifying a standard we get whatever the compiler uses as
> > default which is probably much newer than c99.
> 
> Yes but do we really care ? I think we want at least some gnuXX
> (which gcc does by default) and some c99 for those who don't want to
> depend on gnuXX. Diversity in tests provides faster reports than
> forcing everyone to the same set. By keeping the default build option,
> a backwards-compatibility test is just a matter of setting CC= with the
> relevant compiler to confirm it's still OK, without being fooled by the
> fact that a standard other than the default was used.
> 
> > Having two targets seems to be easy to do but I'm not sure what the
> > advantage would be over compiling once against the intended baseline
> > standard.
> 
> We're providing a set of includes to be used by userland so there isn't
> a single intended baseline standard. I'm not advocating for supporting
> everything on earth at all, but at least it should work with native
> compilers currently found in distros or on the kernel.org crosstools,
> and with some older toolchains that are used once in a while to rebuild
> a few compact tools. For example I've used this codebase to build a
> recovery kernel+tools in the past, which fits everything in a 1MB
> binary, and that's the type of thing where you know that it's not always
> easy nor relevant to port the code to newer compilers, so if it used to
> work on gcc 4.7 you'll just reuse that one if you still have it. My
> position regarding older tools is: we don't make particular efforts to
> test them, but we at least do not try hard to evince them either as
> long as it's not necessary.

I just ran some tests and there's actually better to achieve what you're
looking for. Let's just use -fno-asm, it removes the GNU-specific "asm",
"inline", and "typeof" in favor of the "__" variants. With gcc 11.3 it
gives me this, which is exactly what we were looking for:

  gcc -Os -fno-ident -fno-asynchronous-unwind-tables -fno-stack-protector -DNOLIBC_STACKPROTECTOR -mstack-protector-guard=global -fstack-protector-all -fno-asm -s -o nolibc-test \
    -nostdlib -static -Isysroot/x86/include nolibc-test.c -lgcc
  In file included from sysroot/x86/include/stdlib.h:14,
                   from sysroot/x86/include/nolibc.h:103,
                   from sysroot/x86/include/errno.h:26,
                   from sysroot/x86/include/stdio.h:14,
                   from nolibc-test.c:15:
  sysroot/x86/include/string.h: In function 'memset':
  sysroot/x86/include/string.h:93:17: error: 'asm' undeclared (first use in this function)
     93 |                 asm volatile("");
        |                 ^~~
  sysroot/x86/include/string.h:93:17: note: each undeclared identifier is reported only once for each function it appears in
  sysroot/x86/include/string.h:93:20: error: expected ';' before 'volatile'
     93 |                 asm volatile("");
        |                    ^~~~~~~~~
        |                    ;
  sysroot/x86/include/string.h: In function 'strlen':
  sysroot/x86/include/string.h:142:17: warning: implicit declaration of function 'asm' [-Wimplicit-function-declaration]
    142 |                 asm("");
        |                 ^~~
  nolibc-test.c: In function 'main':
  nolibc-test.c:898:25: error: 'asm' undeclared (first use in this function)
    898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
        |                         ^~~
  nolibc-test.c:898:28: error: expected ';' before 'volatile'
    898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
        |                            ^~~~~~~~~
        |                            ;
  make: *** [Makefile:128: nolibc-test] Error 1

With this, we don't need to force -std=c99 nor to build two variants,
a single variant will catch GCCisms even with older compilers while not
being overly annoying.

Willy

Re: [PATCH] tools/nolibc: validate C99 compatibility

[Thomas Weißschuh]

Hi Willy,

sorry for the late response to your previous mail, I was traveling.

On 2023-04-01 12:03:30+0200, Willy Tarreau wrote:
> On Wed, Mar 29, 2023 at 08:11:58AM +0200, Willy Tarreau wrote:
> > On Wed, Mar 29, 2023 at 05:35:33AM +0000, Thomas Weißschuh wrote:
> > > Hi Willy,
> > > 
> > > On 2023-03-29 07:16:11+0200, Willy Tarreau wrote:
> > > > On Tue, Mar 28, 2023 at 09:07:35PM +0000, Thomas Weißschuh wrote:
> > > > > Most of the code was migrated to C99-conformant __asm__ statements
> > > > > before. It seems string.h was missed.
> > > > > 
> > > > > Fix string.h and also validate during build that nolibc stays within
> > > > > C99.
> > > > 
> > > > I'm all for improving portability, however I have a concern with building
> > > > the test case with -std=c99 which is that it might hide some c99-only
> > > > stuff that we'd introduce by accident in the nolibc's code, and I'd
> > > > rather not do that because it will mean changing build options for some
> > > > external programs using it if it happens. However I totally agree with
> > > > you that we need to make sure that there's no build issues with c99
> > > > compilers. Modern compilers are c99-compatible but generally come with
> > > > GNU extensions and I understand why you're interested in switching to
> > > > std=c99 in order to drop some of these like "asm". Should we have two
> > > > build targets, the default one and a c99 one ? Maybe. The build is so
> > > > small and quick that nobody will care, so we could definitely imagine
> > > > building the two versions. Maybe you have a better idea ?
> > > 
> > > I'm not sure I understand.
> > > Do you want to stay compatible with c89/gnu89?
> > 
> > At least with gnu89, yes, since it's been used by default by a wide
> > range of compilers.
> > 
> > > If so we could use that baseline standard instead of -std=c99.
> > 
> > The only thing is that c99 is both more permissive and more restrictive
> > than gnu89 since (as you noticed) gnu89 allows for example "asm" instead
> > of "__asm__".
> > 
> > > Without specifying a standard we get whatever the compiler uses as
> > > default which is probably much newer than c99.
> > 
> > Yes but do we really care ? I think we want at least some gnuXX
> > (which gcc does by default) and some c99 for those who don't want to
> > depend on gnuXX. Diversity in tests provides faster reports than
> > forcing everyone to the same set. By keeping the default build option,
> > a backwards-compatibility test is just a matter of setting CC= with the
> > relevant compiler to confirm it's still OK, without being fooled by the
> > fact that a standard other than the default was used.
> > 
> > > Having two targets seems to be easy to do but I'm not sure what the
> > > advantage would be over compiling once against the intended baseline
> > > standard.
> > 
> > We're providing a set of includes to be used by userland so there isn't
> > a single intended baseline standard. I'm not advocating for supporting
> > everything on earth at all, but at least it should work with native
> > compilers currently found in distros or on the kernel.org crosstools,
> > and with some older toolchains that are used once in a while to rebuild
> > a few compact tools. For example I've used this codebase to build a
> > recovery kernel+tools in the past, which fits everything in a 1MB
> > binary, and that's the type of thing where you know that it's not always
> > easy nor relevant to port the code to newer compilers, so if it used to
> > work on gcc 4.7 you'll just reuse that one if you still have it. My
> > position regarding older tools is: we don't make particular efforts to
> > test them, but we at least do not try hard to evince them either as
> > long as it's not necessary.
> 
> I just ran some tests and there's actually better to achieve what you're
> looking for. Let's just use -fno-asm, it removes the GNU-specific "asm",
> "inline", and "typeof" in favor of the "__" variants. With gcc 11.3 it
> gives me this, which is exactly what we were looking for:
> 
>   gcc -Os -fno-ident -fno-asynchronous-unwind-tables -fno-stack-protector -DNOLIBC_STACKPROTECTOR -mstack-protector-guard=global -fstack-protector-all -fno-asm -s -o nolibc-test \
>     -nostdlib -static -Isysroot/x86/include nolibc-test.c -lgcc
>   In file included from sysroot/x86/include/stdlib.h:14,
>                    from sysroot/x86/include/nolibc.h:103,
>                    from sysroot/x86/include/errno.h:26,
>                    from sysroot/x86/include/stdio.h:14,
>                    from nolibc-test.c:15:
>   sysroot/x86/include/string.h: In function 'memset':
>   sysroot/x86/include/string.h:93:17: error: 'asm' undeclared (first use in this function)
>      93 |                 asm volatile("");
>         |                 ^~~
>   sysroot/x86/include/string.h:93:17: note: each undeclared identifier is reported only once for each function it appears in
>   sysroot/x86/include/string.h:93:20: error: expected ';' before 'volatile'
>      93 |                 asm volatile("");
>         |                    ^~~~~~~~~
>         |                    ;
>   sysroot/x86/include/string.h: In function 'strlen':
>   sysroot/x86/include/string.h:142:17: warning: implicit declaration of function 'asm' [-Wimplicit-function-declaration]
>     142 |                 asm("");
>         |                 ^~~
>   nolibc-test.c: In function 'main':
>   nolibc-test.c:898:25: error: 'asm' undeclared (first use in this function)
>     898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
>         |                         ^~~
>   nolibc-test.c:898:28: error: expected ';' before 'volatile'
>     898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
>         |                            ^~~~~~~~~
>         |                            ;
>   make: *** [Makefile:128: nolibc-test] Error 1
> 
> With this, we don't need to force -std=c99 nor to build two variants,
> a single variant will catch GCCisms even with older compilers while not
> being overly annoying.

The original goal was to have a fixed and explicit baseline that is
supported. Using -std= ensures that no deviations are accidentally
introduced.

Using -fno-asm only prevents this specific gnu-ism. All other gnu-isms
and features from whatever the current compilers default language level
is are still allowed.

Therefore I'm not convinced of this aproach.

These are the aproaches I can see that reach this goal:

* Declare C99 as baseline, build with -std=c99
* Declare C99 and GNU89 as baseline, build with both.
* Declare C99 and GNU89 as baseline, build with -std=c99,
  wait for people to complain if something breaks on gnu89
  (same as current status)
* Declare C89 as baseline, remove all C99-isms and gnu-isms
  (C++ comments, "inline" and some smaller stuff),
  build with -std=c89.

Personally I think C99 is a baseline that is easy to fulfill by nolibc
and should not restrict users.

Thomas

Re: [PATCH] tools/nolibc: validate C99 compatibility

[Willy Tarreau]

On Sat, Apr 01, 2023 at 10:33:45AM +0000, Thomas Weißschuh wrote:
> Hi Willy,
> 
> sorry for the late response to your previous mail, I was traveling.

Don't be sorry, I hardly have the time to respond between week-ends,
so you're waiting for me more than the other way around!

> > I just ran some tests and there's actually better to achieve what you're
> > looking for. Let's just use -fno-asm, it removes the GNU-specific "asm",
> > "inline", and "typeof" in favor of the "__" variants. With gcc 11.3 it
> > gives me this, which is exactly what we were looking for:
> > 
> >   gcc -Os -fno-ident -fno-asynchronous-unwind-tables -fno-stack-protector -DNOLIBC_STACKPROTECTOR -mstack-protector-guard=global -fstack-protector-all -fno-asm -s -o nolibc-test \
> >     -nostdlib -static -Isysroot/x86/include nolibc-test.c -lgcc
> >   In file included from sysroot/x86/include/stdlib.h:14,
> >                    from sysroot/x86/include/nolibc.h:103,
> >                    from sysroot/x86/include/errno.h:26,
> >                    from sysroot/x86/include/stdio.h:14,
> >                    from nolibc-test.c:15:
> >   sysroot/x86/include/string.h: In function 'memset':
> >   sysroot/x86/include/string.h:93:17: error: 'asm' undeclared (first use in this function)
> >      93 |                 asm volatile("");
> >         |                 ^~~
> >   sysroot/x86/include/string.h:93:17: note: each undeclared identifier is reported only once for each function it appears in
> >   sysroot/x86/include/string.h:93:20: error: expected ';' before 'volatile'
> >      93 |                 asm volatile("");
> >         |                    ^~~~~~~~~
> >         |                    ;
> >   sysroot/x86/include/string.h: In function 'strlen':
> >   sysroot/x86/include/string.h:142:17: warning: implicit declaration of function 'asm' [-Wimplicit-function-declaration]
> >     142 |                 asm("");
> >         |                 ^~~
> >   nolibc-test.c: In function 'main':
> >   nolibc-test.c:898:25: error: 'asm' undeclared (first use in this function)
> >     898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
> >         |                         ^~~
> >   nolibc-test.c:898:28: error: expected ';' before 'volatile'
> >     898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
> >         |                            ^~~~~~~~~
> >         |                            ;
> >   make: *** [Makefile:128: nolibc-test] Error 1
> > 
> > With this, we don't need to force -std=c99 nor to build two variants,
> > a single variant will catch GCCisms even with older compilers while not
> > being overly annoying.
> 
> The original goal was to have a fixed and explicit baseline that is
> supported. Using -std= ensures that no deviations are accidentally
> introduced.
> 
> Using -fno-asm only prevents this specific gnu-ism. All other gnu-isms
> and features from whatever the current compilers default language level
> is are still allowed.
> 
> Therefore I'm not convinced of this aproach.
> 
> These are the aproaches I can see that reach this goal:
> 
> * Declare C99 as baseline, build with -std=c99
> * Declare C99 and GNU89 as baseline, build with both.
> * Declare C99 and GNU89 as baseline, build with -std=c99,
>   wait for people to complain if something breaks on gnu89
>   (same as current status)
> * Declare C89 as baseline, remove all C99-isms and gnu-isms
>   (C++ comments, "inline" and some smaller stuff),
>   build with -std=c89.
> 
> Personally I think C99 is a baseline that is easy to fulfill by nolibc
> and should not restrict users.

I *am* affected. I do maintain some code that builds using an older
version of nolibc and still builds with the current one, that builds
fine on gcc 4.4..4.9 (as still present on some still supported distros
such as RHEL7 which comes with gcc-4.8). I just tried to build
nolibc-test with gcc-4.7 (which I still have for plenty of archs and
that is ~50% faster than 11.x, something appreciable during wide
range bisects) and it breaks on the size_t declared inside the for
statement.

We could possibly go with your 3rd proposal above (i.e. both baselines,
only build with c99 and wait for reports). Or we could equally build
with "-std=gnu89 -fno-asm" and make sure we stay away from any recent
accidental deviation. All relevant compilers in use for a while are
compatible with this because this has been the default for a very long
time. By the way, it's worth noting that no single gcc version ever
used c99 as a default standard.

Thanks,
Willy

Re: [PATCH] tools/nolibc: validate C99 compatibility

[Thomas Weißschuh]

On 2023-04-01 13:04:33+0200, Willy Tarreau wrote:
> On Sat, Apr 01, 2023 at 10:33:45AM +0000, Thomas Weißschuh wrote:
> > Hi Willy,
> > 
> > sorry for the late response to your previous mail, I was traveling.
> 
> Don't be sorry, I hardly have the time to respond between week-ends,
> so you're waiting for me more than the other way around!
> 
> > > I just ran some tests and there's actually better to achieve what you're
> > > looking for. Let's just use -fno-asm, it removes the GNU-specific "asm",
> > > "inline", and "typeof" in favor of the "__" variants. With gcc 11.3 it
> > > gives me this, which is exactly what we were looking for:
> > > 
> > >   gcc -Os -fno-ident -fno-asynchronous-unwind-tables -fno-stack-protector -DNOLIBC_STACKPROTECTOR -mstack-protector-guard=global -fstack-protector-all -fno-asm -s -o nolibc-test \
> > >     -nostdlib -static -Isysroot/x86/include nolibc-test.c -lgcc
> > >   In file included from sysroot/x86/include/stdlib.h:14,
> > >                    from sysroot/x86/include/nolibc.h:103,
> > >                    from sysroot/x86/include/errno.h:26,
> > >                    from sysroot/x86/include/stdio.h:14,
> > >                    from nolibc-test.c:15:
> > >   sysroot/x86/include/string.h: In function 'memset':
> > >   sysroot/x86/include/string.h:93:17: error: 'asm' undeclared (first use in this function)
> > >      93 |                 asm volatile("");
> > >         |                 ^~~
> > >   sysroot/x86/include/string.h:93:17: note: each undeclared identifier is reported only once for each function it appears in
> > >   sysroot/x86/include/string.h:93:20: error: expected ';' before 'volatile'
> > >      93 |                 asm volatile("");
> > >         |                    ^~~~~~~~~
> > >         |                    ;
> > >   sysroot/x86/include/string.h: In function 'strlen':
> > >   sysroot/x86/include/string.h:142:17: warning: implicit declaration of function 'asm' [-Wimplicit-function-declaration]
> > >     142 |                 asm("");
> > >         |                 ^~~
> > >   nolibc-test.c: In function 'main':
> > >   nolibc-test.c:898:25: error: 'asm' undeclared (first use in this function)
> > >     898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
> > >         |                         ^~~
> > >   nolibc-test.c:898:28: error: expected ';' before 'volatile'
> > >     898 |                         asm volatile ("outb %%al, %%dx" :: "d"(0x501), "a"(0));
> > >         |                            ^~~~~~~~~
> > >         |                            ;
> > >   make: *** [Makefile:128: nolibc-test] Error 1
> > > 
> > > With this, we don't need to force -std=c99 nor to build two variants,
> > > a single variant will catch GCCisms even with older compilers while not
> > > being overly annoying.
> > 
> > The original goal was to have a fixed and explicit baseline that is
> > supported. Using -std= ensures that no deviations are accidentally
> > introduced.
> > 
> > Using -fno-asm only prevents this specific gnu-ism. All other gnu-isms
> > and features from whatever the current compilers default language level
> > is are still allowed.
> > 
> > Therefore I'm not convinced of this aproach.
> > 
> > These are the aproaches I can see that reach this goal:
> > 
> > * Declare C99 as baseline, build with -std=c99
> > * Declare C99 and GNU89 as baseline, build with both.
> > * Declare C99 and GNU89 as baseline, build with -std=c99,
> >   wait for people to complain if something breaks on gnu89
> >   (same as current status)
> > * Declare C89 as baseline, remove all C99-isms and gnu-isms
> >   (C++ comments, "inline" and some smaller stuff),
> >   build with -std=c89.
> > 
> > Personally I think C99 is a baseline that is easy to fulfill by nolibc
> > and should not restrict users.
> 
> I *am* affected. I do maintain some code that builds using an older
> version of nolibc and still builds with the current one, that builds
> fine on gcc 4.4..4.9 (as still present on some still supported distros
> such as RHEL7 which comes with gcc-4.8). I just tried to build
> nolibc-test with gcc-4.7 (which I still have for plenty of archs and
> that is ~50% faster than 11.x, something appreciable during wide
> range bisects) and it breaks on the size_t declared inside the for
> statement.

> We could possibly go with your 3rd proposal above (i.e. both baselines,
> only build with c99 and wait for reports). Or we could equally build
> with "-std=gnu89 -fno-asm" and make sure we stay away from any recent
> accidental deviation. All relevant compilers in use for a while are
> compatible with this because this has been the default for a very long
> time. By the way, it's worth noting that no single gcc version ever
> used c99 as a default standard.

Personally I don't trust myself not to accidentally introduce
incompatible code.
Case in point with the above mentioned declaration.
Also it's non-obvious for new contributors.

I'll resend a version that also builds with -std=gnu89.

Thomas