[Qemu-devel] [RFC PATCH 0/3] add QEMU_WARN_NONNULL_ARGS() macro

[Qemu-devel] [RFC PATCH 0/3] add QEMU_WARN_NONNULL_ARGS() macro

[Philippe Mathieu-Daudé]

Some old PoC series I remember after reading
http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg03545.html

I had few more changes but then I found the code was harder to read
so I didn't continue further.
Only 2 patches are included as example.

This might still be useful in few cases, so I'm still sending as RFC
to have different thoughts.

This macro is, however, helpful to the Clang static analizer (reducing
false positive).

BTW another useful macro for the static analizer I used is:

    #define QEMU_FALLTHROUGH __attribute__((fallthrough))

It replaces the /* fall through */ comment, i.e.:

    switch (rap) {
    case BCR_SWS:
        if (!(CSR_STOP(s) || CSR_SPND(s)))
            return;
        val &= ~0x0300;
        QEMU_FALLTHROUGH;
    case BCR_LNKST:
    case BCR_LED1:
    case BCR_LED2:
    case BCR_LED3:
    case BCR_MC:
    case BCR_FDC:
    case BCR_BSBC:
    case BCR_EECAS:
    case BCR_PLAT:
        s->bcr[rap] = val;
        break;

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  compiler: add QEMU_WARN_NONNULL_ARGS()
  virtio: let virtio_add/clear_feature() use QEMU_WARN_NONNULL_ARGS()
  utils: let qemu_find_file() use QEMU_WARN_NONNULL_ARGS()

 include/hw/virtio/virtio.h | 2 ++
 include/qemu-common.h      | 2 +-
 include/qemu/compiler.h    | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.15.1

[Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/qemu/compiler.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 340e5fdc09..d9b2489391 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -26,6 +26,8 @@
 
 #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
 
+#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
+
 #define QEMU_SENTINEL __attribute__((sentinel))
 
 #if QEMU_GNUC_PREREQ(4, 3)
-- 
2.15.1

Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

[Daniel P. Berrange]

On Wed, Jan 17, 2018 at 10:18:19AM -0300, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/qemu/compiler.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 340e5fdc09..d9b2489391 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -26,6 +26,8 @@
>  
>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
>  
> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))

If we take this, it should come with a warning attached to it, because
it has really nasty behaviour with GCC. Consider code like:

  void foo(void *bar) __attribute__((nonnull(1)));

  ...

  void foo(void *bar) { if (!bar) return; }

