[PATCH 00/10] raise minimum GCC version to 5.1

[PATCH 00/10] raise minimum GCC version to 5.1

[Nick Desaulniers]

commit fad7cd3310db ("nbd: add the check to prevent overflow in
__nbd_ioctl()")

raised an issue from the fallback helpers added in

commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")

Specifically, the helpers for checking whether the results of a
multiplication overflowed (__unsigned_mul_overflow,
__signed_add_overflow) use the division operator when
!COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
operands on 32b hosts.

Also, because the macro is type agnostic, it is very difficult to write
a similarly type generic macro that dispatches to one of:
* div64_s64
* div64_u64
* div_s64
* div_u64

Raising the minimum supported versions allows us to remove all of the
fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
dispatching the compiler builtins.

arm64 has already raised the minimum supported GCC version to 5.1, do
this for all targets now. See the link below for the previous
discussion.

Additional patches cleaning up a few obsolete version checks.

Link: https://lore.kernel.org/all/20210909182525.372ee687@canb.auug.org.au/
Link: https://lore.kernel.org/lkml/CAK7LNASs6dvU6D3jL2GG3jW58fXfaj6VNOe55NJnTB8UPuk2pA@mail.gmail.com/
Link: https://github.com/ClangBuiltLinux/linux/issues/1438

Nick Desaulniers (10):
  Documentation: raise minimum supported version of GCC to 5.1
  compiler.h: drop fallback overflow checkers
  mm/ksm: remove old GCC 4.9+ check
  Kconfig.debug: drop GCC 5+ version check for DWARF5
  riscv: remove Kconfig check for GCC version for ARCH_RV64I
  powerpc: remove GCC version check for UPD_CONSTR
  arm64: remove GCC version check for ARCH_SUPPORTS_INT128
  Makefile: drop GCC < 5 -fno-var-tracking-assignments workaround
  compiler-gcc.h: drop checks for older GCC versions
  vmlinux.lds.h: remove old check for GCC 4.9

 Documentation/process/changes.rst    |   2 +-
 Makefile                             |   6 --
 arch/arm64/Kconfig                   |   2 +-
 arch/powerpc/include/asm/asm-const.h |  10 --
 arch/riscv/Kconfig                   |   2 +-
 include/asm-generic/vmlinux.lds.h    |   4 -
 include/linux/compiler-clang.h       |  13 ---
 include/linux/compiler-gcc.h         |   8 +-
 include/linux/overflow.h             | 138 +-------------------------
 lib/Kconfig.debug                    |   2 +-
 mm/ksm.c                             |   2 -
 scripts/min-tool-version.sh          |   8 +-
 tools/include/linux/compiler-gcc.h   |   8 +-
 tools/include/linux/overflow.h       | 140 +--------------------------
 14 files changed, 13 insertions(+), 332 deletions(-)


base-commit: 2d338201d5311bcd79d42f66df4cecbcbc5f4f2c
-- 
2.33.0.309.g3052b89438-goog

[PATCH 01/10] Documentation: raise minimum supported version of GCC to 5.1

[Nick Desaulniers]

commit fad7cd3310db ("nbd: add the check to prevent overflow in
__nbd_ioctl()")

raised an issue from the fallback helpers added in

commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")

Specifically, the helpers for checking whether the results of a
multiplication overflowed (__unsigned_mul_overflow,
__signed_add_overflow) use the division operator when
!COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
operands on 32b hosts.

Also, because the macro is type agnostic, it is very difficult to write
a similarly type generic macro that dispatches to one of:
* div64_s64
* div64_u64
* div_s64
* div_u64

Raising the minimum supported versions allows us to remove all of the
fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
dispatching the compiler builtins.

arm64 has already raised the minimum supported GCC version to 5.1, do
this for all targets now. See the link below for the previous
discussion.

Link: https://lore.kernel.org/all/20210909182525.372ee687@canb.auug.org.au/
Link: https://lore.kernel.org/lkml/CAK7LNASs6dvU6D3jL2GG3jW58fXfaj6VNOe55NJnTB8UPuk2pA@mail.gmail.com/
Link: https://github.com/ClangBuiltLinux/linux/issues/1438
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Reported-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Documentation/process/changes.rst | 2 +-
 scripts/min-tool-version.sh       | 8 +-------
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
index d3a8557b66a1..e35ab74a0f80 100644
--- a/Documentation/process/changes.rst
+++ b/Documentation/process/changes.rst
@@ -29,7 +29,7 @@ you probably needn't concern yourself with pcmciautils.
 ====================== ===============  ========================================
         Program        Minimal version       Command to check the version
 ====================== ===============  ========================================
-GNU C                  4.9              gcc --version
+GNU C                  5.1              gcc --version
 Clang/LLVM (optional)  10.0.1           clang --version
 GNU make               3.81             make --version
 binutils               2.23             ld -v
diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
index 319f92104f56..4edc708baa63 100755
--- a/scripts/min-tool-version.sh
+++ b/scripts/min-tool-version.sh
@@ -17,13 +17,7 @@ binutils)
 	echo 2.23.0
 	;;
 gcc)
-	# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
-	# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
-	if [ "$SRCARCH" = arm64 ]; then
-		echo 5.1.0
-	else
-		echo 4.9.0
-	fi
+	echo 5.1.0
 	;;
 icc)
 	# temporary
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 01/10] Documentation: raise minimum supported version of GCC to 5.1

[Nathan Chancellor]

On 9/10/2021 4:40 PM, Nick Desaulniers wrote:
> commit fad7cd3310db ("nbd: add the check to prevent overflow in
> __nbd_ioctl()")
> 
> raised an issue from the fallback helpers added in
> 
> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
> 
> Specifically, the helpers for checking whether the results of a
> multiplication overflowed (__unsigned_mul_overflow,
> __signed_add_overflow) use the division operator when
> !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
> operands on 32b hosts.

"hosts" -> "targets" or "architectures"?

It might be worth putting the error that Stephen found here?

> Also, because the macro is type agnostic, it is very difficult to write
> a similarly type generic macro that dispatches to one of:
> * div64_s64
> * div64_u64
> * div_s64
> * div_u64
> 
> Raising the minimum supported versions allows us to remove all of the
> fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
> dispatching the compiler builtins.
> 
> arm64 has already raised the minimum supported GCC version to 5.1, do
> this for all targets now. See the link below for the previous
> discussion.
> 
> Link: https://lore.kernel.org/all/20210909182525.372ee687@canb.auug.org.au/
> Link: https://lore.kernel.org/lkml/CAK7LNASs6dvU6D3jL2GG3jW58fXfaj6VNOe55NJnTB8UPuk2pA@mail.gmail.com/
> Link: https://github.com/ClangBuiltLinux/linux/issues/1438
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>   Documentation/process/changes.rst | 2 +-
>   scripts/min-tool-version.sh       | 8 +-------
>   2 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
> index d3a8557b66a1..e35ab74a0f80 100644
> --- a/Documentation/process/changes.rst
> +++ b/Documentation/process/changes.rst
> @@ -29,7 +29,7 @@ you probably needn't concern yourself with pcmciautils.
>   ====================== ===============  ========================================
>           Program        Minimal version       Command to check the version
>   ====================== ===============  ========================================
> -GNU C                  4.9              gcc --version
> +GNU C                  5.1              gcc --version
>   Clang/LLVM (optional)  10.0.1           clang --version
>   GNU make               3.81             make --version
>   binutils               2.23             ld -v
> diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
> index 319f92104f56..4edc708baa63 100755
> --- a/scripts/min-tool-version.sh
> +++ b/scripts/min-tool-version.sh
> @@ -17,13 +17,7 @@ binutils)
>   	echo 2.23.0
>   	;;
>   gcc)
> -	# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
> -	# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
> -	if [ "$SRCARCH" = arm64 ]; then
> -		echo 5.1.0
> -	else
> -		echo 4.9.0
> -	fi
> +	echo 5.1.0
>   	;;
>   icc)
>   	# temporary
> 

Re: [PATCH 01/10] Documentation: raise minimum supported version of GCC to 5.1

[Nick Desaulniers]

