diff mbox series

[v3,1/7] meson: Run some compiler checks using -Wno-unused-value

Message ID 20241218182106.78800-2-philmd@linaro.org
State New
Headers show
Series hw/ppc: Remove tswap() calls | expand

Commit Message

Philippe Mathieu-Daudé Dec. 18, 2024, 6:21 p.m. UTC 
When running Clang static analyzer on macOS I'm getting:

  include/qemu/osdep.h:634:8: error: redefinition of 'iovec'
    634 | struct iovec {
        |        ^
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/_types/_iovec_t.h:31:8: note: previous definition is here
     31 | struct iovec {
        |        ^
  1 error generated.

Looking at meson-logs.txt, the analyzer enables -Wunused-value
making meson generated code to fail:

    Code:
    #include <sys/uio.h>
            void bar(void) {
                sizeof(struct iovec);
            }
    -----------
    stderr:
    meson-private/tmpe8_1b_00/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
        3 |             sizeof(struct iovec);
          |             ^~~~~~~~~~~~~~~~~~~~
    1 error generated.
    -----------
    Checking for type "struct iovec" : NO

    Code:
    #include <utmpx.h>
            void bar(void) {
                sizeof(struct utmpx);
            }
    -----------
    stderr:
    meson-private/tmp3n0u490p/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
        3 |             sizeof(struct utmpx);
          |             ^~~~~~~~~~~~~~~~~~~~
    1 error generated.
    -----------
    Checking for type "struct utmpx" : NO

    Code:

            #include <getopt.h>
            int main(void) {
                /* If it's not defined as a macro, try to use as a symbol */
                #ifndef optreset
                    optreset;
                #endif
                return 0;
            }
    -----------
    stderr:
    meson-private/tmp1rzob_os/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
        6 |                 optreset;
          |                 ^~~~~~~~
    1 error generated.
    -----------
    Header "getopt.h" has symbol "optreset" : NO

    Code:

            #include <vmnet/vmnet.h>
            int main(void) {
                /* If it's not defined as a macro, try to use as a symbol */
                #ifndef VMNET_BRIDGED_MODE
                    VMNET_BRIDGED_MODE;
                #endif
                return 0;
            }
    -----------
    stderr:
    meson-private/tmpl9jgsxpt/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
        6 |                 VMNET_BRIDGED_MODE;
          |                 ^~~~~~~~~~~~~~~~~~
    1 error generated.
    -----------
    Header "vmnet/vmnet.h" has symbol "VMNET_BRIDGED_MODE" with dependency appleframeworks: NO
    ../meson.build:1174: WARNING: vmnet.framework API is outdated, disabling

Fix by explicitly disabling -Wunused-value from these meson checks.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Probably meson should do that in has_header_symbol() / has_type()?
---
 meson.build | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Nicholas Piggin Dec. 19, 2024, 12:37 a.m. UTC | #1 
On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
> When running Clang static analyzer on macOS I'm getting:
>
>   include/qemu/osdep.h:634:8: error: redefinition of 'iovec'
>     634 | struct iovec {
>         |        ^
>   /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/_types/_iovec_t.h:31:8: note: previous definition is here
>      31 | struct iovec {
>         |        ^
>   1 error generated.
>
> Looking at meson-logs.txt, the analyzer enables -Wunused-value
> making meson generated code to fail:
>
>     Code:
>     #include <sys/uio.h>
>             void bar(void) {
>                 sizeof(struct iovec);
>             }
>     -----------
>     stderr:
>     meson-private/tmpe8_1b_00/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
>         3 |             sizeof(struct iovec);
>           |             ^~~~~~~~~~~~~~~~~~~~
>     1 error generated.
>     -----------
>     Checking for type "struct iovec" : NO
>
>     Code:
>     #include <utmpx.h>
>             void bar(void) {
>                 sizeof(struct utmpx);
>             }
>     -----------
>     stderr:
>     meson-private/tmp3n0u490p/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
>         3 |             sizeof(struct utmpx);
>           |             ^~~~~~~~~~~~~~~~~~~~
>     1 error generated.
>     -----------
>     Checking for type "struct utmpx" : NO
>
>     Code:
>
>             #include <getopt.h>
>             int main(void) {
>                 /* If it's not defined as a macro, try to use as a symbol */
>                 #ifndef optreset
>                     optreset;
>                 #endif
>                 return 0;
>             }
>     -----------
>     stderr:
>     meson-private/tmp1rzob_os/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>         6 |                 optreset;
>           |                 ^~~~~~~~
>     1 error generated.
>     -----------
>     Header "getopt.h" has symbol "optreset" : NO
>
>     Code:
>
>             #include <vmnet/vmnet.h>
>             int main(void) {
>                 /* If it's not defined as a macro, try to use as a symbol */
>                 #ifndef VMNET_BRIDGED_MODE
>                     VMNET_BRIDGED_MODE;
>                 #endif
>                 return 0;
>             }
>     -----------
>     stderr:
>     meson-private/tmpl9jgsxpt/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>         6 |                 VMNET_BRIDGED_MODE;
>           |                 ^~~~~~~~~~~~~~~~~~
>     1 error generated.
>     -----------
>     Header "vmnet/vmnet.h" has symbol "VMNET_BRIDGED_MODE" with dependency appleframeworks: NO
>     ../meson.build:1174: WARNING: vmnet.framework API is outdated, disabling
>
> Fix by explicitly disabling -Wunused-value from these meson checks.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Probably meson should do that in has_header_symbol() / has_type()?

I don't know about the build system to answer this, but should we
instead disable -Werror on these tests to be a bit more future-proof?
Compilers often add new warnings or catch more cases of existing
warnings.

Alternative would be to keep -Werror but fail the build if a test
throws a warning, but that seems like a lot more work for little
benefit...

Thanks,
Nick

> ---
>  meson.build | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 85f74854735..9d93dcd95d7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1189,7 +1189,8 @@ cocoa = dependency('appleframeworks',
>  vmnet = dependency('appleframeworks', modules: 'vmnet', required: get_option('vmnet'))
>  if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
>                                                'VMNET_BRIDGED_MODE',
> -                                              dependencies: vmnet)
> +                                              dependencies: vmnet,
> +                                              args: '-Wno-unused-value')
>    vmnet = not_found
>    if get_option('vmnet').enabled()
>      error('vmnet.framework API is outdated')
> @@ -2713,7 +2714,7 @@ config_host_data.set('CONFIG_RTNETLINK',
>  config_host_data.set('CONFIG_SYSMACROS',
>                       cc.has_header_symbol('sys/sysmacros.h', 'makedev'))
>  config_host_data.set('HAVE_OPTRESET',
> -                     cc.has_header_symbol('getopt.h', 'optreset'))
> +                     cc.has_header_symbol('getopt.h', 'optreset', args: '-Wno-unused-value'))
>  config_host_data.set('HAVE_IPPROTO_MPTCP',
>                       cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
>  
> @@ -2731,10 +2732,12 @@ config_host_data.set('HAVE_BLK_ZONE_REP_CAPACITY',
>  # has_type
>  config_host_data.set('CONFIG_IOVEC',
>                       cc.has_type('struct iovec',
> -                                 prefix: '#include <sys/uio.h>'))
> +                                 prefix: '#include <sys/uio.h>',
> +                                 args: '-Wno-unused-value'))
>  config_host_data.set('HAVE_UTMPX',
>                       cc.has_type('struct utmpx',
> -                                 prefix: '#include <utmpx.h>'))
> +                                 prefix: '#include <utmpx.h>',
> +                                 args: '-Wno-unused-value'))
>  
>  config_host_data.set('CONFIG_EVENTFD', cc.links('''
>    #include <sys/eventfd.h>
Philippe Mathieu-Daudé Dec. 19, 2024, 5:39 p.m. UTC | #2 
On 19/12/24 01:37, Nicholas Piggin wrote:
> On Thu Dec 19, 2024 at 4:21 AM AEST, Philippe Mathieu-Daudé wrote:
>> When running Clang static analyzer on macOS I'm getting:
>>
>>    include/qemu/osdep.h:634:8: error: redefinition of 'iovec'
>>      634 | struct iovec {
>>          |        ^
>>    /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/sys/_types/_iovec_t.h:31:8: note: previous definition is here
>>       31 | struct iovec {
>>          |        ^
>>    1 error generated.
>>
>> Looking at meson-logs.txt, the analyzer enables -Wunused-value
>> making meson generated code to fail:
>>
>>      Code:
>>      #include <sys/uio.h>
>>              void bar(void) {
>>                  sizeof(struct iovec);
>>              }
>>      -----------
>>      stderr:
>>      meson-private/tmpe8_1b_00/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
>>          3 |             sizeof(struct iovec);
>>            |             ^~~~~~~~~~~~~~~~~~~~
>>      1 error generated.
>>      -----------
>>      Checking for type "struct iovec" : NO
>>
>>      Code:
>>      #include <utmpx.h>
>>              void bar(void) {
>>                  sizeof(struct utmpx);
>>              }
>>      -----------
>>      stderr:
>>      meson-private/tmp3n0u490p/testfile.c:3:13: error: expression result unused [-Werror,-Wunused-value]
>>          3 |             sizeof(struct utmpx);
>>            |             ^~~~~~~~~~~~~~~~~~~~
>>      1 error generated.
>>      -----------
>>      Checking for type "struct utmpx" : NO
>>
>>      Code:
>>
>>              #include <getopt.h>
>>              int main(void) {
>>                  /* If it's not defined as a macro, try to use as a symbol */
>>                  #ifndef optreset
>>                      optreset;
>>                  #endif
>>                  return 0;
>>              }
>>      -----------
>>      stderr:
>>      meson-private/tmp1rzob_os/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>>          6 |                 optreset;
>>            |                 ^~~~~~~~
>>      1 error generated.
>>      -----------
>>      Header "getopt.h" has symbol "optreset" : NO
>>
>>      Code:
>>
>>              #include <vmnet/vmnet.h>
>>              int main(void) {
>>                  /* If it's not defined as a macro, try to use as a symbol */
>>                  #ifndef VMNET_BRIDGED_MODE
>>                      VMNET_BRIDGED_MODE;
>>                  #endif
>>                  return 0;
>>              }
>>      -----------
>>      stderr:
>>      meson-private/tmpl9jgsxpt/testfile.c:6:17: error: expression result unused [-Werror,-Wunused-value]
>>          6 |                 VMNET_BRIDGED_MODE;
>>            |                 ^~~~~~~~~~~~~~~~~~
>>      1 error generated.
>>      -----------
>>      Header "vmnet/vmnet.h" has symbol "VMNET_BRIDGED_MODE" with dependency appleframeworks: NO
>>      ../meson.build:1174: WARNING: vmnet.framework API is outdated, disabling
>>
>> Fix by explicitly disabling -Wunused-value from these meson checks.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC: Probably meson should do that in has_header_symbol() / has_type()?
> 
> I don't know about the build system to answer this, but should we
> instead disable -Werror on these tests to be a bit more future-proof?
> Compilers often add new warnings or catch more cases of existing
> warnings.

Sorry, I didn't mean to include this patch in this series. I happen
to have my series on top of it and forgot to change the base commit.

> Alternative would be to keep -Werror but fail the build if a test
> throws a warning, but that seems like a lot more work for little
> benefit...

I'm trying to fix it on the meson side with this:

-- >8 --
diff --git a/mesonbuild/compilers/mixins/clike.py 
b/mesonbuild/compilers/mixins/clike.py
index d56547b47..9d6957973 100644
--- a/mesonbuild/compilers/mixins/clike.py
+++ b/mesonbuild/compilers/mixins/clike.py
@@ -360,7 +360,7 @@ class CLikeCompiler(Compiler):
          int main(void) {{
              /* If it's not defined as a macro, try to use as a symbol */
              #ifndef {symbol}
-                {symbol};
+            (void) {symbol};
              #endif
              return 0;
          }}'''
@@ -885,7 +885,8 @@ class CLikeCompiler(Compiler):
                   dependencies: T.Optional[T.List['Dependency']] = 
None) -> T.Tuple[bool, bool]:
          t = f'''{prefix}
          void bar(void) {{
-            (void) sizeof({typename});
+            size_t foo = sizeof({typename});
+            (void) foo;
          }}'''
          return self.compiles(t, env, extra_args=extra_args,
                               dependencies=dependencies)
---
Richard Henderson Dec. 19, 2024, 6:14 p.m. UTC | #3 
On 12/19/24 09:39, Philippe Mathieu-Daudé wrote:
> @@ -885,7 +885,8 @@ class CLikeCompiler(Compiler):
>                    dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, 
> bool]:
>           t = f'''{prefix}
>           void bar(void) {{
> -            (void) sizeof({typename});
> +            size_t foo = sizeof({typename});
> +            (void) foo;
>           }}'''

As I mentioned elsewhere, sizeof is a compile-time constant.
The function wrapper is getting in the way.  This can be just

unsigned long foo = sizeof({typename});

I.e. initialization of a global variable, with no need for <stddef.h> or any other system 
header for size_t.


r~
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index 85f74854735..9d93dcd95d7 100644
--- a/meson.build
+++ b/meson.build
@@ -1189,7 +1189,8 @@  cocoa = dependency('appleframeworks',
 vmnet = dependency('appleframeworks', modules: 'vmnet', required: get_option('vmnet'))
 if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
                                               'VMNET_BRIDGED_MODE',
-                                              dependencies: vmnet)
+                                              dependencies: vmnet,
+                                              args: '-Wno-unused-value')
   vmnet = not_found
   if get_option('vmnet').enabled()
     error('vmnet.framework API is outdated')