GCC may or may not warn you about passing NULL for the 'bar'
parameter, but it will none the less assume nothing passes
NULL, and thus remove the 'if (!bar)' conditional during
optimization. IOW, adding nonnull annotations can actually
make your code less robust :-(

After having a number of crashes in libvirt caused by gcc
optimizing out checks for NULL, we now only define nonnull
when running under static analysis (coverity) and not when
compiling normally.

https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/internal.h;h=5895030415968d72200599e8a59bbf01ffc2d5a3;hb=HEAD#l162

The 2 functions you've added nonnull attrs to look safe enough,
but people might unwittingly use this elsewhere in QEMU in future
not realizing the side-effect it has.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

[Philippe Mathieu-Daudé]

On 01/17/2018 10:32 AM, Daniel P. Berrange wrote:
> On Wed, Jan 17, 2018 at 10:18:19AM -0300, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/qemu/compiler.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
>> index 340e5fdc09..d9b2489391 100644
>> --- a/include/qemu/compiler.h
>> +++ b/include/qemu/compiler.h
>> @@ -26,6 +26,8 @@
>>  
>>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
>>  
>> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
> 
> If we take this, it should come with a warning attached to it, because
> it has really nasty behaviour with GCC. Consider code like:
> 
>   void foo(void *bar) __attribute__((nonnull(1)));
> 
>   ...
> 
>   void foo(void *bar) { if (!bar) return; }
> 
> GCC may or may not warn you about passing NULL for the 'bar'
> parameter, but it will none the less assume nothing passes
> NULL, and thus remove the 'if (!bar)' conditional during
> optimization. IOW, adding nonnull annotations can actually
> make your code less robust :-(

TIL!

> After having a number of crashes in libvirt caused by gcc
> optimizing out checks for NULL, we now only define nonnull
> when running under static analysis (coverity) and not when
> compiling normally.
> 
> https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/internal.h;h=5895030415968d72200599e8a59bbf01ffc2d5a3;hb=HEAD#l162

Why do you use __attribute__(()) ? Isn't this enough:

#if defined __clang_analyzer__ || defined __COVERITY__
#define QEMU_STATIC_ANALYSIS 1
+#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
+#else
+#define QEMU_WARN_NONNULL_ARGS(args...)
#endif

> The 2 functions you've added nonnull attrs to look safe enough,
> but people might unwittingly use this elsewhere in QEMU in future
> not realizing the side-effect it has.
> 
> Regards,
> Daniel
> 

Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

[Daniel P. Berrange]

On Wed, Jan 17, 2018 at 11:33:34AM -0300, Philippe Mathieu-Daudé wrote:
> On 01/17/2018 10:32 AM, Daniel P. Berrange wrote:
> > On Wed, Jan 17, 2018 at 10:18:19AM -0300, Philippe Mathieu-Daudé wrote:
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >>  include/qemu/compiler.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> >> index 340e5fdc09..d9b2489391 100644
> >> --- a/include/qemu/compiler.h
> >> +++ b/include/qemu/compiler.h
> >> @@ -26,6 +26,8 @@
> >>  
> >>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> >>  
> >> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
> > 
> > If we take this, it should come with a warning attached to it, because
> > it has really nasty behaviour with GCC. Consider code like:
> > 
> >   void foo(void *bar) __attribute__((nonnull(1)));
> > 
> >   ...
> > 
> >   void foo(void *bar) { if (!bar) return; }
> > 
> > GCC may or may not warn you about passing NULL for the 'bar'
> > parameter, but it will none the less assume nothing passes
> > NULL, and thus remove the 'if (!bar)' conditional during
> > optimization. IOW, adding nonnull annotations can actually
> > make your code less robust :-(
> 
> TIL!
> 
> > After having a number of crashes in libvirt caused by gcc
> > optimizing out checks for NULL, we now only define nonnull
> > when running under static analysis (coverity) and not when
> > compiling normally.
> > 
> > https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/internal.h;h=5895030415968d72200599e8a59bbf01ffc2d5a3;hb=HEAD#l162
> 
> Why do you use __attribute__(()) ? Isn't this enough:

No idea offhand - Eric wrote this so perhaps he had a reason for that
else branch style.

> 
> #if defined __clang_analyzer__ || defined __COVERITY__
> #define QEMU_STATIC_ANALYSIS 1
> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
> +#else
> +#define QEMU_WARN_NONNULL_ARGS(args...)
> #endif
> 
> > The 2 functions you've added nonnull attrs to look safe enough,
> > but people might unwittingly use this elsewhere in QEMU in future
> > not realizing the side-effect it has.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

[Eric Blake]

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

On 01/17/2018 08:39 AM, Daniel P. Berrange wrote:

>>>
>>> GCC may or may not warn you about passing NULL for the 'bar'
>>> parameter, but it will none the less assume nothing passes
>>> NULL, and thus remove the 'if (!bar)' conditional during
>>> optimization. IOW, adding nonnull annotations can actually
>>> make your code less robust :-(

The gcc bug mentioned in the libvirt code says that newer gcc may be
smarter than when we added the libvirt workaround:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308

But I haven't played with it in a long time (the sour experience with
misoptimized code with no warning has made me wary of re-trying).


>>
>> Why do you use __attribute__(()) ? Isn't this enough:
> 
> No idea offhand - Eric wrote this so perhaps he had a reason for that
> else branch style.
> 
>>
>> #if defined __clang_analyzer__ || defined __COVERITY__
>> #define QEMU_STATIC_ANALYSIS 1
>> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
>> +#else
>> +#define QEMU_WARN_NONNULL_ARGS(args...)
>> #endif

My reasoning for using the empty attribute was to at least ensure that
the gcc compilation agrees that the annotation is syntactically valid
(nothing worse than sticking an __attribute__ code in a place that gcc
doesn't like, but only for static checker builds, so you don't learn
about it right away).  Defining the macro to nothing, rather than an
empty attribute, makes it harder for the common-case compilation to
detect placement problems.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

[Philippe Mathieu-Daudé]

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

On 01/17/2018 11:56 AM, Eric Blake wrote:
> On 01/17/2018 08:39 AM, Daniel P. Berrange wrote:
> 
>>>>
>>>> GCC may or may not warn you about passing NULL for the 'bar'
>>>> parameter, but it will none the less assume nothing passes
>>>> NULL, and thus remove the 'if (!bar)' conditional during
>>>> optimization. IOW, adding nonnull annotations can actually
>>>> make your code less robust :-(
> 
> The gcc bug mentioned in the libvirt code says that newer gcc may be
> smarter than when we added the libvirt workaround:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308
> 
> But I haven't played with it in a long time (the sour experience with
> misoptimized code with no warning has made me wary of re-trying).

I have been there...

>>>
>>> Why do you use __attribute__(()) ? Isn't this enough:
>>
>> No idea offhand - Eric wrote this so perhaps he had a reason for that
>> else branch style.
>>
>>>
>>> #if defined __clang_analyzer__ || defined __COVERITY__
>>> #define QEMU_STATIC_ANALYSIS 1
>>> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
>>> +#else
>>> +#define QEMU_WARN_NONNULL_ARGS(args...)
>>> #endif
> 
> My reasoning for using the empty attribute was to at least ensure that
> the gcc compilation agrees that the annotation is syntactically valid
> (nothing worse than sticking an __attribute__ code in a place that gcc
> doesn't like, but only for static checker builds, so you don't learn
> about it right away).  Defining the macro to nothing, rather than an
> empty attribute, makes it harder for the common-case compilation to
> detect placement problems.

I wouldn't have thought of that, thanks!

Phil.


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

[Qemu-devel] [RFC PATCH 2/3] virtio: let virtio_add/clear_feature() use QEMU_WARN_NONNULL_ARGS()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/virtio/virtio.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 098bdaaea3..d63be28039 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -292,12 +292,14 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
 VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
 VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
 
+QEMU_WARN_NONNULL_ARGS(1)
 static inline void virtio_add_feature(uint64_t *features, unsigned int fbit)
 {
     assert(fbit < 64);
     *features |= (1ULL << fbit);
 }
 
+QEMU_WARN_NONNULL_ARGS(1)
 static inline void virtio_clear_feature(uint64_t *features, unsigned int fbit)
 {
     assert(fbit < 64);
-- 
2.15.1

[Qemu-devel] [RFC PATCH 3/3] utils: let qemu_find_file() use QEMU_WARN_NONNULL_ARGS()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/qemu-common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 05319b9ddc..337203e7c3 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -134,7 +134,7 @@ const char *qemu_get_vm_name(void);
 
 #define QEMU_FILE_TYPE_BIOS   0
 #define QEMU_FILE_TYPE_KEYMAP 1
-char *qemu_find_file(int type, const char *name);
+char *qemu_find_file(int type, const char *name) QEMU_WARN_NONNULL_ARGS(2);
 
 /* OS specific functions */
 void os_setup_early_signal_handling(void);
-- 
2.15.1

Re: [Qemu-devel] [RFC PATCH 0/3] add QEMU_WARN_NONNULL_ARGS() macro

[Philippe Mathieu-Daudé]

On 01/17/2018 10:18 AM, Philippe Mathieu-Daudé wrote:
> Some old PoC series I remember after reading
> http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg03545.html
> 
> I had few more changes but then I found the code was harder to read
> so I didn't continue further.
> Only 2 patches are included as example.
> 
> This might still be useful in few cases, so I'm still sending as RFC
> to have different thoughts.
> 
> This macro is, however, helpful to the Clang static analizer (reducing
> false positive).
> 
> BTW another useful macro for the static analizer I used is:
> 
>     #define QEMU_FALLTHROUGH __attribute__((fallthrough))

and:

#define QEMU_STATIC_ANALYSIS_ASSERT(expression) assert(expression)

i.e.:

linux-user/syscall.c:5581:9: warning: Dereference of undefined pointer value
        if (*host_rt_dev_ptr != 0) {
            ^~~~~~~~~~~~~~~~

This can safely be silenced using:

     unlock_user(argptr, arg, 0);
+    QEMU_STATIC_ANALYSIS_ASSERT(host_rt_dev_ptr);
     ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
     if (*host_rt_dev_ptr != 0) {
         ...

Or:

target/mips/msa_helper.c:1008:23: warning: The result of the '<<'
expression is undefined
        int64_t r_bit = 1 << (DF_BITS(df) - 2);
                        ~~^~~~~~~~~~~~~~~~~~~~

Using:

static inline int64_t msa_mulr_q_df(uint32_t df, int64_t arg1, int64_t arg2)
{
     int64_t q_min = DF_MIN_INT(df);
     int64_t q_max = DF_MAX_INT(df);
-    int64_t r_bit = 1 << (DF_BITS(df) - 2);
+    int64_t r_bit;

+    QEMU_STATIC_ANALYSIS_ASSERT(DF_BITS(df) < 32);
+    r_bit = 1 << (DF_BITS(df) - 2);

Similar:

target/arm/helper.c:4283:25: warning: The result of the '>>' expression
is undefined
            len = cto32(bas >> basstart);
                        ~~~~^~~~~~~~~~~

Using:

 basstart = ctz32(bas);
+QEMU_STATIC_ANALYSIS_ASSERT(basstart <= 8); /* bas is at most 8-bit */
 len = cto32(bas >> basstart);

Another:

monitor.c:1481:26: warning: Access to field 'name' results in a
dereference of a null pointer (loaded from variable 'mr')
                       addr, mr->name, ptr);
                             ^~~~~~~~

Using:

     if (local_err) {
         error_report_err(local_err);
         return;
+    } else {
+        QEMU_STATIC_ANALYSIS_ASSERT(ptr != NULL);
     }
     physaddr = vtop(ptr, &local_err);
     if (local_err) {
         error_report_err(local_err);
     } else {
         monitor_printf(mon, "Host physical address for 0x%" HWADDR_PRIx
                        " (%s) is 0x%" PRIx64 "\n",
                        addr, mr->name, (uint64_t) physaddr);

etc..

Too many false positive leads to unused report,
but personally I still prefer readable code.

> 
> It replaces the /* fall through */ comment, i.e.:
> 
>     switch (rap) {
>     case BCR_SWS:
>         if (!(CSR_STOP(s) || CSR_SPND(s)))
>             return;
>         val &= ~0x0300;
>         QEMU_FALLTHROUGH;
>     case BCR_LNKST:
>     case BCR_LED1:
>     case BCR_LED2:
>     case BCR_LED3:
>     case BCR_MC:
>     case BCR_FDC:
>     case BCR_BSBC:
>     case BCR_EECAS:
>     case BCR_PLAT:
>         s->bcr[rap] = val;
>         break;
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (3):
>   compiler: add QEMU_WARN_NONNULL_ARGS()
>   virtio: let virtio_add/clear_feature() use QEMU_WARN_NONNULL_ARGS()
>   utils: let qemu_find_file() use QEMU_WARN_NONNULL_ARGS()
> 
>  include/hw/virtio/virtio.h | 2 ++
>  include/qemu-common.h      | 2 +-
>  include/qemu/compiler.h    | 2 ++
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 

Re: [Qemu-devel] [RFC PATCH 0/3] add QEMU_WARN_NONNULL_ARGS() macro

[Richard Henderson]

On 01/17/2018 05:18 AM, Philippe Mathieu-Daudé wrote:
> BTW another useful macro for the static analizer I used is:
> 
>     #define QEMU_FALLTHROUGH __attribute__((fallthrough))
> 
> It replaces the /* fall through */ comment, i.e.:

That's unfortunate.  Does it help if you use the actual lint spelling of /*
FALLTHRU */?


r~

Re: [Qemu-devel] [RFC PATCH 0/3] add QEMU_WARN_NONNULL_ARGS() macro

[Eric Blake]

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

On 01/17/2018 09:36 AM, Richard Henderson wrote:
> On 01/17/2018 05:18 AM, Philippe Mathieu-Daudé wrote:
>> BTW another useful macro for the static analizer I used is:
>>
>>     #define QEMU_FALLTHROUGH __attribute__((fallthrough))
>>
>> It replaces the /* fall through */ comment, i.e.:
> 
> That's unfortunate.  Does it help if you use the actual lint spelling of /*
> FALLTHRU */?

New gcc has this:

'-Wimplicit-fallthrough=N'
     Warn when a switch case falls through.  For example:
...

     Since there are occasions where a switch case fall through is
     desirable, GCC provides an attribute, '__attribute__
     ((fallthrough))', that is to be used along with a null statement to
     suppress this warning that would normally occur:

          switch (cond)
            {
            case 1:
              bar (0);
              __attribute__ ((fallthrough));
            default:
              ...
            }

     C++17 provides a standard way to suppress the
     '-Wimplicit-fallthrough' warning using '[[fallthrough]];' instead
     of the GNU attribute.  In C++11 or C++14 users can use
     '[[gnu::fallthrough]];', which is a GNU extension.  Instead of the
     these attributes, it is also possible to add a fallthrough comment
     to silence the warning.  The whole body of the C or C++ style
     comment should match the given regular expressions listed below.

     The option argument N specifies what kind of comments are accepted:

        * '-Wimplicit-fallthrough=0' disables the warning altogether.

        * '-Wimplicit-fallthrough=1' matches '.*' regular expression,
          any comment is used as fallthrough comment.

        * '-Wimplicit-fallthrough=2' case insensitively matches
          '.*falls?[ \t-]*thr(ough|u).*' regular expression.

        * '-Wimplicit-fallthrough=3' case sensitively matches one of the
          following regular expressions:

             * '-fallthrough'

             * '@fallthrough@'

             * 'lint -fallthrough[ \t]*'

             * '[ \t.!]*(ELSE,? |INTENTIONAL(LY)? )?
               FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)?'

             * '[ \t.!]*(Else,? |Intentional(ly)? )?
               Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)?'

             * '[ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?
               fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?'

        * '-Wimplicit-fallthrough=4' case sensitively matches one of the
          following regular expressions:

             * '-fallthrough'

             * '@fallthrough@'

             * 'lint -fallthrough[ \t]*'

             * '[ \t]*FALLTHR(OUGH|U)[ \t]*'

        * '-Wimplicit-fallthrough=5' doesn't recognize any comments as
          fallthrough comments, only attributes disable the warning.

     The comment needs to be followed after optional whitespace and
     other comments by 'case' or 'default' keywords or by a user label
     that precedes some 'case' or 'default' label.

          switch (cond)
            {
            case 1:
              bar (0);
              /* FALLTHRU */
            default:
              ...
            }

     The '-Wimplicit-fallthrough=3' warning is enabled by '-Wextra'.


No thanks to level 5, these days, you HAVE to use a macro that expands
to the attribute and/or C17 spelling, rather than relying on the lint
spelling, if you cannot control what level a user will request; although
with level 3, the lint spelling of a comment still works.  I'm not sure
which static analyzers recognize which other spellings; which is why a
macro that consistently expands to the same string known to work, rather
than 20 different ad hoc comments (many of which work, but auditing that
all of them work is a bigger task), may be worthwhile.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]