On Fri, Sep 10, 2021 at 4:56 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On 9/10/2021 4:40 PM, Nick Desaulniers wrote:
> > commit fad7cd3310db ("nbd: add the check to prevent overflow in
> > __nbd_ioctl()")
> >
> > raised an issue from the fallback helpers added in
> >
> > commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
> >
> > Specifically, the helpers for checking whether the results of a
> > multiplication overflowed (__unsigned_mul_overflow,
> > __signed_add_overflow) use the division operator when
> > !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
> > operands on 32b hosts.
>
> "hosts" -> "targets" or "architectures"?

Yes, will revise to "targets."

>
> It might be worth putting the error that Stephen found here?

Doh! Yes, in v2, I'll include the following additional context:
```
The following error is observed when building for 32b powerpc:
> ERROR: modpost: "__divdi3" [drivers/block/nbd.ko] undefined!

As Stephen Rothwell notes:
> The added check_mul_overflow() call is being passed 64 bit values.
> COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW is not set for this build (see
> include/linux/overflow.h).
```


>
> > Also, because the macro is type agnostic, it is very difficult to write
> > a similarly type generic macro that dispatches to one of:
> > * div64_s64
> > * div64_u64
> > * div_s64
> > * div_u64
> >
> > Raising the minimum supported versions allows us to remove all of the
> > fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
> > dispatching the compiler builtins.
> >
> > arm64 has already raised the minimum supported GCC version to 5.1, do
> > this for all targets now. See the link below for the previous
> > discussion.
> >
> > Link: https://lore.kernel.org/all/20210909182525.372ee687@canb.auug.org.au/
> > Link: https://lore.kernel.org/lkml/CAK7LNASs6dvU6D3jL2GG3jW58fXfaj6VNOe55NJnTB8UPuk2pA@mail.gmail.com/
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1438
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Reported-by: Nathan Chancellor <nathan@kernel.org>
> > Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
> > ---
> >   Documentation/process/changes.rst | 2 +-
> >   scripts/min-tool-version.sh       | 8 +-------
> >   2 files changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst
> > index d3a8557b66a1..e35ab74a0f80 100644
> > --- a/Documentation/process/changes.rst
> > +++ b/Documentation/process/changes.rst
> > @@ -29,7 +29,7 @@ you probably needn't concern yourself with pcmciautils.
> >   ====================== ===============  ========================================
> >           Program        Minimal version       Command to check the version
> >   ====================== ===============  ========================================
> > -GNU C                  4.9              gcc --version
> > +GNU C                  5.1              gcc --version
> >   Clang/LLVM (optional)  10.0.1           clang --version
> >   GNU make               3.81             make --version
> >   binutils               2.23             ld -v
> > diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
> > index 319f92104f56..4edc708baa63 100755
> > --- a/scripts/min-tool-version.sh
> > +++ b/scripts/min-tool-version.sh
> > @@ -17,13 +17,7 @@ binutils)
> >       echo 2.23.0
> >       ;;
> >   gcc)
> > -     # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293
> > -     # https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk
> > -     if [ "$SRCARCH" = arm64 ]; then
> > -             echo 5.1.0
> > -     else
> > -             echo 4.9.0
> > -     fi
> > +     echo 5.1.0
> >       ;;
> >   icc)
> >       # temporary
> >



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 01/10] Documentation: raise minimum supported version of GCC to 5.1

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:38PM -0700, Nick Desaulniers wrote:
> commit fad7cd3310db ("nbd: add the check to prevent overflow in
> __nbd_ioctl()")
> 
> raised an issue from the fallback helpers added in
> 
> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
> 
> Specifically, the helpers for checking whether the results of a
> multiplication overflowed (__unsigned_mul_overflow,
> __signed_add_overflow) use the division operator when
> !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
> operands on 32b hosts.
> 
> Also, because the macro is type agnostic, it is very difficult to write
> a similarly type generic macro that dispatches to one of:
> * div64_s64
> * div64_u64
> * div_s64
> * div_u64
> 
> Raising the minimum supported versions allows us to remove all of the
> fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
> dispatching the compiler builtins.
> 
> arm64 has already raised the minimum supported GCC version to 5.1, do
> this for all targets now. See the link below for the previous
> discussion.
> 
> Link: https://lore.kernel.org/all/20210909182525.372ee687@canb.auug.org.au/
> Link: https://lore.kernel.org/lkml/CAK7LNASs6dvU6D3jL2GG3jW58fXfaj6VNOe55NJnTB8UPuk2pA@mail.gmail.com/
> Link: https://github.com/ClangBuiltLinux/linux/issues/1438
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH 02/10] compiler.h: drop fallback overflow checkers

[Nick Desaulniers]

Once upgrading the minimum supported version of GCC to 5.1, we can drop
the fallback code for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW.

Effectively a revert of
commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and
add fallback code")

Link: https://github.com/ClangBuiltLinux/linux/issues/1438#issuecomment-916745801
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 include/linux/compiler-clang.h     |  13 ---
 include/linux/compiler-gcc.h       |   4 -
 include/linux/overflow.h           | 138 +---------------------------
 tools/include/linux/compiler-gcc.h |   4 -
 tools/include/linux/overflow.h     | 140 +----------------------------
 5 files changed, 6 insertions(+), 293 deletions(-)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 49b0ac8b6fd3..3c4de9b6c6e3 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -62,19 +62,6 @@
 #define __no_sanitize_coverage
 #endif
 
-/*
- * Not all versions of clang implement the type-generic versions
- * of the builtin overflow checkers. Fortunately, clang implements
- * __has_builtin allowing us to avoid awkward version
- * checks. Unfortunately, we don't know which version of gcc clang
- * pretends to be, so the macro may or may not be defined.
- */
-#if __has_builtin(__builtin_mul_overflow) && \
-    __has_builtin(__builtin_add_overflow) && \
-    __has_builtin(__builtin_sub_overflow)
-#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
-#endif
-
 #if __has_feature(shadow_call_stack)
 # define __noscs	__attribute__((__no_sanitize__("shadow-call-stack")))
 #endif
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index cb9217fc60af..3f7f6fa0e051 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -128,10 +128,6 @@
 #define __no_sanitize_coverage
 #endif
 
