On Fri, Mar 28, 2025 at 01:45:54PM +0000, Andre Przywara wrote: > On Fri, 28 Mar 2025 10:39:59 +0000 > Andre Przywara <andre.przyw...@arm.com> wrote: > > Hi, > > > On Fri, 28 Mar 2025 11:28:05 +0100 > > Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > > > Hi, > > > > > On 27.03.25 16:32, Andre Przywara wrote: > > > > C's implicit fallthrough behaviour in switch/case statements can lead to > > > > subtle bugs. Quite some while ago many compilers introduced warnings in > > > > those cases, requiring intentional fallthrough's to be annotated. > > > > > > > > So far we were not enabling that compiler option, so many ambiguities > > > > and some bugs in the code went unnoticed. > > > > > > > > This series adds the required annotations in code paths that the first > > > > stage of the U-Boot CI covers. There is a large number of cases left > > > > in the libbz2 code. The usage of switch/case is borderline insane there, > > > > labels are hidden in macros, and there are no breaks, but just goto's. > > > > Upstream still uses very similar code, without any annotations. I still > > > > am not 100% sure those are meant to fall through or not, and plan to do > > > > further investigations, but didn't want to hold the rest of the patches > > > > back. You can see for yourself by applying patch 18/18 and building for > > > > sandbox64, for instance. > > > > > > Can we use something like > > > > > > CFLAGS_REMOVE_bzlib.o = -Wimplicit-fallthrough > > > > Ah, didn't know we have that in U-Boot as well! Sounds promising, and > > fixes the sandbox build for me (when using bzlib_decompress.o). I will add > > this to the last patch and will check what the CI has to say about this. > > So for the records: silencing the libbzip2 warnings uncovered a whole new > bunch of warnings, in stage 1 still. Some compilers used in the CI (clang?) > seem to be more picky about how to annotate, so a pure comment (/* fall > through */) would not cut it, it has to be the attribute - provided by our > "statement macro". > So I fixed those quickly, stuffed them into some patch, and now the first > CI stage (test.py) passes - but only to uncover a large number of new > warnings in the world build. > So I will keep on patching, as some kind of procrastination project ;-)
So, for the flag, what I would like is to borrow the CONFIG_CC_IMPLICIT_FALLTHROUGH logic from the kernel. Then we should be able to do: CFLAGS_REMOVE_bzlib.o += $(CONFIG_CC_IMPLICIT_FALLTHROUGH) I think. Then as follow-up convert any of our just /* fallthrough */ to 'fallthrough;'. -- Tom
signature.asc
Description: PGP signature
