[PATCH v2] tools/nolibc: compiler: add macro __nolibc_fallthrough

[PATCH v2] tools/nolibc: compiler: add macro __nolibc_fallthrough

[Thomas Weißschuh]

Recent version of GCC and clang gained -Wimplicit-fallthrough,
warning about implicit fall-through between switch labels.
As nolibc does not control the compilation flags, this can trigger
warnings for when built by the user.
Make use of the "fallthrough" attribute to explicitly annotate the
expected fall-throughs and silence the warning.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Changes in v2:
- Add a do-while loop to improve compiler compatibility
- Link to v1: https://lore.kernel.org/r/20240929-nolibc-fallthrough-v1-1-5ee07ea9a683@weissschuh.net
---
 tools/include/nolibc/compiler.h | 6 ++++++
 tools/include/nolibc/stdio.h    | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h
index 9bc6a706a332378e5af1f5baabf953f137f99749..fa1f547e7f13d151ea98b9c585b36659d2d27324 100644
--- a/tools/include/nolibc/compiler.h
+++ b/tools/include/nolibc/compiler.h
@@ -32,4 +32,10 @@
 #  define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector")))
 #endif /* __nolibc_has_attribute(no_stack_protector) */
 
+#if __nolibc_has_attribute(fallthrough)
+#  define __nolibc_fallthrough do { } while (0); __attribute__((fallthrough))
+#else
+#  define __nolibc_fallthrough do { } while (0)
+#endif /* __nolibc_has_attribute(fallthrough) */
+
 #endif /* _NOLIBC_COMPILER_H */
diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index c968dbbc4ef8137e237b859bf18a6d2970230cbf..3892034198dd566d21a5cc0a9f67cf097d428393 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -15,6 +15,7 @@
 #include "stdarg.h"
 #include "stdlib.h"
 #include "string.h"
+#include "compiler.h"
 
 #ifndef EOF
 #define EOF (-1)
@@ -264,7 +265,7 @@ int vfprintf(FILE *stream, const char *fmt, va_list args)
 				case 'p':
 					*(out++) = '0';
 					*(out++) = 'x';
-					/* fall through */
+					__nolibc_fallthrough;
 				default: /* 'x' and 'p' above */
 					u64toh_r(v, out);
 					break;

---
base-commit: e7ed343658792771cf1b868df061661b7bcc5cef
change-id: 20240929-nolibc-fallthrough-ba822a4a9255

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

RE: [PATCH v2] tools/nolibc: compiler: add macro __nolibc_fallthrough

[David Laight]

From: Thomas Weißschuh
> Sent: 30 September 2024 06:35
> 
> Recent version of GCC and clang gained -Wimplicit-fallthrough,
> warning about implicit fall-through between switch labels.
> As nolibc does not control the compilation flags, this can trigger
> warnings for when built by the user.
> Make use of the "fallthrough" attribute to explicitly annotate the
> expected fall-throughs and silence the warning.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Changes in v2:
> - Add a do-while loop to improve compiler compatibility
> - Link to v1: https://lore.kernel.org/r/20240929-nolibc-fallthrough-v1-1-5ee07ea9a683@weissschuh.net
...
> 
> +#if __nolibc_has_attribute(fallthrough)
> +#  define __nolibc_fallthrough do { } while (0); __attribute__((fallthrough))
> +#else
> +#  define __nolibc_fallthrough do { } while (0)
> +#endif /* __nolibc_has_attribute(fallthrough) */
> +
>  #endif /* _NOLIBC_COMPILER_H */
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index c968dbbc4ef8137e237b859bf18a6d2970230cbf..3892034198dd566d21a5cc0a9f67cf097d428393 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -15,6 +15,7 @@
>  #include "stdarg.h"
>  #include "stdlib.h"
>  #include "string.h"
> +#include "compiler.h"
> 
>  #ifndef EOF
>  #define EOF (-1)
> @@ -264,7 +265,7 @@ int vfprintf(FILE *stream, const char *fmt, va_list args)
>  				case 'p':
>  					*(out++) = '0';
>  					*(out++) = 'x';
> -					/* fall through */
> +					__nolibc_fallthrough;
>  				default: /* 'x' and 'p' above */