-#if GCC_VERSION >= 50100
-#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
-#endif
-
 /*
  * Turn individual warnings and errors on and off locally, depending
  * on version.
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index 0f12345c21fb..4669632bd72b 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -6,12 +6,9 @@
 #include <linux/limits.h>
 
 /*
- * In the fallback code below, we need to compute the minimum and
- * maximum values representable in a given type. These macros may also
- * be useful elsewhere, so we provide them outside the
- * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
- *
- * It would seem more obvious to do something like
+ * We need to compute the minimum and maximum values representable in a given
+ * type. These macros may also be useful elsewhere. It would seem more obvious
+ * to do something like:
  *
  * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
  * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
@@ -54,7 +51,6 @@ static inline bool __must_check __must_check_overflow(bool overflow)
 	return unlikely(overflow);
 }
 
-#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
 /*
  * For simplicity and code hygiene, the fallback code below insists on
  * a, b and *d having the same type (similar to the min() and max()
@@ -90,134 +86,6 @@ static inline bool __must_check __must_check_overflow(bool overflow)
 	__builtin_mul_overflow(__a, __b, __d);	\
 }))
 
-#else
-
-
-/* Checking for unsigned overflow is relatively easy without causing UB. */
-#define __unsigned_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a + __b;			\
-	*__d < __a;				\
-})
-#define __unsigned_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a - __b;			\
-	__a < __b;				\
-})
-/*
- * If one of a or b is a compile-time constant, this avoids a division.
- */
-#define __unsigned_mul_overflow(a, b, d) ({		\
-	typeof(a) __a = (a);				\
-	typeof(b) __b = (b);				\
-	typeof(d) __d = (d);				\
-	(void) (&__a == &__b);				\
-	(void) (&__a == __d);				\
-	*__d = __a * __b;				\
-	__builtin_constant_p(__b) ?			\
-	  __b > 0 && __a > type_max(typeof(__a)) / __b : \
-	  __a > 0 && __b > type_max(typeof(__b)) / __a;	 \
-})
-
-/*
- * For signed types, detecting overflow is much harder, especially if
- * we want to avoid UB. But the interface of these macros is such that
- * we must provide a result in *d, and in fact we must produce the
- * result promised by gcc's builtins, which is simply the possibly
- * wrapped-around value. Fortunately, we can just formally do the
- * operations in the widest relevant unsigned type (u64) and then
- * truncate the result - gcc is smart enough to generate the same code
- * with and without the (u64) casts.
- */
-
-/*
- * Adding two signed integers can overflow only if they have the same
- * sign, and overflow has happened iff the result has the opposite
- * sign.
- */
-#define __signed_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a + (u64)__b;		\
-	(((~(__a ^ __b)) & (*__d ^ __a))	\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Subtraction is similar, except that overflow can now happen only
- * when the signs are opposite. In this case, overflow has happened if
- * the result has the opposite sign of a.
- */
-#define __signed_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a - (u64)__b;		\
-	((((__a ^ __b)) & (*__d ^ __a))		\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Signed multiplication is rather hard. gcc always follows C99, so
- * division is truncated towards 0. This means that we can write the
- * overflow check like this:
- *
- * (a > 0 && (b > MAX/a || b < MIN/a)) ||
- * (a < -1 && (b > MIN/a || b < MAX/a) ||
- * (a == -1 && b == MIN)
- *
- * The redundant casts of -1 are to silence an annoying -Wtype-limits
- * (included in -Wextra) warning: When the type is u8 or u16, the
- * __b_c_e in check_mul_overflow obviously selects
- * __unsigned_mul_overflow, but unfortunately gcc still parses this
- * code and warns about the limited range of __b.
- */
-
-#define __signed_mul_overflow(a, b, d) ({				\
-	typeof(a) __a = (a);						\
-	typeof(b) __b = (b);						\
-	typeof(d) __d = (d);						\
-	typeof(a) __tmax = type_max(typeof(a));				\
-	typeof(a) __tmin = type_min(typeof(a));				\
-	(void) (&__a == &__b);						\
-	(void) (&__a == __d);						\
-	*__d = (u64)__a * (u64)__b;					\
-	(__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||	\
-	(__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
-	(__b == (typeof(__b))-1 && __a == __tmin);			\
-})
-
-
-#define check_add_overflow(a, b, d)	__must_check_overflow(		\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_add_overflow(a, b, d),			\
-			__unsigned_add_overflow(a, b, d)))
-
-#define check_sub_overflow(a, b, d)	__must_check_overflow(		\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_sub_overflow(a, b, d),			\
-			__unsigned_sub_overflow(a, b, d)))
-
-#define check_mul_overflow(a, b, d)	__must_check_overflow(		\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_mul_overflow(a, b, d),			\
-			__unsigned_mul_overflow(a, b, d)))
-
-#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
-
 /** check_shl_overflow() - Calculate a left-shifted value and check overflow
  *
  * @a: Value to be shifted
diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
index 95c072b70d0e..a590a1dfafd9 100644
--- a/tools/include/linux/compiler-gcc.h
+++ b/tools/include/linux/compiler-gcc.h
@@ -38,7 +38,3 @@
 #endif
 #define __printf(a, b)	__attribute__((format(printf, a, b)))
 #define __scanf(a, b)	__attribute__((format(scanf, a, b)))
-
-#if GCC_VERSION >= 50100
-#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
-#endif
diff --git a/tools/include/linux/overflow.h b/tools/include/linux/overflow.h
index 8712ff70995f..dcb0c1bf6866 100644
--- a/tools/include/linux/overflow.h
+++ b/tools/include/linux/overflow.h
@@ -5,12 +5,9 @@
 #include <linux/compiler.h>
 
 /*
- * In the fallback code below, we need to compute the minimum and
- * maximum values representable in a given type. These macros may also
- * be useful elsewhere, so we provide them outside the
- * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
- *
- * It would seem more obvious to do something like
+ * We need to compute the minimum and maximum values representable in a given
+ * type. These macros may also be useful elsewhere. It would seem more obvious
+ * to do something like:
  *
  * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
  * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
@@ -36,8 +33,6 @@
 #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
 #define type_min(T) ((T)((T)-type_max(T)-(T)1))
 
-
-#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
 /*
  * For simplicity and code hygiene, the fallback code below insists on
  * a, b and *d having the same type (similar to the min() and max()
@@ -73,135 +68,6 @@
 	__builtin_mul_overflow(__a, __b, __d);	\
 })
 
-#else
-
-
-/* Checking for unsigned overflow is relatively easy without causing UB. */
-#define __unsigned_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a + __b;			\
-	*__d < __a;				\
-})
-#define __unsigned_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = __a - __b;			\
-	__a < __b;				\
-})
-/*
- * If one of a or b is a compile-time constant, this avoids a division.
- */
-#define __unsigned_mul_overflow(a, b, d) ({		\
-	typeof(a) __a = (a);				\
-	typeof(b) __b = (b);				\
-	typeof(d) __d = (d);				\
-	(void) (&__a == &__b);				\
-	(void) (&__a == __d);				\
-	*__d = __a * __b;				\
-	__builtin_constant_p(__b) ?			\
-	  __b > 0 && __a > type_max(typeof(__a)) / __b : \
-	  __a > 0 && __b > type_max(typeof(__b)) / __a;	 \
-})
-
-/*
- * For signed types, detecting overflow is much harder, especially if
- * we want to avoid UB. But the interface of these macros is such that
- * we must provide a result in *d, and in fact we must produce the
- * result promised by gcc's builtins, which is simply the possibly
- * wrapped-around value. Fortunately, we can just formally do the
- * operations in the widest relevant unsigned type (u64) and then
- * truncate the result - gcc is smart enough to generate the same code
- * with and without the (u64) casts.
- */
-
-/*
- * Adding two signed integers can overflow only if they have the same
- * sign, and overflow has happened iff the result has the opposite
- * sign.
- */
-#define __signed_add_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a + (u64)__b;		\
-	(((~(__a ^ __b)) & (*__d ^ __a))	\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Subtraction is similar, except that overflow can now happen only
- * when the signs are opposite. In this case, overflow has happened if
- * the result has the opposite sign of a.
- */
-#define __signed_sub_overflow(a, b, d) ({	\
-	typeof(a) __a = (a);			\
-	typeof(b) __b = (b);			\
-	typeof(d) __d = (d);			\
-	(void) (&__a == &__b);			\
-	(void) (&__a == __d);			\
-	*__d = (u64)__a - (u64)__b;		\
-	((((__a ^ __b)) & (*__d ^ __a))		\
-		& type_min(typeof(__a))) != 0;	\
-})
-
-/*
- * Signed multiplication is rather hard. gcc always follows C99, so
- * division is truncated towards 0. This means that we can write the
- * overflow check like this:
- *
- * (a > 0 && (b > MAX/a || b < MIN/a)) ||
- * (a < -1 && (b > MIN/a || b < MAX/a) ||
- * (a == -1 && b == MIN)
- *
- * The redundant casts of -1 are to silence an annoying -Wtype-limits
- * (included in -Wextra) warning: When the type is u8 or u16, the
- * __b_c_e in check_mul_overflow obviously selects
- * __unsigned_mul_overflow, but unfortunately gcc still parses this
- * code and warns about the limited range of __b.
- */
-
-#define __signed_mul_overflow(a, b, d) ({				\
-	typeof(a) __a = (a);						\
-	typeof(b) __b = (b);						\
-	typeof(d) __d = (d);						\
-	typeof(a) __tmax = type_max(typeof(a));				\
-	typeof(a) __tmin = type_min(typeof(a));				\
-	(void) (&__a == &__b);						\
-	(void) (&__a == __d);						\
-	*__d = (u64)__a * (u64)__b;					\
-	(__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||	\
-	(__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
-	(__b == (typeof(__b))-1 && __a == __tmin);			\
-})
-
-
-#define check_add_overflow(a, b, d)					\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_add_overflow(a, b, d),			\
-			__unsigned_add_overflow(a, b, d))
-
-#define check_sub_overflow(a, b, d)					\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_sub_overflow(a, b, d),			\
-			__unsigned_sub_overflow(a, b, d))
-
-#define check_mul_overflow(a, b, d)					\
-	__builtin_choose_expr(is_signed_type(typeof(a)),		\
-			__signed_mul_overflow(a, b, d),			\
-			__unsigned_mul_overflow(a, b, d))
-
-
-#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
-
 /**
  * array_size() - Calculate size of 2-dimensional array.
  *
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 02/10] compiler.h: drop fallback overflow checkers

[Nathan Chancellor]

On Fri, Sep 10, 2021 at 04:40:39PM -0700, Nick Desaulniers wrote:
> Once upgrading the minimum supported version of GCC to 5.1, we can drop
> the fallback code for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW.
> 
> Effectively a revert of
> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and
> add fallback code")
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1438#issuecomment-916745801
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Some comments inline, please carry the tag forward?

> ---
>  include/linux/compiler-clang.h     |  13 ---
>  include/linux/compiler-gcc.h       |   4 -
>  include/linux/overflow.h           | 138 +---------------------------
>  tools/include/linux/compiler-gcc.h |   4 -
>  tools/include/linux/overflow.h     | 140 +----------------------------
>  5 files changed, 6 insertions(+), 293 deletions(-)
> 
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 49b0ac8b6fd3..3c4de9b6c6e3 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -62,19 +62,6 @@
>  #define __no_sanitize_coverage
>  #endif
>  
> -/*
> - * Not all versions of clang implement the type-generic versions
> - * of the builtin overflow checkers. Fortunately, clang implements
> - * __has_builtin allowing us to avoid awkward version
> - * checks. Unfortunately, we don't know which version of gcc clang
> - * pretends to be, so the macro may or may not be defined.
> - */
> -#if __has_builtin(__builtin_mul_overflow) && \
> -    __has_builtin(__builtin_add_overflow) && \
> -    __has_builtin(__builtin_sub_overflow)
> -#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> -#endif
> -

