Re: [PATCH] MIPS: tlbex: Explicitly compare _PAGE_NO_EXEC against 0

[Thomas Bogendoerfer <tsbogend@alpha.franken.de>]

On Tue, Aug 02, 2022 at 10:59:36AM -0700, Nathan Chancellor wrote:
> When CONFIG_XPA is enabled, Clang warns:
> 
>   arch/mips/mm/tlbex.c:629:24: error: converting the result of '<<' to a boolean; did you mean '(1 << _PAGE_NO_EXEC_SHIFT) != 0'? [-Werror,-Wint-in-bool-context]
>           if (cpu_has_rixi && !!_PAGE_NO_EXEC) {
>                               ^
>   arch/mips/include/asm/pgtable-bits.h:174:28: note: expanded from macro '_PAGE_NO_EXEC'
>   # define _PAGE_NO_EXEC          (1 << _PAGE_NO_EXEC_SHIFT)
>                                      ^
>   arch/mips/mm/tlbex.c:2568:24: error: converting the result of '<<' to a boolean; did you mean '(1 << _PAGE_NO_EXEC_SHIFT) != 0'? [-Werror,-Wint-in-bool-context]
>           if (!cpu_has_rixi || !_PAGE_NO_EXEC) {
>                                 ^
>   arch/mips/include/asm/pgtable-bits.h:174:28: note: expanded from macro '_PAGE_NO_EXEC'
>   # define _PAGE_NO_EXEC          (1 << _PAGE_NO_EXEC_SHIFT)
>                                      ^
>   2 errors generated.
> 
> _PAGE_NO_EXEC can be '0' or '1 << _PAGE_NO_EXEC_SHIFT' depending on the
> build and runtime configuration, which is what the negation operators
> are trying to convey. To silence the warning, explicitly compare against
> 0 so the result of the '<<' operator is not implicitly converted to a
> boolean.
> 
> According to its documentation, GCC enables -Wint-in-bool-context with
> -Wall but this warning is not visible when building the same
> configuration with GCC. It appears GCC only warns when compiling C++,
> not C, although the documentation makes no note of this:
> https://godbolt.org/z/x39q3brxf
> 
> Reported-by: Sudip Mukherjee (Codethink) <sudipm.mukherjee@gmail.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  arch/mips/mm/tlbex.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> index 8dbbd99fc7e8..be4d4670d649 100644
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -626,7 +626,7 @@ static __maybe_unused void build_convert_pte_to_entrylo(u32 **p,
>  		return;
>  	}
>  
> -	if (cpu_has_rixi && !!_PAGE_NO_EXEC) {
> +	if (cpu_has_rixi && _PAGE_NO_EXEC != 0) {
>  		if (fill_includes_sw_bits) {
>  			UASM_i_ROTR(p, reg, reg, ilog2(_PAGE_GLOBAL));
>  		} else {
> @@ -2565,7 +2565,7 @@ static void check_pabits(void)
>  	unsigned long entry;
>  	unsigned pabits, fillbits;
>  
> -	if (!cpu_has_rixi || !_PAGE_NO_EXEC) {
> +	if (!cpu_has_rixi || _PAGE_NO_EXEC == 0) {
>  		/*
>  		 * We'll only be making use of the fact that we can rotate bits
>  		 * into the fill if the CPU supports RIXI, so don't bother
> 
> base-commit: 7d0d3fa7339ed5a06d6608b7cde9f079eba62bb1
> -- 
> 2.37.1

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]