October 2021 ClangBuiltLinux Work

This was a bit of a shorter month for me, as I took some vacation at the beginning of the month to recouperate and meet half of my girlfriend’s extended family and friends for the first time. Thankfully, the rest of the ClangBuiltLinux team was able to keep everything churning along in my absence and I was able to be super productive the rest of the month once I returned.

Occasionally, I will forget to link something from the mailing list in this post. To see my full mailing list activity (patches, reviews, and reports), you can view it on lore.kernel.org.

Linux kernel patches

  • -Wbitwise-instead-of-logical: A new warning in LLVM exposed a few places in the kernel where bitwise operations were being used with boolean expressions with side effects, which may be undesirable (ChromeOS had a massive bug due to this behavior…). Cleaning these up in the kernel was done with this patch and the following six here. Luckily, there were no bugs but getting the kernel free of this warning will allow new instances to pop up clearly.

    • Input: touchscreen - Avoid bitwise vs logical OR warning (v1)
    • drm/i915: Avoid bitwise vs logical OR warning in snb_wm_latency_quirk() (v1)
    • staging: wlan-ng: Avoid bitwise vs logical OR warning in hfa384x_usb_throttlefn() (v1)
    • platform/x86: thinkpad_acpi: Fix bitwise vs. logical warning (v1)
    • nfp: bpf: Fix bitwise vs. logical OR warning (v1)
    • lib: zstd: Add cast to silence clang's -Wbitwise-instead-of-logical (v1)
    • soc/tegra: fuse: Fix bitwise vs. logical OR warning (v1)
  • -Wenum-conversion: A simple implicit conversion between two different enumerated types, which can be a bug if the two values are not mapped in a one to one manner. Thankfully, the one instance of this warning that cropped up this month was not a true bug because the two enums had the same value but it was simple enough to clean up to keep the kernel warning free.

    • regulator: lp872x: Remove lp872x_dvs_state (v1)
  • -Wimplicit-fallthrough: I would argue one of the biggest deficiencies of C as a language is implicit fallthroughs in switch statements. There has been a lot of work done by Gustavo A.R. Silva to clean these up for both GCC and clang because they disagree in one specific case. These patches help keep the warning free of new instances in linux-next so that this warning can be turned on for clang in 5.16.

    • ice: Fix clang -Wimplicit-fallthrough in ice_pull_qvec_from_rc() (v1)
    • net: ax88796c: Fix clang -Wimplicit-fallthrough in ax88796c_set_mac() (v1)
    • ASoC: qdsp6: audioreach: Fix clang -Wimplicit-fallthrough (v1):
  • -Winfinite-recursion: This is a super interesting warning and the first time I have ever seen an instance of this warning in the kernel since I started building with clang a little over three years ago. In the kernel, it is common to have a function name then the same function name with two underscores before it (e.g. foo() and __foo()) to signify that foo() holds a lock while __foo() does not so that more code can be reused. In this particular instance, __foo() was created but the newly created foo() was not updated properly.

    • iio: frequency: adrf6780: Fix adrf6780_spi_{read,write}() (v1)
  • -Wpointer-bool-conversion: This warning points out that arrays in structures cannot be NULL when it is not the first member in said structure so the check does not do anything. The author may have intended to have a different check (such as the first element of the array being 0) or the check can just be removed. In this case, it was the latter.

    • net: ax88796c: Remove pointless check in ax88796c_open() (v1)
  • -Wuninitialized/-Wsometimes-uninitialized: Unfortunately, because the kernel is written in C, uninitialized variables continue to be an issue, especially since GCC’s -Wmaybe-uninitialized has been disabled by default since 5.7. These will continue to pop up until GCC can be fixed but it does not seem like that will occur anytime soon as it has been a deficiency for a long time; until then, we will continue to fix them.

    • mm/memory_failure: Initialize extra_pins in me_pagecache_clean() (v1)
    • drm/msm/dpu: Remove commit and its uses in dpu_crtc_set_crc_source() (v1)
    • net/mlx5: Add esw assignment back in mlx5e_tc_sample_unoffload() (v1)

Patch review and input

For the next sections, I link directly to my first response in the thread when possible but there are times where the link is to the main post. My responses can be seen inline by going to the bottom of the thread and clicking on my name.

Reviewing patches that are submitted is incredibly important, as it helps ensure good code quality due to catching mistakes before the patches get accepted and it can help get patches accepted faster, as some maintainers will blindly pick up patches that have been reviewed by someone that they trust.

Issue triage and reporting

The unfortunate thing about working at the intersection of two projects is we will often find bugs that are not strictly related to the project, which require some triage and reporting back to the original author of the breakage so that they can be fixed and not impact our own testing. Some of these bugs fall into that category while others are issues strictly related to this project.

Tooling improvements

Behind the scenes

  • Every day that there is a new linux-next release, I rebase and build a few different kernel trees then boot and runtime test them on several different machines, including a Raspberry Pi 3 and 4, HP desktop, ASUS laptop, and Hyper-V and VMware platforms on my workstation. This is not always visible because I do not report anything unless there is something broken but it can take up to a few hours each day, depending on the amount of churn and issues uncovered.

  • While researching an old issue, I noticed that some of the links were dead. As a result, I spent some time going through our issue tracker to replace all links to other archive sites (such as lkml.org, spinics.net, and marc.info) with canonical links to lore.kernel.org, as those links are expected to be stable and not disappear; even if they did, the message ID is in the URL so it can be looked up across the internet, rather than some arbitrary URL. There is nothing worse than looking into an old issue and having a dead link (I always think of this xkcd). There is a lot of mailing list discussions that come up later so being able to access that is of the utmost importance.

Special thanks to:

comments powered by Disqus