I am guessing the comment has been stale for a while since we did not
have a minimum version of LLVM when commit f0907827a8a9 ("compiler.h:
enable builtin overflow checkers and add fallback code")?

$ git grep -P "__builtin_(add|mul|sub)_overflow" llvmorg-10.0.1 clang/include/clang/Basic/Builtins.def
llvmorg-10.0.1:clang/include/clang/Basic/Builtins.def:BUILTIN(__builtin_add_overflow, "b.", "nt")
llvmorg-10.0.1:clang/include/clang/Basic/Builtins.def:BUILTIN(__builtin_sub_overflow, "b.", "nt")
llvmorg-10.0.1:clang/include/clang/Basic/Builtins.def:BUILTIN(__builtin_mul_overflow, "b.", "nt")

>  #if __has_feature(shadow_call_stack)
>  # define __noscs	__attribute__((__no_sanitize__("shadow-call-stack")))
>  #endif
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index cb9217fc60af..3f7f6fa0e051 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -128,10 +128,6 @@
>  #define __no_sanitize_coverage
>  #endif
>  
> -#if GCC_VERSION >= 50100
> -#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> -#endif
> -
>  /*
>   * Turn individual warnings and errors on and off locally, depending
>   * on version.
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index 0f12345c21fb..4669632bd72b 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -6,12 +6,9 @@
>  #include <linux/limits.h>
>  
>  /*
> - * In the fallback code below, we need to compute the minimum and
> - * maximum values representable in a given type. These macros may also
> - * be useful elsewhere, so we provide them outside the
> - * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
> - *
> - * It would seem more obvious to do something like
> + * We need to compute the minimum and maximum values representable in a given
> + * type. These macros may also be useful elsewhere. It would seem more obvious
> + * to do something like:
>   *
>   * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
>   * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)

The signed and type macros right below this comment can be removed as
they were only used in the !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW case.

Also applies to the tools/ version.

> @@ -54,7 +51,6 @@ static inline bool __must_check __must_check_overflow(bool overflow)
>  	return unlikely(overflow);
>  }
>  
> -#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
>  /*
>   * For simplicity and code hygiene, the fallback code below insists on
>   * a, b and *d having the same type (similar to the min() and max()
> @@ -90,134 +86,6 @@ static inline bool __must_check __must_check_overflow(bool overflow)
>  	__builtin_mul_overflow(__a, __b, __d);	\
>  }))
>  
> -#else
> -
> -
> -/* Checking for unsigned overflow is relatively easy without causing UB. */
> -#define __unsigned_add_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = __a + __b;			\
> -	*__d < __a;				\
> -})
> -#define __unsigned_sub_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = __a - __b;			\
> -	__a < __b;				\
> -})
> -/*
> - * If one of a or b is a compile-time constant, this avoids a division.
> - */
> -#define __unsigned_mul_overflow(a, b, d) ({		\
> -	typeof(a) __a = (a);				\
> -	typeof(b) __b = (b);				\
> -	typeof(d) __d = (d);				\
> -	(void) (&__a == &__b);				\
> -	(void) (&__a == __d);				\
> -	*__d = __a * __b;				\
> -	__builtin_constant_p(__b) ?			\
> -	  __b > 0 && __a > type_max(typeof(__a)) / __b : \
> -	  __a > 0 && __b > type_max(typeof(__b)) / __a;	 \
> -})
> -
> -/*
> - * For signed types, detecting overflow is much harder, especially if
> - * we want to avoid UB. But the interface of these macros is such that
> - * we must provide a result in *d, and in fact we must produce the
> - * result promised by gcc's builtins, which is simply the possibly
> - * wrapped-around value. Fortunately, we can just formally do the
> - * operations in the widest relevant unsigned type (u64) and then
> - * truncate the result - gcc is smart enough to generate the same code
> - * with and without the (u64) casts.
> - */
> -
> -/*
> - * Adding two signed integers can overflow only if they have the same
> - * sign, and overflow has happened iff the result has the opposite
> - * sign.
> - */
> -#define __signed_add_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = (u64)__a + (u64)__b;		\
> -	(((~(__a ^ __b)) & (*__d ^ __a))	\
> -		& type_min(typeof(__a))) != 0;	\
> -})
> -
> -/*
> - * Subtraction is similar, except that overflow can now happen only
> - * when the signs are opposite. In this case, overflow has happened if
> - * the result has the opposite sign of a.
> - */
> -#define __signed_sub_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = (u64)__a - (u64)__b;		\
> -	((((__a ^ __b)) & (*__d ^ __a))		\
> -		& type_min(typeof(__a))) != 0;	\
> -})
> -
> -/*
> - * Signed multiplication is rather hard. gcc always follows C99, so
> - * division is truncated towards 0. This means that we can write the
> - * overflow check like this:
> - *
> - * (a > 0 && (b > MAX/a || b < MIN/a)) ||
> - * (a < -1 && (b > MIN/a || b < MAX/a) ||
> - * (a == -1 && b == MIN)
> - *
> - * The redundant casts of -1 are to silence an annoying -Wtype-limits
> - * (included in -Wextra) warning: When the type is u8 or u16, the
> - * __b_c_e in check_mul_overflow obviously selects
> - * __unsigned_mul_overflow, but unfortunately gcc still parses this
> - * code and warns about the limited range of __b.
> - */
> -
> -#define __signed_mul_overflow(a, b, d) ({				\
> -	typeof(a) __a = (a);						\
> -	typeof(b) __b = (b);						\
> -	typeof(d) __d = (d);						\
> -	typeof(a) __tmax = type_max(typeof(a));				\
> -	typeof(a) __tmin = type_min(typeof(a));				\
> -	(void) (&__a == &__b);						\
> -	(void) (&__a == __d);						\
> -	*__d = (u64)__a * (u64)__b;					\
> -	(__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||	\
> -	(__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
> -	(__b == (typeof(__b))-1 && __a == __tmin);			\
> -})
> -
> -
> -#define check_add_overflow(a, b, d)	__must_check_overflow(		\
> -	__builtin_choose_expr(is_signed_type(typeof(a)),		\
> -			__signed_add_overflow(a, b, d),			\
> -			__unsigned_add_overflow(a, b, d)))
> -
> -#define check_sub_overflow(a, b, d)	__must_check_overflow(		\
> -	__builtin_choose_expr(is_signed_type(typeof(a)),		\
> -			__signed_sub_overflow(a, b, d),			\
> -			__unsigned_sub_overflow(a, b, d)))
> -
> -#define check_mul_overflow(a, b, d)	__must_check_overflow(		\
> -	__builtin_choose_expr(is_signed_type(typeof(a)),		\
> -			__signed_mul_overflow(a, b, d),			\
> -			__unsigned_mul_overflow(a, b, d)))
> -
> -#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
> -
>  /** check_shl_overflow() - Calculate a left-shifted value and check overflow
>   *
>   * @a: Value to be shifted
> diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
> index 95c072b70d0e..a590a1dfafd9 100644
> --- a/tools/include/linux/compiler-gcc.h
> +++ b/tools/include/linux/compiler-gcc.h
> @@ -38,7 +38,3 @@
>  #endif
>  #define __printf(a, b)	__attribute__((format(printf, a, b)))
>  #define __scanf(a, b)	__attribute__((format(scanf, a, b)))
> -
> -#if GCC_VERSION >= 50100
> -#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> -#endif
> diff --git a/tools/include/linux/overflow.h b/tools/include/linux/overflow.h
> index 8712ff70995f..dcb0c1bf6866 100644
> --- a/tools/include/linux/overflow.h
> +++ b/tools/include/linux/overflow.h
> @@ -5,12 +5,9 @@
>  #include <linux/compiler.h>
>  
>  /*
> - * In the fallback code below, we need to compute the minimum and
> - * maximum values representable in a given type. These macros may also
> - * be useful elsewhere, so we provide them outside the
> - * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
> - *
> - * It would seem more obvious to do something like
> + * We need to compute the minimum and maximum values representable in a given
> + * type. These macros may also be useful elsewhere. It would seem more obvious
> + * to do something like:
>   *
>   * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
>   * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
> @@ -36,8 +33,6 @@
>  #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
>  #define type_min(T) ((T)((T)-type_max(T)-(T)1))
>  
> -
> -#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
>  /*
>   * For simplicity and code hygiene, the fallback code below insists on
>   * a, b and *d having the same type (similar to the min() and max()
> @@ -73,135 +68,6 @@
>  	__builtin_mul_overflow(__a, __b, __d);	\
>  })
>  
> -#else
> -
> -
> -/* Checking for unsigned overflow is relatively easy without causing UB. */
> -#define __unsigned_add_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = __a + __b;			\
> -	*__d < __a;				\
> -})
> -#define __unsigned_sub_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = __a - __b;			\
> -	__a < __b;				\
> -})
> -/*
> - * If one of a or b is a compile-time constant, this avoids a division.
> - */
> -#define __unsigned_mul_overflow(a, b, d) ({		\
> -	typeof(a) __a = (a);				\
> -	typeof(b) __b = (b);				\
> -	typeof(d) __d = (d);				\
> -	(void) (&__a == &__b);				\
> -	(void) (&__a == __d);				\
> -	*__d = __a * __b;				\
> -	__builtin_constant_p(__b) ?			\
> -	  __b > 0 && __a > type_max(typeof(__a)) / __b : \
> -	  __a > 0 && __b > type_max(typeof(__b)) / __a;	 \
> -})
> -
> -/*
> - * For signed types, detecting overflow is much harder, especially if
> - * we want to avoid UB. But the interface of these macros is such that
> - * we must provide a result in *d, and in fact we must produce the
> - * result promised by gcc's builtins, which is simply the possibly
> - * wrapped-around value. Fortunately, we can just formally do the
> - * operations in the widest relevant unsigned type (u64) and then
> - * truncate the result - gcc is smart enough to generate the same code
> - * with and without the (u64) casts.
> - */
> -
> -/*
> - * Adding two signed integers can overflow only if they have the same
> - * sign, and overflow has happened iff the result has the opposite
> - * sign.
> - */
> -#define __signed_add_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = (u64)__a + (u64)__b;		\
> -	(((~(__a ^ __b)) & (*__d ^ __a))	\
> -		& type_min(typeof(__a))) != 0;	\
> -})
> -
> -/*
> - * Subtraction is similar, except that overflow can now happen only
> - * when the signs are opposite. In this case, overflow has happened if
> - * the result has the opposite sign of a.
> - */
> -#define __signed_sub_overflow(a, b, d) ({	\
> -	typeof(a) __a = (a);			\
> -	typeof(b) __b = (b);			\
> -	typeof(d) __d = (d);			\
> -	(void) (&__a == &__b);			\
> -	(void) (&__a == __d);			\
> -	*__d = (u64)__a - (u64)__b;		\
> -	((((__a ^ __b)) & (*__d ^ __a))		\
> -		& type_min(typeof(__a))) != 0;	\
> -})
> -
> -/*
> - * Signed multiplication is rather hard. gcc always follows C99, so
> - * division is truncated towards 0. This means that we can write the
> - * overflow check like this:
> - *
> - * (a > 0 && (b > MAX/a || b < MIN/a)) ||
> - * (a < -1 && (b > MIN/a || b < MAX/a) ||
> - * (a == -1 && b == MIN)
> - *
> - * The redundant casts of -1 are to silence an annoying -Wtype-limits
> - * (included in -Wextra) warning: When the type is u8 or u16, the
> - * __b_c_e in check_mul_overflow obviously selects
> - * __unsigned_mul_overflow, but unfortunately gcc still parses this
> - * code and warns about the limited range of __b.
> - */
> -
> -#define __signed_mul_overflow(a, b, d) ({				\
> -	typeof(a) __a = (a);						\
> -	typeof(b) __b = (b);						\
> -	typeof(d) __d = (d);						\
> -	typeof(a) __tmax = type_max(typeof(a));				\
> -	typeof(a) __tmin = type_min(typeof(a));				\
> -	(void) (&__a == &__b);						\
> -	(void) (&__a == __d);						\
> -	*__d = (u64)__a * (u64)__b;					\
> -	(__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||	\
> -	(__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
> -	(__b == (typeof(__b))-1 && __a == __tmin);			\
> -})
> -
> -
> -#define check_add_overflow(a, b, d)					\
> -	__builtin_choose_expr(is_signed_type(typeof(a)),		\
> -			__signed_add_overflow(a, b, d),			\
> -			__unsigned_add_overflow(a, b, d))
> -
> -#define check_sub_overflow(a, b, d)					\
> -	__builtin_choose_expr(is_signed_type(typeof(a)),		\
> -			__signed_sub_overflow(a, b, d),			\
> -			__unsigned_sub_overflow(a, b, d))
> -
> -#define check_mul_overflow(a, b, d)					\
> -	__builtin_choose_expr(is_signed_type(typeof(a)),		\
> -			__signed_mul_overflow(a, b, d),			\
> -			__unsigned_mul_overflow(a, b, d))
> -
> -
> -#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
> -
>  /**
>   * array_size() - Calculate size of 2-dimensional array.
>   *
> -- 
> 2.33.0.309.g3052b89438-goog
> 
> 

Re: [PATCH 02/10] compiler.h: drop fallback overflow checkers

[Nick Desaulniers]

On Fri, Sep 10, 2021 at 5:04 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Fri, Sep 10, 2021 at 04:40:39PM -0700, Nick Desaulniers wrote:
> > diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> > index 0f12345c21fb..4669632bd72b 100644
> > --- a/include/linux/overflow.h
> > +++ b/include/linux/overflow.h
> > @@ -6,12 +6,9 @@
> >  #include <linux/limits.h>
> >
> >  /*
> > - * In the fallback code below, we need to compute the minimum and
> > - * maximum values representable in a given type. These macros may also
> > - * be useful elsewhere, so we provide them outside the
> > - * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
> > - *
> > - * It would seem more obvious to do something like
> > + * We need to compute the minimum and maximum values representable in a given
> > + * type. These macros may also be useful elsewhere. It would seem more obvious
> > + * to do something like:
> >   *
> >   * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
> >   * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
>
> The signed and type macros right below this comment can be removed as
> they were only used in the !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW case.

Did you check for users outside of this header?

I see:
type_min ->
lib/test_scanf.c:189
include/rdma/uverbs_ioctl.h:951
include/rdma/uverbs_ioctl.h:973

type_max ->
lib/test_scanf.c:189
lib/test_scanf.c:190
include/rdma/uverbs_ioctl.h:952
include/rdma/uverbs_ioctl.h:962
include/rdma/uverbs_ioctl.h:974
include/rdma/uverbs_ioctl.h:985

is_signed_type has many many users throughout the kernel.

Or were you referring to other defines?

>
> Also applies to the tools/ version.

The version in tools/ should probably be "refreshed" ie. copy+pasted
over.  Why there is a separate copy under tools/...
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 02/10] compiler.h: drop fallback overflow checkers

[Nathan Chancellor]

On 9/14/2021 8:33 AM, Nick Desaulniers wrote:
> On Fri, Sep 10, 2021 at 5:04 PM Nathan Chancellor <nathan@kernel.org> wrote:
>>
>> On Fri, Sep 10, 2021 at 04:40:39PM -0700, Nick Desaulniers wrote:
>>> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
>>> index 0f12345c21fb..4669632bd72b 100644
>>> --- a/include/linux/overflow.h
>>> +++ b/include/linux/overflow.h
>>> @@ -6,12 +6,9 @@
>>>   #include <linux/limits.h>
>>>
>>>   /*
>>> - * In the fallback code below, we need to compute the minimum and
>>> - * maximum values representable in a given type. These macros may also
>>> - * be useful elsewhere, so we provide them outside the
>>> - * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
>>> - *
>>> - * It would seem more obvious to do something like
>>> + * We need to compute the minimum and maximum values representable in a given
>>> + * type. These macros may also be useful elsewhere. It would seem more obvious
>>> + * to do something like:
>>>    *
>>>    * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
>>>    * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
>>
>> The signed and type macros right below this comment can be removed as
>> they were only used in the !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW case.
> 
> Did you check for users outside of this header?
> 
> I see:
> type_min ->
> lib/test_scanf.c:189
> include/rdma/uverbs_ioctl.h:951
> include/rdma/uverbs_ioctl.h:973
> 
> type_max ->
> lib/test_scanf.c:189
> lib/test_scanf.c:190
> include/rdma/uverbs_ioctl.h:952
> include/rdma/uverbs_ioctl.h:962
> include/rdma/uverbs_ioctl.h:974
> include/rdma/uverbs_ioctl.h:985
> 
> is_signed_type has many many users throughout the kernel.
> 
> Or were you referring to other defines?

Ah, I did not even think to look outside this file, I figured they were 
intended to only be used here :/ good catch.

>>
>> Also applies to the tools/ version.
> 
> The version in tools/ should probably be "refreshed" ie. copy+pasted
> over.  Why there is a separate copy under tools/...
> 

Yes, they probably should, as I noted in commit d0ee23f9d78b ("tools: 
compiler-gcc.h: Guard error attribute use with __has_attribute"). At the 
same time, I don't really want to do it :)

Cheers,
Nathan

Re: [PATCH 02/10] compiler.h: drop fallback overflow checkers

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:39PM -0700, Nick Desaulniers wrote:
> Once upgrading the minimum supported version of GCC to 5.1, we can drop
> the fallback code for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW.
> 
> Effectively a revert of
> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and
> add fallback code")
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1438#issuecomment-916745801
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

With Nathan's comments addressed:

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook

[PATCH 03/10] mm/ksm: remove old GCC 4.9+ check

[Nick Desaulniers]

The minimum supported version of GCC has been raised to GCC 5.1.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 mm/ksm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 025338128cd9..a5716fdec1aa 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -651,10 +651,8 @@ static void remove_node_from_stable_tree(struct stable_node *stable_node)
 	 * from &migrate_nodes. This will verify that future list.h changes
 	 * don't break STABLE_NODE_DUP_HEAD. Only recent gcc can handle it.
 	 */
