On Mon, Oct 18, 2021 at 05:38:09PM -1000, Linus Torvalds wrote:
> On Mon, Oct 18, 2021 at 10:14 AM Nick Desaulniers
> <ndesaulniers@xxxxxxxxxx> wrote:
> >
> > Right, the patch that added the warning explicitly checks for side effects.
>
> Well, it's a bit questionable. The "side effects" are things like any
> pointer dereference, because it could fault, but if you know that
> isn't an issue, then clang basically ends up complaining about code
> that is perfectly fine. Maybe it was written that way on purpose, like
> the kvm code.
>
> Now, it's probably not worth keeping that "bitops of booleans" logic -
> if it is a noticeable optimization, it's generally something that the
> compiler should do for us, but basically clang is warning about
> perfectly valid code.
>
> And what I find absolutely disgusting is the suggested "fix" that
> clang gives you.
>
> If the warning said "maybe you meant to use a logical or (||)", then
> that would be one thing. But what clang suggests as the "fix" for the
> warning is just bad coding practice.
For what it's worth, the suggested fix is the '||' underneath the
warning text:
In file included from arch/x86/kvm/mmu/tdp_iter.c:5:
arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
return __is_bad_mt_xwr(rsvd_check, spte) |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||
arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
1 error generated.
Perhaps that hint should also be added to the warning text, like:
In file included from arch/x86/kvm/mmu/tdp_iter.c:5:
arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands; did you mean logical '||'? [-Werror,-Wbitwise-instead-of-logical]
return __is_bad_mt_xwr(rsvd_check, spte) |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||
arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
1 error generated.
It is late for me but I can push that change to the clang developers and
see what they think tomorrow if that would help?
Cheers,
Nathan
