2022-08-02 16:38:38

by Sudip Mukherjee

[permalink] [raw]
Subject: mips maltaup_xpa_defconfig build failure with clang

Hi All,

Not sure if it has been reported, mips maltaup_xpa_defconfig fails to
build with clang with the error:

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)
^

--
Regards
Sudip


2022-08-02 18:04:41

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] MIPS: tlbex: Explicitly compare _PAGE_NO_EXEC against 0

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) <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
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


2022-08-04 14:52:40

by Thomas Bogendoerfer

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

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) <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> 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 ]