-#if defined(GCC_VERSION) && GCC_VERSION >= 40903
 	BUILD_BUG_ON(STABLE_NODE_DUP_HEAD <= &migrate_nodes);
 	BUILD_BUG_ON(STABLE_NODE_DUP_HEAD >= &migrate_nodes + 1);
-#endif
 
 	if (stable_node->head == &migrate_nodes)
 		list_del(&stable_node->list);
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 03/10] mm/ksm: remove old GCC 4.9+ check

[Nathan Chancellor]

On Fri, Sep 10, 2021 at 04:40:40PM -0700, Nick Desaulniers wrote:
> The minimum supported version of GCC has been raised to GCC 5.1.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

This is technically new for clang but arm64 allmodconfig does not
complain.

> ---
>  mm/ksm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 025338128cd9..a5716fdec1aa 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -651,10 +651,8 @@ static void remove_node_from_stable_tree(struct stable_node *stable_node)
>  	 * from &migrate_nodes. This will verify that future list.h changes
>  	 * don't break STABLE_NODE_DUP_HEAD. Only recent gcc can handle it.

Probably worth removing the recent GCC comment.

>  	 */
> -#if defined(GCC_VERSION) && GCC_VERSION >= 40903
>  	BUILD_BUG_ON(STABLE_NODE_DUP_HEAD <= &migrate_nodes);
>  	BUILD_BUG_ON(STABLE_NODE_DUP_HEAD >= &migrate_nodes + 1);
> -#endif
>  
>  	if (stable_node->head == &migrate_nodes)
>  		list_del(&stable_node->list);
> -- 
> 2.33.0.309.g3052b89438-goog
> 
> 

Re: [PATCH 03/10] mm/ksm: remove old GCC 4.9+ check

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:40PM -0700, Nick Desaulniers wrote:
> The minimum supported version of GCC has been raised to GCC 5.1.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH 04/10] Kconfig.debug: drop GCC 5+ version check for DWARF5

[Nick Desaulniers]

Now that the minimum supported version of GCC is 5.1, we no longer need
this Kconfig version check for CONFIG_DEBUG_INFO_DWARF5.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 lib/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ed4a31e34098..d566f601780f 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -295,7 +295,7 @@ config DEBUG_INFO_DWARF4
 
 config DEBUG_INFO_DWARF5
 	bool "Generate DWARF Version 5 debuginfo"
-	depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)))
+	depends on !CC_IS_CLANG || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)))
 	depends on !DEBUG_INFO_BTF
 	help
 	  Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ (gcc
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 04/10] Kconfig.debug: drop GCC 5+ version check for DWARF5

[Nathan Chancellor]

On Fri, Sep 10, 2021 at 04:40:41PM -0700, Nick Desaulniers wrote:
> Now that the minimum supported version of GCC is 5.1, we no longer need
> this Kconfig version check for CONFIG_DEBUG_INFO_DWARF5.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  lib/Kconfig.debug | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index ed4a31e34098..d566f601780f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -295,7 +295,7 @@ config DEBUG_INFO_DWARF4
>  
>  config DEBUG_INFO_DWARF5
>  	bool "Generate DWARF Version 5 debuginfo"
> -	depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)))
> +	depends on !CC_IS_CLANG || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)))
>  	depends on !DEBUG_INFO_BTF
>  	help
>  	  Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ (gcc
> -- 
> 2.33.0.309.g3052b89438-goog
> 
> 

Re: [PATCH 04/10] Kconfig.debug: drop GCC 5+ version check for DWARF5

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:41PM -0700, Nick Desaulniers wrote:
> Now that the minimum supported version of GCC is 5.1, we no longer need
> this Kconfig version check for CONFIG_DEBUG_INFO_DWARF5.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH 05/10] riscv: remove Kconfig check for GCC version for ARCH_RV64I

[Nick Desaulniers]