Doesn't this break any old tools that would have parsed the /* fall though */
comment (or any of its variants)?

If you move the ; into the define the 'old' definition can be empty.
And then it is possible that:
				case x:
					xxxxx;
					/* fall though */
					__nolibc_fallthough
				case y:

will be processed correctly be all tools.
I know I had to lower the warning level for one of our kernel drivers.
But I've forgotten why - that code has to go through a lot of
compilers - including Microsofts.

	David


>  					u64toh_r(v, out);
>  					break;
> 
> ---
> base-commit: e7ed343658792771cf1b868df061661b7bcc5cef
> change-id: 20240929-nolibc-fallthrough-ba822a4a9255
> 
> Best regards,
> --
> Thomas Weißschuh <linux@weissschuh.net>
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2] tools/nolibc: compiler: add macro __nolibc_fallthrough

[Willy Tarreau]

Hi David,

On Mon, Sep 30, 2024 at 03:29:26PM +0000, David Laight wrote:
> From: Thomas Weißschuh
> > Sent: 30 September 2024 06:35
> > 
> > Recent version of GCC and clang gained -Wimplicit-fallthrough,
> > warning about implicit fall-through between switch labels.
> > As nolibc does not control the compilation flags, this can trigger
> > warnings for when built by the user.
> > Make use of the "fallthrough" attribute to explicitly annotate the
> > expected fall-throughs and silence the warning.
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > Changes in v2:
> > - Add a do-while loop to improve compiler compatibility
> > - Link to v1: https://lore.kernel.org/r/20240929-nolibc-fallthrough-v1-1-5ee07ea9a683@weissschuh.net
> ...
> > 
> > +#if __nolibc_has_attribute(fallthrough)
> > +#  define __nolibc_fallthrough do { } while (0); __attribute__((fallthrough))
> > +#else
> > +#  define __nolibc_fallthrough do { } while (0)
> > +#endif /* __nolibc_has_attribute(fallthrough) */
> > +
> >  #endif /* _NOLIBC_COMPILER_H */
> > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > index c968dbbc4ef8137e237b859bf18a6d2970230cbf..3892034198dd566d21a5cc0a9f67cf097d428393 100644
> > --- a/tools/include/nolibc/stdio.h
> > +++ b/tools/include/nolibc/stdio.h
> > @@ -15,6 +15,7 @@
> >  #include "stdarg.h"
> >  #include "stdlib.h"
> >  #include "string.h"
> > +#include "compiler.h"
> > 
> >  #ifndef EOF
> >  #define EOF (-1)
> > @@ -264,7 +265,7 @@ int vfprintf(FILE *stream, const char *fmt, va_list args)
> >  				case 'p':
> >  					*(out++) = '0';
> >  					*(out++) = 'x';
> > -					/* fall through */
> > +					__nolibc_fallthrough;
> >  				default: /* 'x' and 'p' above */
> 
> Doesn't this break any old tools that would have parsed the /* fall though */
> comment (or any of its variants)?

Apparently not, I had rechecked with gcc-6.5 which is the one that
used to expect comments, and it's apparently fine as well with the
macro, probably due to its name which contains "fallthrough" in it.
Anyway that one was broken when running cpp separately from cc1, so
those at risk of seeing any warning already see it in their programs.

> If you move the ; into the define the 'old' definition can be empty.
> And then it is possible that:
> 				case x:
> 					xxxxx;
> 					/* fall though */
> 					__nolibc_fallthough
> 				case y:
> 
> will be processed correctly be all tools.

Missing trailing commas is a real PITA, which can even cause indent
issues with editors, better not do that, frankly.

> I know I had to lower the warning level for one of our kernel drivers.
> But I've forgotten why - that code has to go through a lot of
> compilers - including Microsofts.

Probably a case like I mentioned above: pre-processing being run as
a first step, making the compiler not see the comment and complain.

Cheers,
Willy