[GSoC] Coreboot Coverity, weeks 5-7

Hello again! It’s been three weeks since my last post (well, one of those weeks I was on vacation, so more like two weeks), so there are many many updates to write about. If you recall from my last post, Coverity has been down for maintenance, so I started looking around for other things to do. Here is a list of some of the highlights:

  • Coverity isn’t the only static analyzer we use – coreboot also runs nightly scans using the Clang Static Analyzer, an open source tool from the LLVM project. The results from these scans aren’t as detailed as Coverity and unfortunately contain many duplicates and false positives, but I was able to submit patches for some of the issues. Usability of the analyzer is also still somewhat limited, since we have no way of recording fixes for past issues or ignoring false positives. (A framework like CodeChecker might be able to help with this.)
  • coreboot will (hopefully) soon be (almost) VLA free! Variable length arrays (VLAs) were a feature added in C99 that allows the length of an array to be determined at run time, rather than compile time. While convenient, this is also very dangerous, since it allows use of an unlimited amount of stack memory, potentially leading to stack overflow. This is especially dangerous in coreboot, which often has very little stack space to begin with. Fortunately, almost all of the VLAs in coreboot can be replaced with small fixed-sized buffers, aside from one tricky exception that will require a larger rewrite. This patch was inspired by a similar one for Linux, and is currently under review.
  • Similar to the above, coreboot will also soon be free of implicit fall throughs! Switches in C have the unfortunate property of each case statement implicitly falling through to the next one, which has led to untold number of bugs over the decades and at least 38 in coreboot itself (by Coverity’s count). GCC however recently added the -Wimplicit-fallthrough warning, which we can use to banish them for good. As of this patch, all accidental fall throughs have been eliminated, with the intentional ones marked with the /* fall through */ comment. Hopefully this is the last we see of this type of bug.
  • coreboot now has a single implementation of <stdint.h>. Previously, each of the six supported architectures (arm, arm64, mips, ppc64, riscv, and x86) had their own custom headers with arch-specific quirks, but now they’ve all been tidied up and merged together. This should make implementing further standard library headers easier to do (such as <inttypes.h>).
  • Finally, many coreboot utilities and tools such as flashrom, coreinfo, nvramtool, inteltool, ifdtool, and cbmem have stricter compiler warnings, mostly from -Wmissing-prototypes and -Wextra. Fixing and adding compiler warnings like these are very easy to do, and make great first time contributions (-Wconversion and -Wsign-compare in particular can always use more work).

That’s it for this week! Thankfully Coverity is back online now, so expect more regular blog posts in the future.