The minimum supported version of GCC is now 5.1. The check wasn't
correct as written anyways since GCC_VERSION is 0 when CC=clang.

Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: linux-riscv@lists.infradead.org
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/riscv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index c79955655fa4..5fc1d0cc82e1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -235,7 +235,7 @@ config ARCH_RV32I
 config ARCH_RV64I
 	bool "RV64I"
 	select 64BIT
-	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000
+	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
 	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 05/10] riscv: remove Kconfig check for GCC version for ARCH_RV64I

[Nathan Chancellor]

On Fri, Sep 10, 2021 at 04:40:42PM -0700, Nick Desaulniers wrote:
> The minimum supported version of GCC is now 5.1. The check wasn't
> correct as written anyways since GCC_VERSION is 0 when CC=clang.
> 
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: linux-riscv@lists.infradead.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Indeed, I meant to clean this up a while ago but forgot :/

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/riscv/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index c79955655fa4..5fc1d0cc82e1 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -235,7 +235,7 @@ config ARCH_RV32I
>  config ARCH_RV64I
>  	bool "RV64I"
>  	select 64BIT
> -	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && GCC_VERSION >= 50000
> +	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
>  	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
>  	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>  	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> -- 
> 2.33.0.309.g3052b89438-goog
> 
> 

Re: [PATCH 05/10] riscv: remove Kconfig check for GCC version for ARCH_RV64I

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:42PM -0700, Nick Desaulniers wrote:
> The minimum supported version of GCC is now 5.1. The check wasn't
> correct as written anyways since GCC_VERSION is 0 when CC=clang.
> 
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: linux-riscv@lists.infradead.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Yeah, good catch for Clang too.

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH 05/10] riscv: remove Kconfig check for GCC version for ARCH_RV64I

[Palmer Dabbelt]

On Mon, 13 Sep 2021 09:23:45 PDT (-0700), keescook@chromium.org wrote:
> On Fri, Sep 10, 2021 at 04:40:42PM -0700, Nick Desaulniers wrote:
>> The minimum supported version of GCC is now 5.1. The check wasn't
>> correct as written anyways since GCC_VERSION is 0 when CC=clang.
>>
>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> Cc: Albert Ou <aou@eecs.berkeley.edu>
>> Cc: linux-riscv@lists.infradead.org
>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Yeah, good catch for Clang too.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

Only this patch ended up in my inbox.  Where you guys planning on 
keeping this whole series together, or did you want me to pick this for 
the RISC-V tree?

Re: [PATCH 05/10] riscv: remove Kconfig check for GCC version for ARCH_RV64I

[Kees Cook]

On Mon, Oct 04, 2021 at 05:40:42PM -0700, Palmer Dabbelt wrote:
> On Mon, 13 Sep 2021 09:23:45 PDT (-0700), keescook@chromium.org wrote:
> > On Fri, Sep 10, 2021 at 04:40:42PM -0700, Nick Desaulniers wrote:
> > > The minimum supported version of GCC is now 5.1. The check wasn't
> > > correct as written anyways since GCC_VERSION is 0 when CC=clang.
> > > 
> > > Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > Cc: Albert Ou <aou@eecs.berkeley.edu>
> > > Cc: linux-riscv@lists.infradead.org
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > 
> > Yeah, good catch for Clang too.
> > 
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> 
> Only this patch ended up in my inbox.  Where you guys planning on keeping
> this whole series together, or did you want me to pick this for the RISC-V
> tree?

Linus already merged this series (see d20758951f8f28c0ee1b2a8a6bb8189858083895)

-- 
Kees Cook

Re: [PATCH 05/10] riscv: remove Kconfig check for GCC version for ARCH_RV64I

[Palmer Dabbelt]

On Mon, 04 Oct 2021 17:50:46 PDT (-0700), keescook@chromium.org wrote:
> On Mon, Oct 04, 2021 at 05:40:42PM -0700, Palmer Dabbelt wrote:
>> On Mon, 13 Sep 2021 09:23:45 PDT (-0700), keescook@chromium.org wrote:
>> > On Fri, Sep 10, 2021 at 04:40:42PM -0700, Nick Desaulniers wrote:
>> > > The minimum supported version of GCC is now 5.1. The check wasn't
>> > > correct as written anyways since GCC_VERSION is 0 when CC=clang.
>> > >
>> > > Cc: Paul Walmsley <paul.walmsley@sifive.com>
>> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> > > Cc: Albert Ou <aou@eecs.berkeley.edu>
>> > > Cc: linux-riscv@lists.infradead.org
>> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>> >
>> > Yeah, good catch for Clang too.
>> >
>> > Reviewed-by: Kees Cook <keescook@chromium.org>
>>
>> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>
>> Only this patch ended up in my inbox.  Where you guys planning on keeping
>> this whole series together, or did you want me to pick this for the RISC-V
>> tree?
>
> Linus already merged this series (see d20758951f8f28c0ee1b2a8a6bb8189858083895)

Thanks.

[PATCH 06/10] powerpc: remove GCC version check for UPD_CONSTR

[Nick Desaulniers]

Now that GCC 5.1 is the minimum supported version, we can drop this
workaround for older versions of GCC. This adversely affected clang,
too.

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/powerpc/include/asm/asm-const.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-const.h b/arch/powerpc/include/asm/asm-const.h
index 0ce2368bd20f..dbfa5e1e3198 100644
--- a/arch/powerpc/include/asm/asm-const.h
+++ b/arch/powerpc/include/asm/asm-const.h
@@ -12,16 +12,6 @@
 #  define ASM_CONST(x)		__ASM_CONST(x)
 #endif
 
-/*
- * Inline assembly memory constraint
- *
- * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
- *
- */
-#if defined(GCC_VERSION) && GCC_VERSION < 50000
-#define UPD_CONSTR ""
-#else
 #define UPD_CONSTR "<>"
-#endif
 
 #endif /* _ASM_POWERPC_ASM_CONST_H */
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 06/10] powerpc: remove GCC version check for UPD_CONSTR

[Nathan Chancellor]

On 9/10/2021 4:40 PM, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimum supported version, we can drop this
> workaround for older versions of GCC. This adversely affected clang,
> too.
> 
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>   arch/powerpc/include/asm/asm-const.h | 10 ----------
>   1 file changed, 10 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/asm-const.h b/arch/powerpc/include/asm/asm-const.h
> index 0ce2368bd20f..dbfa5e1e3198 100644
> --- a/arch/powerpc/include/asm/asm-const.h
> +++ b/arch/powerpc/include/asm/asm-const.h
> @@ -12,16 +12,6 @@
>   #  define ASM_CONST(x)		__ASM_CONST(x)
>   #endif
>   
> -/*
> - * Inline assembly memory constraint
> - *
> - * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
> - *
> - */
> -#if defined(GCC_VERSION) && GCC_VERSION < 50000
> -#define UPD_CONSTR ""
> -#else
>   #define UPD_CONSTR "<>"
> -#endif

The only reason this exists is because of commit 592bbe9c505d 
("powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9"). It is 
probably just worth sinking "<>" into all of the callsites and removing
UPD_CONSTR.

>   
>   #endif /* _ASM_POWERPC_ASM_CONST_H */
> 

Re: [PATCH 06/10] powerpc: remove GCC version check for UPD_CONSTR

[Michael Ellerman]

Nathan Chancellor <nathan@kernel.org> writes:
> On 9/10/2021 4:40 PM, Nick Desaulniers wrote:
>> Now that GCC 5.1 is the minimum supported version, we can drop this
>> workaround for older versions of GCC. This adversely affected clang,
>> too.
>> 
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Segher Boessenkool <segher@kernel.crashing.org>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>> ---
>>   arch/powerpc/include/asm/asm-const.h | 10 ----------
>>   1 file changed, 10 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/asm-const.h b/arch/powerpc/include/asm/asm-const.h
>> index 0ce2368bd20f..dbfa5e1e3198 100644
>> --- a/arch/powerpc/include/asm/asm-const.h
>> +++ b/arch/powerpc/include/asm/asm-const.h
>> @@ -12,16 +12,6 @@
>>   #  define ASM_CONST(x)		__ASM_CONST(x)
>>   #endif
>>   
>> -/*
>> - * Inline assembly memory constraint
>> - *
>> - * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
>> - *
>> - */
>> -#if defined(GCC_VERSION) && GCC_VERSION < 50000
>> -#define UPD_CONSTR ""
>> -#else
>>   #define UPD_CONSTR "<>"
>> -#endif
>
> The only reason this exists is because of commit 592bbe9c505d 
> ("powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9"). It is 
> probably just worth sinking "<>" into all of the callsites and removing
> UPD_CONSTR.

Yeah that would be great if you're doing a v2. Or we can do it as a
follow-up.

cheers

Re: [PATCH 06/10] powerpc: remove GCC version check for UPD_CONSTR

[Christophe Leroy]

Le 11/09/2021 à 01:40, Nick Desaulniers a écrit :
> Now that GCC 5.1 is the minimum supported version, we can drop this
> workaround for older versions of GCC. This adversely affected clang,
> too.

Why do you say that GCC 5.1 is the minimum supported ?

As far as I can see, the minimum supported is still 4.9, see 
https://github.com/torvalds/linux/blob/master/Documentation/process/changes.rst

> 
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>   arch/powerpc/include/asm/asm-const.h | 10 ----------
>   1 file changed, 10 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/asm-const.h b/arch/powerpc/include/asm/asm-const.h
> index 0ce2368bd20f..dbfa5e1e3198 100644
> --- a/arch/powerpc/include/asm/asm-const.h
> +++ b/arch/powerpc/include/asm/asm-const.h
> @@ -12,16 +12,6 @@
>   #  define ASM_CONST(x)		__ASM_CONST(x)
>   #endif
>   
> -/*
> - * Inline assembly memory constraint
> - *
> - * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
> - *
> - */
> -#if defined(GCC_VERSION) && GCC_VERSION < 50000
> -#define UPD_CONSTR ""
> -#else
>   #define UPD_CONSTR "<>"
> -#endif

