[PATCH v3 0/3] Add compile time sanity check of GENMASK inputs
From: Rikard Falkeborn
Date: Sun Aug 11 2019 - 14:49:49 EST
Hello,
A new attempt to try to add build time validity checks of GENMASK (and
GENMASK_ULL) inputs. There main differences from v2:
Remove a define of BUILD_BUG_ON in x86/boot to avoid a compiler warning
about redefining BUILD_BUG_ON. Instead, use the common one from
include/.
Drop patch 2 in v2 where GENMASK arguments where made more verbose.
Add a cast in the BUILD_BUG_ON_ZERO macro change the type to int to
avoid the somewhat clumpsy casts of BUILD_BUG_ON_ZERO. The second patch
in this series adds such a cast to BUILD_BUG_ON_ZERO, which makes it
possible to avoid casts when using BUILD_BUG_ON_ZERO in patch 3.
I have checked all users of BUILD_BUG_ON_ZERO and I did not find a case
where adding a cast to int would affect existing users but I'd feel much
more comfortable if someone else double (or tripple) checked (there are
~80 instances plus ~10 copies in tools). Perhaps I should have CC:d
maintainers of files using BUILD_BUG_ON_ZERO?
Finally, use __builtin_constant_p instead of __is_constexpr. This avoids
pulling in kernel.h in bits.h.
Joe Perches sent a patch series to fix existing misuses, currently there
are five such misuses (which patches pending) left in Linus tree and two
(with patches pending) in linux-next. Those patches should fix all
"simple" misuses of GENMASK (cases where the arguments are numerical
constants). Pushing v2 to linux-next also revealed an arm-specific
misuse where GENMASK was used in another macro (and also broke the
arm-builds). There is a patch to fix that by Nathan Chancellor (not in
linux-next yet). Those patches should be merged before the last patch of
this series to avoid breaking builds.
Changelog
Since v2
- Use __builtin_constant_p instead of __is_constexpr to avoid pulling
in kernel.h (that include was missing in v2, so the header was no
longer builable standalone
- add cast to BUILD_BUG_ON_ZERO to make the type int
- Remove unnecessary casts due to the above
- Drop patch that renamed macro arguments
Since v1
- Add comment about why inputs are not checked when used in asm file
- Use UL(0) instead of 0
- Extract mask creation in a separate macro to improve readability
- Use high and low instead of h and l (part of this was extracted to a
separate patch)
- Updated commit message
Rikard Falkeborn (3):
x86/boot: Use common BUILD_BUG_ON
linux/build_bug.h: Change type to int
linux/bits.h: Add compile time sanity check of GENMASK inputs
arch/x86/boot/boot.h | 2 --
arch/x86/boot/main.c | 1 +
include/linux/bits.h | 21 +++++++++++++++++++--
include/linux/build_bug.h | 4 ++--
4 files changed, 22 insertions(+), 6 deletions(-)
--
2.22.0