@@ -2713,7 +2714,7 @@  config_host_data.set('CONFIG_RTNETLINK',
 config_host_data.set('CONFIG_SYSMACROS',
                      cc.has_header_symbol('sys/sysmacros.h', 'makedev'))
 config_host_data.set('HAVE_OPTRESET',
-                     cc.has_header_symbol('getopt.h', 'optreset'))
+                     cc.has_header_symbol('getopt.h', 'optreset', args: '-Wno-unused-value'))
 config_host_data.set('HAVE_IPPROTO_MPTCP',
                      cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
 
@@ -2731,10 +2732,12 @@  config_host_data.set('HAVE_BLK_ZONE_REP_CAPACITY',
 # has_type
 config_host_data.set('CONFIG_IOVEC',
                      cc.has_type('struct iovec',
-                                 prefix: '#include <sys/uio.h>'))
+                                 prefix: '#include <sys/uio.h>',
+                                 args: '-Wno-unused-value'))
 config_host_data.set('HAVE_UTMPX',
                      cc.has_type('struct utmpx',
-                                 prefix: '#include <utmpx.h>'))
+                                 prefix: '#include <utmpx.h>',
+                                 args: '-Wno-unused-value'))
 
 config_host_data.set('CONFIG_EVENTFD', cc.links('''
   #include <sys/eventfd.h>