There is no point in keeping UPD_CONSTR if it becomes invariant. You 
should just replace all instances of UPD_CONSTR with <> and drop 
UPD_CONSTR completely.

Christophe

[PATCH 07/10] arm64: remove GCC version check for ARCH_SUPPORTS_INT128

[Nick Desaulniers]

Now that GCC 5.1 is the minimally supported compiler version, this
Kconfig check is no longer necessary.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 077f2ec4eeb2..5c7ae4c3954b 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -86,7 +86,7 @@ config ARM64
 	select ARCH_SUPPORTS_LTO_CLANG_THIN
 	select ARCH_SUPPORTS_CFI_CLANG
 	select ARCH_SUPPORTS_ATOMIC_RMW
-	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && (GCC_VERSION >= 50000 || CC_IS_CLANG)
+	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
 	select ARCH_SUPPORTS_NUMA_BALANCING
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION if COMPAT
 	select ARCH_WANT_DEFAULT_BPF_JIT
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 07/10] arm64: remove GCC version check for ARCH_SUPPORTS_INT128

[Nathan Chancellor]

On Fri, Sep 10, 2021 at 04:40:44PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported compiler version, this
> Kconfig check is no longer necessary.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/arm64/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 077f2ec4eeb2..5c7ae4c3954b 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -86,7 +86,7 @@ config ARM64
>  	select ARCH_SUPPORTS_LTO_CLANG_THIN
>  	select ARCH_SUPPORTS_CFI_CLANG
>  	select ARCH_SUPPORTS_ATOMIC_RMW
> -	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && (GCC_VERSION >= 50000 || CC_IS_CLANG)
> +	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
>  	select ARCH_SUPPORTS_NUMA_BALANCING
>  	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION if COMPAT
>  	select ARCH_WANT_DEFAULT_BPF_JIT
> -- 
> 2.33.0.309.g3052b89438-goog
> 
> 

Re: [PATCH 07/10] arm64: remove GCC version check for ARCH_SUPPORTS_INT128

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:44PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported compiler version, this
> Kconfig check is no longer necessary.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH 07/10] arm64: remove GCC version check for ARCH_SUPPORTS_INT128

[Catalin Marinas]

On Fri, Sep 10, 2021 at 04:40:44PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported compiler version, this
> Kconfig check is no longer necessary.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH 08/10] Makefile: drop GCC < 5 -fno-var-tracking-assignments workaround

[Nick Desaulniers]

Now that GCC 5.1 is the minimally supported version, we can drop this
workaround for older versions of GCC.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Makefile b/Makefile
index 2d1e491f7737..f9ef07f573e0 100644
--- a/Makefile
+++ b/Makefile
@@ -849,12 +849,6 @@ endif
 
 DEBUG_CFLAGS	:=
 
-# Workaround for GCC versions < 5.0
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
-ifdef CONFIG_CC_IS_GCC
-DEBUG_CFLAGS	+= $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))
-endif
-
 ifdef CONFIG_DEBUG_INFO
 
 ifdef CONFIG_DEBUG_INFO_SPLIT
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 08/10] Makefile: drop GCC < 5 -fno-var-tracking-assignments workaround

[Nathan Chancellor]

On Fri, Sep 10, 2021 at 04:40:45PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported version, we can drop this
> workaround for older versions of GCC.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  Makefile | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 2d1e491f7737..f9ef07f573e0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -849,12 +849,6 @@ endif
>  
>  DEBUG_CFLAGS	:=
>  
> -# Workaround for GCC versions < 5.0
> -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
> -ifdef CONFIG_CC_IS_GCC
> -DEBUG_CFLAGS	+= $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))
> -endif
> -
>  ifdef CONFIG_DEBUG_INFO
>  
>  ifdef CONFIG_DEBUG_INFO_SPLIT
> -- 
> 2.33.0.309.g3052b89438-goog
> 
> 

Re: [PATCH 08/10] Makefile: drop GCC < 5 -fno-var-tracking-assignments workaround

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:45PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported version, we can drop this
> workaround for older versions of GCC.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH 09/10] compiler-gcc.h: drop checks for older GCC versions

[Nick Desaulniers]

Now that GCC 5.1 is the minimally supported default, drop the values we
don't use.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 include/linux/compiler-gcc.h       | 4 +---
 tools/include/linux/compiler-gcc.h | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 3f7f6fa0e051..fd82ce169ce9 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -98,10 +98,8 @@
 
 #if GCC_VERSION >= 70000
 #define KASAN_ABI_VERSION 5
-#elif GCC_VERSION >= 50000
+#else
 #define KASAN_ABI_VERSION 4
-#elif GCC_VERSION >= 40902
-#define KASAN_ABI_VERSION 3
 #endif
 
 #if __has_attribute(__no_sanitize_address__)
diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
index a590a1dfafd9..43d9a46d36f0 100644
--- a/tools/include/linux/compiler-gcc.h
+++ b/tools/include/linux/compiler-gcc.h
@@ -16,9 +16,7 @@
 # define __fallthrough __attribute__ ((fallthrough))
 #endif
 
-#if GCC_VERSION >= 40300
-# define __compiletime_error(message) __attribute__((error(message)))
-#endif /* GCC_VERSION >= 40300 */
+#define __compiletime_error(message) __attribute__((error(message)))
 
 /* &a[0] degrades to a pointer: a different type from an array */
 #define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 09/10] compiler-gcc.h: drop checks for older GCC versions

[Nathan Chancellor]

On Fri, Sep 10, 2021 at 04:40:46PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported default, drop the values we
> don't use.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  include/linux/compiler-gcc.h       | 4 +---
>  tools/include/linux/compiler-gcc.h | 4 +---
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 3f7f6fa0e051..fd82ce169ce9 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -98,10 +98,8 @@
>  
>  #if GCC_VERSION >= 70000
>  #define KASAN_ABI_VERSION 5
> -#elif GCC_VERSION >= 50000
> +#else
>  #define KASAN_ABI_VERSION 4
> -#elif GCC_VERSION >= 40902
> -#define KASAN_ABI_VERSION 3
>  #endif
>  
>  #if __has_attribute(__no_sanitize_address__)
> diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
> index a590a1dfafd9..43d9a46d36f0 100644
> --- a/tools/include/linux/compiler-gcc.h
> +++ b/tools/include/linux/compiler-gcc.h
> @@ -16,9 +16,7 @@
>  # define __fallthrough __attribute__ ((fallthrough))
>  #endif
>  
> -#if GCC_VERSION >= 40300
> -# define __compiletime_error(message) __attribute__((error(message)))
> -#endif /* GCC_VERSION >= 40300 */
> +#define __compiletime_error(message) __attribute__((error(message)))
>  
>  /* &a[0] degrades to a pointer: a different type from an array */
>  #define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
> -- 
> 2.33.0.309.g3052b89438-goog
> 
> 

Re: [PATCH 09/10] compiler-gcc.h: drop checks for older GCC versions

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:46PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported default, drop the values we
> don't use.
> 
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  include/linux/compiler-gcc.h       | 4 +---
>  tools/include/linux/compiler-gcc.h | 4 +---
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 3f7f6fa0e051..fd82ce169ce9 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -98,10 +98,8 @@
>  
>  #if GCC_VERSION >= 70000
>  #define KASAN_ABI_VERSION 5
> -#elif GCC_VERSION >= 50000
> +#else
>  #define KASAN_ABI_VERSION 4
> -#elif GCC_VERSION >= 40902
> -#define KASAN_ABI_VERSION 3
>  #endif
>  
>  #if __has_attribute(__no_sanitize_address__)

Reviewed-by: Kees Cook <keescook@chromium.org>

> diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
> index a590a1dfafd9..43d9a46d36f0 100644
> --- a/tools/include/linux/compiler-gcc.h
> +++ b/tools/include/linux/compiler-gcc.h
> @@ -16,9 +16,7 @@
>  # define __fallthrough __attribute__ ((fallthrough))
>  #endif
>  
> -#if GCC_VERSION >= 40300
> -# define __compiletime_error(message) __attribute__((error(message)))
> -#endif /* GCC_VERSION >= 40300 */
> +#define __compiletime_error(message) __attribute__((error(message)))
>  
>  /* &a[0] degrades to a pointer: a different type from an array */
>  #define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))

