[v3] tools/nolibc: enable compiler warnings

[PATCH v3 05/14] tools/nolibc: stdint: use int for size_t on 32bit

Posted by Thomas Weißschuh 2 years, 6 months ago

Otherwise both gcc and clang may generate warnings about type
mismatches:

sysroot/mips/include/string.h:12:14: warning: mismatch in argument 1 type of built-in function 'malloc'; expected 'unsigned int' [-Wbuiltin-declaration-mismatch]
   12 | static void *malloc(size_t len);
      |              ^~~~~~

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/stdint.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
index 4b282435a59a..0f390c3028d8 100644
--- a/tools/include/nolibc/stdint.h
+++ b/tools/include/nolibc/stdint.h
@@ -15,7 +15,11 @@ typedef unsigned int       uint32_t;
 typedef   signed int        int32_t;
 typedef unsigned long long uint64_t;
 typedef   signed long long  int64_t;
+#if __SIZE_WIDTH__ == 64
 typedef unsigned long        size_t;
+#else
+typedef unsigned int         size_t;
+#endif
 typedef   signed long       ssize_t;
 typedef unsigned long     uintptr_t;
 typedef   signed long      intptr_t;

-- 
2.41.0

Re: [PATCH v3 05/14] tools/nolibc: stdint: use int for size_t on 32bit

Posted by Willy Tarreau 2 years, 6 months ago

Hi Thomas,

On Thu, Aug 03, 2023 at 09:28:49AM +0200, Thomas Weißschuh wrote:
> Otherwise both gcc and clang may generate warnings about type
> mismatches:
> 
> sysroot/mips/include/string.h:12:14: warning: mismatch in argument 1 type of built-in function 'malloc'; expected 'unsigned int' [-Wbuiltin-declaration-mismatch]
>    12 | static void *malloc(size_t len);
>       |              ^~~~~~
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  tools/include/nolibc/stdint.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
> index 4b282435a59a..0f390c3028d8 100644
> --- a/tools/include/nolibc/stdint.h
> +++ b/tools/include/nolibc/stdint.h
> @@ -15,7 +15,11 @@ typedef unsigned int       uint32_t;
>  typedef   signed int        int32_t;
>  typedef unsigned long long uint64_t;
>  typedef   signed long long  int64_t;
> +#if __SIZE_WIDTH__ == 64
>  typedef unsigned long        size_t;
> +#else
> +typedef unsigned int         size_t;
> +#endif

This one breaks gcc < 7 for me because __SIZE_WIDTH__ is not defined
there. However I could trace __SIZE_TYPE__ to be defined since at least
gcc-3.4 so instead we can do this, which will always match the type set
by the compiler (either "unsigned int" or "unsigned long int") :

  #ifdef __SIZE_TYPE__
  typedef __SIZE_TYPE__ size_t;
  #else
  typedef unsigned long size_t;
  #endif

Please just let me know if you want me to modify your patch accordingly.
I'm still continuing the tests.

Thanks,
Willy

Re: [PATCH v3 05/14] tools/nolibc: stdint: use int for size_t on 32bit

Posted by Thomas Weißschuh 2 years, 6 months ago

