Mailing List Archive: clang 10 -Wimplicit-fallthrough

clang 10 -Wimplicit-fallthrough

Jun 4, 2020, 4:02 PM

Post #1 of 5 (277 views)

Hi.

I upgraded my main build host and the clang -Werror builds started
failing.

This is because clang 10's -Wimplicit-fallthrough doesn't understand
/* FALLTHROUGH */ but rather requires __attribute__((fallthrough)):

clang -Wall -O2 [...] -Wimplicit-fallthrough [...] -D_XOPEN_SOURCE=600 -D_BSD_SOURCE -D_DEFAULT_SOURCE -DHAVE_CONFIG_H -c /openbsd-compat/base64.c
openbsd-compat/base64.c:284:3: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]

case 3: /* Valid, means two bytes of info */
openbsd-compat/base64.c:284:3: note: insert ´__attribute__((fallthrough));´ to silence this warning

None of our code has this (OpenSSH itself or the OpenBSD compat code) so
at the moment it makes the -Werror build useless with clang. I'd like
to add a /* FALLTHROUGH */ to our test which will effectively disable
-Wimplicit-fallthrough where it is currently not useful to us:

$ clang --version
clang version 10.0.0 (Fedora 10.0.0-1.fc32)
$ CC=clang ../../configure
[...]
checking if clang supports compile flag -Wunused-result... yes
checking if clang supports compile flag -Wimplicit-fallthrough... no
checking if clang supports link flag -Wl,-z,retpolineplt... no

Can anyone suggest a better solution? Annotating these points with a
FALLTHROUGH macro would make more work keeping the code in sync and so
is currently a non-starter.

diff --git a/aclocal.m4 b/aclocal.m4
index 25ecc49a..fca940dd 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -21,6 +21,11 @@ int main(int argc, char **argv) {
double m = l / 0.5;
long long int n = argc * 12345LL, o = 12345LL * (long long int)argc;
printf("%d %d %d %f %f %lld %lld\n", i, j, k, l, m, n, o);
+ switch(i){
+ case 0: j += i;
+ /* FALLTHROUGH */
+ default: j += k;
+ }
exit(0);
}
]])],
--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: clang 10 -Wimplicit-fallthrough [ In reply to ]

peter at stuge

Jun 5, 2020, 9:57 AM

Post #2 of 5 (274 views)

Permalink

Darren Tucker wrote:
> Annotating these points with a FALLTHROUGH macro would make more
> work keeping the code in sync and so is currently a non-starter.

Upstream OpenSSH/OpenBSD has no interest in embracing such a macro?

Isn't there some clang/llvm intent in OpenBSD?

> diff --git a/aclocal.m4 b/aclocal.m4
> index 25ecc49a..fca940dd 100644
> --- a/aclocal.m4
> +++ b/aclocal.m4
> @@ -21,6 +21,11 @@ int main(int argc, char **argv) {
> double m = l / 0.5;
> long long int n = argc * 12345LL, o = 12345LL * (long long int)argc;
> printf("%d %d %d %f %f %lld %lld\n", i, j, k, l, m, n, o);
> + switch(i){
> + case 0: j += i;
> + /* FALLTHROUGH */
> + default: j += k;
> + }

Are you thinking to also add a test case for when it's missing?

//Peter
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: clang 10 -Wimplicit-fallthrough [ In reply to ]

t.glaser at tarent

Jun 5, 2020, 10:19 AM

Post #3 of 5 (274 views)

Permalink

On Fri, 5 Jun 2020, Peter Stuge wrote:

> Darren Tucker wrote:
> > Annotating these points with a FALLTHROUGH macro would make more
> > work keeping the code in sync and so is currently a non-starter.
>
> Upstream OpenSSH/OpenBSD has no interest in embracing such a macro?

It cannot be implemented as macro, because cpp normally strips
comments on expansion (unless -Wp,-CC is given), so you cannot
define it to expand to /* FALLTHROUGH */ for other compilers
and lint. It’s also KNF to use the comment.

bye,
//mirabilos
--
tarent solutions GmbH
Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/
Tel: +49 228 54881-393 • Fax: +49 228 54881-235
HRB 5168 (AG Bonn) • USt-ID (VAT): DE122264941
Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: clang 10 -Wimplicit-fallthrough [ In reply to ]

Todd.Miller at sudo

Jun 5, 2020, 11:23 AM

Post #4 of 5 (274 views)

Permalink

On Fri, 05 Jun 2020 09:02:01 +1000, Darren Tucker wrote:

> This is because clang 10's -Wimplicit-fallthrough doesn't understand
> /* FALLTHROUGH */ but rather requires __attribute__((fallthrough)):

Correct. Older versions of clang ignored -Wimplicit-fallthrough
for C code because there wasn't a way to annotate the fallthrough.
Unlike gcc, clang doesn't recognize /* FALLTHROUGH */ comments.
The full list of patterns accepted by gcc are docutmented in
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough

> None of our code has this (OpenSSH itself or the OpenBSD compat code) so
> at the moment it makes the -Werror build useless with clang. I'd like
> to add a /* FALLTHROUGH */ to our test which will effectively disable
> -Wimplicit-fallthrough where it is currently not useful to us:

That seems like the best approach. Currently, clang's
-Wimplicit-fallthrough option is deficient, so it is best to just
avoid using it.

- todd
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: clang 10 -Wimplicit-fallthrough [ In reply to ]

dtucker at dtucker

Jun 5, 2020, 6:45 PM

Post #5 of 5 (274 views)

Permalink

On Sat, 6 Jun 2020 at 03:07, Peter Stuge <peter@stuge.se> wrote:
>
> Darren Tucker wrote:
> > Annotating these points with a FALLTHROUGH macro would make more
> > work keeping the code in sync and so is currently a non-starter.
>
> Upstream OpenSSH/OpenBSD has no interest in embracing such a macro?
> Isn't there some clang/llvm intent in OpenBSD?

OpenBSD uses clang 8 or gcc, depending on the architecture. From
brief testing, clang 8 doesn't implement __attribute__((fallthrough))
or /* FALLTHROUGH */.

That means to implement it in OpenBSD it'd require at least updating
the system compiler to clang 10 and possibly update gcc architectures
to clang before changing all of the FALLTHROUGHs. This is scope creep
well beyond what I'm willing to engage in :-)

> Are you thinking to also add a test case for when it's missing?

No, because this test is used to with -Werror to determine which
warnings the compiler accepts. If I did that it'll cause gcc to flag
it as an error and have the effect of disabling the warnings on gcc
too.

--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev