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.
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
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
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
On Sat, 6 Jun 2020 at 03:07, Peter Stuge <peter at 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.