Re: [PATCH v3 0/3] Add compile time sanity check of GENMASK inputs

From: Rikard Falkeborn
Date: Sat Oct 05 2019 - 15:52:21 EST


On Sun, Aug 11, 2019 at 08:49:35PM +0200, Rikard Falkeborn wrote:
> 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.
>

Ping.

The current status is that patch 1 has been merged into Linus tree
through the x86 tree. The patches mentioned about actually fixing the
existing misuses have all been merged except two of Joe Perches patches
[0], [1], but those patches fixes misuse in unused macros, and will not
affect anyones build.

I have testbuilt the two remaining patches rebased on top of Linus tree
and on linux-next for aarch64 and x86 without seeing any problems (when
v2 of the series went into linux-next, those were the only archs where
any problems were spotted (build warning on x86, build error on
aarch64)).

[0] https://lore.kernel.org/lkml/cddd7ad7e9f81dec1e86c106f04229d21fc21920.1562734889.git.joe@xxxxxxxxxxx/
[1] https://lore.kernel.org/lkml/d149d2851f9aa2425c927cb8e311e20c4b83e186.1562734889.git.joe@xxxxxxxxxxx/

Rikard