On 2023-08-05 18:19:29+0200, Willy Tarreau wrote:
> Hi Thomas,
> 
> On Thu, Aug 03, 2023 at 09:28:49AM +0200, Thomas Weißschuh wrote:
> > Otherwise both gcc and clang may generate warnings about type
> > mismatches:
> > 
> > sysroot/mips/include/string.h:12:14: warning: mismatch in argument 1 type of built-in function 'malloc'; expected 'unsigned int' [-Wbuiltin-declaration-mismatch]
> >    12 | static void *malloc(size_t len);
> >       |              ^~~~~~
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >  tools/include/nolibc/stdint.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
> > index 4b282435a59a..0f390c3028d8 100644
> > --- a/tools/include/nolibc/stdint.h
> > +++ b/tools/include/nolibc/stdint.h
> > @@ -15,7 +15,11 @@ typedef unsigned int       uint32_t;
> >  typedef   signed int        int32_t;
> >  typedef unsigned long long uint64_t;
> >  typedef   signed long long  int64_t;
> > +#if __SIZE_WIDTH__ == 64
> >  typedef unsigned long        size_t;
> > +#else
> > +typedef unsigned int         size_t;
> > +#endif
> 
> This one breaks gcc < 7 for me because __SIZE_WIDTH__ is not defined
> there. However I could trace __SIZE_TYPE__ to be defined since at least
> gcc-3.4 so instead we can do this, which will always match the type set
> by the compiler (either "unsigned int" or "unsigned long int") :
> 
>   #ifdef __SIZE_TYPE__
>   typedef __SIZE_TYPE__ size_t;
>   #else
>   typedef unsigned long size_t;
>   #endif

Sounds good. But do we need the fallback?

Further below we are also unconditionally using preprocessor-defines
like __INT_MAX__ and __LONG_MAX__.

So I guess we can drop the proposed #ifdef.

> Please just let me know if you want me to modify your patch accordingly.
> I'm still continuing the tests.

Feel free to modify the patch.

Thanks!
Thomas

Re: [PATCH v3 05/14] tools/nolibc: stdint: use int for size_t on 32bit

Posted by Willy Tarreau 2 years, 6 months ago

On Sat, Aug 05, 2023 at 06:25:52PM +0200, Thomas Weißschuh wrote:
> On 2023-08-05 18:19:29+0200, Willy Tarreau wrote:
> > Hi Thomas,
> > 
> > On Thu, Aug 03, 2023 at 09:28:49AM +0200, Thomas Weißschuh wrote:
> > > Otherwise both gcc and clang may generate warnings about type
> > > mismatches:
> > > 
> > > sysroot/mips/include/string.h:12:14: warning: mismatch in argument 1 type of built-in function 'malloc'; expected 'unsigned int' [-Wbuiltin-declaration-mismatch]
> > >    12 | static void *malloc(size_t len);
> > >       |              ^~~~~~
> > > 
> > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > >  tools/include/nolibc/stdint.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
> > > index 4b282435a59a..0f390c3028d8 100644
> > > --- a/tools/include/nolibc/stdint.h
> > > +++ b/tools/include/nolibc/stdint.h
> > > @@ -15,7 +15,11 @@ typedef unsigned int       uint32_t;
> > >  typedef   signed int        int32_t;
> > >  typedef unsigned long long uint64_t;
> > >  typedef   signed long long  int64_t;
> > > +#if __SIZE_WIDTH__ == 64
> > >  typedef unsigned long        size_t;
> > > +#else
> > > +typedef unsigned int         size_t;
> > > +#endif
> > 
> > This one breaks gcc < 7 for me because __SIZE_WIDTH__ is not defined
> > there. However I could trace __SIZE_TYPE__ to be defined since at least
> > gcc-3.4 so instead we can do this, which will always match the type set
> > by the compiler (either "unsigned int" or "unsigned long int") :
> > 
> >   #ifdef __SIZE_TYPE__
> >   typedef __SIZE_TYPE__ size_t;
> >   #else
> >   typedef unsigned long size_t;
> >   #endif
> 
> Sounds good. But do we need the fallback?

I don't know. It's always the same when using a compiler-defined macro
that you discover when you need it, you never know how spread it is.
At least I've also found it in clang as old as 3.8, so maybe it can be
considered safe enough.

> Further below we are also unconditionally using preprocessor-defines
> like __INT_MAX__ and __LONG_MAX__.
> 
> So I guess we can drop the proposed #ifdef.

I'll try with this, the risk is quite low anyway (famous last words).

> > Please just let me know if you want me to modify your patch accordingly.
> > I'm still continuing the tests.
> 
> Feel free to modify the patch.

Will do, thanks!

Willy