This part could be applied regardless (it's a missed 4.3 check, yes?)

-- 
Kees Cook

[PATCH 10/10] vmlinux.lds.h: remove old check for GCC 4.9

[Nick Desaulniers]

Now that GCC 5.1 is the minimally supported version of GCC, we can
effectively revert

commit 85c2ce9104eb ("sched, vmlinux.lds: Increase STRUCT_ALIGNMENT to
64 bytes for GCC-4.9")

Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 include/asm-generic/vmlinux.lds.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index aa50bf2959fe..f2984af2b85b 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -116,11 +116,7 @@
  * GCC 4.5 and later have a 32 bytes section alignment for structures.
  * Except GCC 4.9, that feels the need to align on 64 bytes.
  */
-#if __GNUC__ == 4 && __GNUC_MINOR__ == 9
-#define STRUCT_ALIGNMENT 64
-#else
 #define STRUCT_ALIGNMENT 32
-#endif
 #define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
 
 /*
-- 
2.33.0.309.g3052b89438-goog

Re: [PATCH 10/10] vmlinux.lds.h: remove old check for GCC 4.9

[Nathan Chancellor]

On 9/10/2021 4:40 PM, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported version of GCC, we can
> effectively revert
> 
> commit 85c2ce9104eb ("sched, vmlinux.lds: Increase STRUCT_ALIGNMENT to
> 64 bytes for GCC-4.9")
> 
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>   include/asm-generic/vmlinux.lds.h | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index aa50bf2959fe..f2984af2b85b 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -116,11 +116,7 @@
>    * GCC 4.5 and later have a 32 bytes section alignment for structures.
>    * Except GCC 4.9, that feels the need to align on 64 bytes.

Comment should be adjusted.

>    */
> -#if __GNUC__ == 4 && __GNUC_MINOR__ == 9
> -#define STRUCT_ALIGNMENT 64
> -#else
>   #define STRUCT_ALIGNMENT 32
> -#endif
>   #define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
>   
>   /*
> 

Re: [PATCH 10/10] vmlinux.lds.h: remove old check for GCC 4.9

[Nick Desaulniers]

On Fri, Sep 10, 2021 at 4:51 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On 9/10/2021 4:40 PM, Nick Desaulniers wrote:
> > Now that GCC 5.1 is the minimally supported version of GCC, we can
> > effectively revert
> >
> > commit 85c2ce9104eb ("sched, vmlinux.lds: Increase STRUCT_ALIGNMENT to
> > 64 bytes for GCC-4.9")
> >
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >   include/asm-generic/vmlinux.lds.h | 4 ----
> >   1 file changed, 4 deletions(-)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index aa50bf2959fe..f2984af2b85b 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -116,11 +116,7 @@
> >    * GCC 4.5 and later have a 32 bytes section alignment for structures.
> >    * Except GCC 4.9, that feels the need to align on 64 bytes.
>
> Comment should be adjusted.

I thought it was still interesting. Perhaps one day it will be super irrelevant.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 10/10] vmlinux.lds.h: remove old check for GCC 4.9

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:47PM -0700, Nick Desaulniers wrote:
> Now that GCC 5.1 is the minimally supported version of GCC, we can
> effectively revert
> 
> commit 85c2ce9104eb ("sched, vmlinux.lds: Increase STRUCT_ALIGNMENT to
> 64 bytes for GCC-4.9")
> 
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  include/asm-generic/vmlinux.lds.h | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index aa50bf2959fe..f2984af2b85b 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -116,11 +116,7 @@
>   * GCC 4.5 and later have a 32 bytes section alignment for structures.
>   * Except GCC 4.9, that feels the need to align on 64 bytes.
>   */
> -#if __GNUC__ == 4 && __GNUC_MINOR__ == 9
> -#define STRUCT_ALIGNMENT 64
> -#else
>  #define STRUCT_ALIGNMENT 32
> -#endif
>  #define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)

Yeah, looking at users of the macro, I think it's best to keep the macro
instead of hard-coding it everywhere.

Acked-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH 00/10] raise minimum GCC version to 5.1

[Kees Cook]

On Fri, Sep 10, 2021 at 04:40:37PM -0700, Nick Desaulniers wrote:
> commit fad7cd3310db ("nbd: add the check to prevent overflow in
> __nbd_ioctl()")
> 
> raised an issue from the fallback helpers added in
> 
> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
> 
> Specifically, the helpers for checking whether the results of a
> multiplication overflowed (__unsigned_mul_overflow,
> __signed_add_overflow) use the division operator when
> !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
> operands on 32b hosts.
> 
> Also, because the macro is type agnostic, it is very difficult to write
> a similarly type generic macro that dispatches to one of:
> * div64_s64
> * div64_u64
> * div_s64
> * div_u64

Given that it's all compile-time type-aware goo, this isn't so bad. The
gist[1] you linked off the bug report is pretty close. Needs some
bikeshedding. ;)

> Raising the minimum supported versions allows us to remove all of the
> fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
> dispatching the compiler builtins.
> 
> arm64 has already raised the minimum supported GCC version to 5.1, do
> this for all targets now. See the link below for the previous
> discussion.

That said, I'd much prefer raising the minimum GCC -- no one appears
to actually be building on 4.9 -- there are close to 200 errors (neé
warnings) on x86_64 allmodconfig there currently.

-Kees

[1] https://gist.github.com/nickdesaulniers/2479818f4983bbf2d688cebbab435863

-- 
Kees Cook

Re: [PATCH 00/10] raise minimum GCC version to 5.1

[Michael Ellerman]

Kees Cook <keescook@chromium.org> writes:
> On Fri, Sep 10, 2021 at 04:40:37PM -0700, Nick Desaulniers wrote:
>> commit fad7cd3310db ("nbd: add the check to prevent overflow in
>> __nbd_ioctl()")
>> 
>> raised an issue from the fallback helpers added in
>> 
>> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
>> 
>> Specifically, the helpers for checking whether the results of a
>> multiplication overflowed (__unsigned_mul_overflow,
>> __signed_add_overflow) use the division operator when
>> !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
>> operands on 32b hosts.
>> 
>> Also, because the macro is type agnostic, it is very difficult to write
>> a similarly type generic macro that dispatches to one of:
>> * div64_s64
>> * div64_u64
>> * div_s64
>> * div_u64
>
> Given that it's all compile-time type-aware goo, this isn't so bad. The
> gist[1] you linked off the bug report is pretty close. Needs some
> bikeshedding. ;)
>
>> Raising the minimum supported versions allows us to remove all of the
>> fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
>> dispatching the compiler builtins.
>> 
>> arm64 has already raised the minimum supported GCC version to 5.1, do
>> this for all targets now. See the link below for the previous
>> discussion.
>
> That said, I'd much prefer raising the minimum GCC -- no one appears
> to actually be building on 4.9 -- there are close to 200 errors (neé
> warnings) on x86_64 allmodconfig there currently.

I still do 4.9 builds on kisskb, but I agree there are a lot of
warnings, and no one ever has time to fix any.

cheers

Re: [PATCH 00/10] raise minimum GCC version to 5.1

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

Hi!

> commit fad7cd3310db ("nbd: add the check to prevent overflow in
> __nbd_ioctl()")
> 
> raised an issue from the fallback helpers added in
> 
> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
> 
> Specifically, the helpers for checking whether the results of a
> multiplication overflowed (__unsigned_mul_overflow,
> __signed_add_overflow) use the division operator when
> !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
> operands on 32b hosts.

Please don't. gcc 4.9.2 is still in use here.

Plus, someone will want to prevent overflow in -stable, too, and you
can't really raise gcc version there.

Best regards,
									Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 00/10] raise minimum GCC version to 5.1

[Kees Cook]

On Mon, Sep 13, 2021 at 11:49:48AM +0200, Pavel Machek wrote:
> Hi!
> 
> > commit fad7cd3310db ("nbd: add the check to prevent overflow in
> > __nbd_ioctl()")
> > 
> > raised an issue from the fallback helpers added in
> > 
> > commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
> > 
> > Specifically, the helpers for checking whether the results of a
> > multiplication overflowed (__unsigned_mul_overflow,
> > __signed_add_overflow) use the division operator when
> > !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
> > operands on 32b hosts.
> 
> Please don't. gcc 4.9.2 is still in use here.
> 
> Plus, someone will want to prevent overflow in -stable, too, and you
> can't really raise gcc version there.

These changes won't go to prior stable kernels.

-- 
Kees Cook

Re: [PATCH 00/10] raise minimum GCC version to 5.1

[Arnd Bergmann]

On Sat, Sep 11, 2021 at 1:41 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> commit fad7cd3310db ("nbd: add the check to prevent overflow in
> __nbd_ioctl()")
>
> raised an issue from the fallback helpers added in
>
> commit f0907827a8a9 ("compiler.h: enable builtin overflow checkers and add fallback code")
>
> Specifically, the helpers for checking whether the results of a
> multiplication overflowed (__unsigned_mul_overflow,
> __signed_add_overflow) use the division operator when
> !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW. This is problematic for 64b
> operands on 32b hosts.
>
> Also, because the macro is type agnostic, it is very difficult to write
> a similarly type generic macro that dispatches to one of:
> * div64_s64
> * div64_u64
> * div_s64
> * div_u64
>
> Raising the minimum supported versions allows us to remove all of the
> fallback helpers for !COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW, instead
> dispatching the compiler builtins.
>
> arm64 has already raised the minimum supported GCC version to 5.1, do
> this for all targets now. See the link below for the previous
> discussion.
>
> Additional patches cleaning up a few obsolete version checks.

Agreed, I think this is a good time for removing gcc-4.9 support, with
the tradeoff between the number of remaining users of that compiler and
the number of problems that are solved in gcc-5.1.

After this, I think we can also change the --std=gnu89 to --std=gnu11,
as an additional benefit.

According to distrowatch, this will lose support for the distro gcc version
of Debian 8 (Jessie), Ubuntu 15.04, and older Android AOSP (new versions
use clang to build kernels, old versions also need older kernels).
I think that is acceptable.

For reference, the following distro releases use

Centos 8: gcc-8.3
Debian 9: gcc-6.3
RHEL 8: gcc-8.4
Slackware-14.2: gcc-5.3
SLES15: gcc-10.2
Ubuntu 16.04: gcc-5.3

Most older releases of these already don't support building current kernels.
I expect the number of users of Ubuntu 16.04 and Slackware-14.2 that
want to build their own mainline kernels instead of running what was
provided by the distro to be really low.

The other case that always gets brought up are embedded users that
want to use an ancient user space that is only validated with an old gcc,
but need a new kernel without revalidating user space. The only answer
for those is to use two different compilers.

       Arnd