[PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Sean Christopherson]

Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
of the same name defined by linux/overflow.h.  Note, overflow.h's version
takes a type as the input, whereas the harness's version takes a variable!

This fixes warnings (and presumably potential test failures) in tests
that utilize the selftests harness and happen to (indirectly) include
overflow.h.

  In file included from tools/include/linux/bits.h:34,
                   from tools/include/linux/bitops.h:14,
                   from tools/include/linux/hashtable.h:13,
                   from include/kvm_util.h:11,
                   from x86/userspace_msr_exit_test.c:11:
  tools/include/linux/overflow.h:31:9: error: "is_signed_type" redefined [-Werror]
     31 | #define is_signed_type(type)       (((type)(-1)) < (type)1)
        |         ^~~~~~~~~~~~~~
  In file included from include/kvm_test_harness.h:11,
                   from x86/userspace_msr_exit_test.c:9:
  ../kselftest_harness.h:754:9: note: this is the location of the previous definition
    754 | #define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
        |         ^~~~~~~~~~~~~~

Opportunistically use is_signed_type() to implement is_signed_var() so
that the relationship and differences are obvious.

Fixes: fc92099902fb ("tools headers: Synchronize linux/bits.h with the kernel sources")
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---

This is probably compile-tested only, I don't think any of the KVM selftests
utilize the harness's EXPECT macros.

 tools/testing/selftests/kselftest_harness.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 2925e47db995..f3e7a46345db 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -56,6 +56,7 @@
 #include <asm/types.h>
 #include <ctype.h>
 #include <errno.h>
+#include <linux/overflow.h>
 #include <linux/unistd.h>
 #include <poll.h>
 #include <stdbool.h>
@@ -751,7 +752,7 @@
 	for (; _metadata->trigger; _metadata->trigger = \
 			__bail(_assert, _metadata))
 
-#define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
+#define is_signed_var(var)	is_signed_type(__typeof__(var))
 
 #define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
 	/* Avoid multiple evaluation of the cases */ \
@@ -759,7 +760,7 @@
 	__typeof__(_seen) __seen = (_seen); \
 	if (!(__exp _t __seen)) { \
 		/* Report with actual signedness to avoid weird output. */ \
-		switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { \
+		switch (is_signed_var(__exp) * 2 + is_signed_var(__seen)) { \
 		case 0: { \
 			uintmax_t __exp_print = (uintmax_t)__exp; \
 			uintmax_t __seen_print = (uintmax_t)__seen; \

base-commit: 78f4e737a53e1163ded2687a922fce138aee73f5
-- 
2.50.0.714.g196bf9f422-goog

Re: [PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Vincent Mailhol]

On 25/06/2025 at 08:19, Sean Christopherson wrote:
> Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
> of the same name defined by linux/overflow.h.  Note, overflow.h's version
> takes a type as the input, whereas the harness's version takes a variable!
> 
> This fixes warnings (and presumably potential test failures) in tests
> that utilize the selftests harness and happen to (indirectly) include
> overflow.h.
> 
>   In file included from tools/include/linux/bits.h:34,
>                    from tools/include/linux/bitops.h:14,
>                    from tools/include/linux/hashtable.h:13,
>                    from include/kvm_util.h:11,
>                    from x86/userspace_msr_exit_test.c:11:
>   tools/include/linux/overflow.h:31:9: error: "is_signed_type" redefined [-Werror]
>      31 | #define is_signed_type(type)       (((type)(-1)) < (type)1)
>         |         ^~~~~~~~~~~~~~
>   In file included from include/kvm_test_harness.h:11,
>                    from x86/userspace_msr_exit_test.c:9:
>   ../kselftest_harness.h:754:9: note: this is the location of the previous definition
>     754 | #define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
>         |         ^~~~~~~~~~~~~~
> 
> Opportunistically use is_signed_type() to implement is_signed_var() so
> that the relationship and differences are obvious.
> 
> Fixes: fc92099902fb ("tools headers: Synchronize linux/bits.h with the kernel sources")
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> 
> This is probably compile-tested only, I don't think any of the KVM selftests
> utilize the harness's EXPECT macros.
> 
>  tools/testing/selftests/kselftest_harness.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 2925e47db995..f3e7a46345db 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -56,6 +56,7 @@
>  #include <asm/types.h>
>  #include <ctype.h>
>  #include <errno.h>
> +#include <linux/overflow.h>
>  #include <linux/unistd.h>
>  #include <poll.h>
>  #include <stdbool.h>
> @@ -751,7 +752,7 @@
>  	for (; _metadata->trigger; _metadata->trigger = \
>  			__bail(_assert, _metadata))
>  
> -#define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
> +#define is_signed_var(var)	is_signed_type(__typeof__(var))

It would potentially make sense to add the is_signed_var() to overflow.h. I see
a couple places doing some is_signed_type(typeof(x)):

  $ git grep "is_signed_type(typeof" | wc -l
  5

And I can imagine this number growing in the future.

But this is by no way a reason strong enough to block this patch. It is just a
random idea I am sharing here.

So that said:

Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

>  #define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
>  	/* Avoid multiple evaluation of the cases */ \
> @@ -759,7 +760,7 @@
>  	__typeof__(_seen) __seen = (_seen); \
>  	if (!(__exp _t __seen)) { \
>  		/* Report with actual signedness to avoid weird output. */ \
> -		switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { \
> +		switch (is_signed_var(__exp) * 2 + is_signed_var(__seen)) { \
>  		case 0: { \
>  			uintmax_t __exp_print = (uintmax_t)__exp; \
>  			uintmax_t __seen_print = (uintmax_t)__seen; \
> 
> base-commit: 78f4e737a53e1163ded2687a922fce138aee73f5


Yours sincerely,
Vincent Mailhol

Re: [PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Kees Cook]

On Tue, Jun 24, 2025 at 04:19:30PM -0700, Sean Christopherson wrote:
> Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
> of the same name defined by linux/overflow.h.  Note, overflow.h's version
> takes a type as the input, whereas the harness's version takes a variable!

Can we just update compiler.h to use typeof() and drop duplicates?
(typeof() a type is a pass-thru). Totally untested:

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 6f04a1d8c720..cb925b883806 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -347,7 +347,7 @@ static inline void *offset_to_ptr(const int *off)
  * Whether 'type' is a signed type or an unsigned type. Supports scalar types,
  * bool and also pointer types.
  */
-#define is_signed_type(type) (((type)(-1)) < (__force type)1)
+#define is_signed_type(type) (((typeof(type))(-1)) < (__force typeof(type))1)
 #define is_unsigned_type(type) (!is_signed_type(type))
 
 /*


-- 
Kees Cook

Re: [PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Sean Christopherson]

On Fri, Jun 27, 2025, Kees Cook wrote:
> On Tue, Jun 24, 2025 at 04:19:30PM -0700, Sean Christopherson wrote:
> > Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
> > of the same name defined by linux/overflow.h.  Note, overflow.h's version
> > takes a type as the input, whereas the harness's version takes a variable!
> 
> Can we just update compiler.h to use typeof() and drop duplicates?

I have no objection to doing so, but I'd prefer any such change go on top.  I'd
like to get this fixed in 6.16, and as your diff alludes to, I think tools' version
of compiler.h should follow the kernel's version (and presumably the below isn't
an rc6+ candidate :-) ).

> (typeof() a type is a pass-thru). Totally untested:
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 6f04a1d8c720..cb925b883806 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -347,7 +347,7 @@ static inline void *offset_to_ptr(const int *off)
>   * Whether 'type' is a signed type or an unsigned type. Supports scalar types,
>   * bool and also pointer types.
>   */
> -#define is_signed_type(type) (((type)(-1)) < (__force type)1)
> +#define is_signed_type(type) (((typeof(type))(-1)) < (__force typeof(type))1)
>  #define is_unsigned_type(type) (!is_signed_type(type))
>  
>  /*
> 
> 
> -- 
> Kees Cook

Re: [PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Sean Christopherson]

On Tue, 24 Jun 2025 16:19:30 -0700, Sean Christopherson wrote:
> Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
> of the same name defined by linux/overflow.h.  Note, overflow.h's version
> takes a type as the input, whereas the harness's version takes a variable!
> 
> This fixes warnings (and presumably potential test failures) in tests
> that utilize the selftests harness and happen to (indirectly) include
> overflow.h.
> 
> [...]

Applied to kvm-x86 fixes, as this is still a big thorn in my side.

[1/1] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h
      https://github.com/kvm-x86/linux/commit/40d5b2ab0d72

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Mark Brown]

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

On Tue, Jun 24, 2025 at 04:19:30PM -0700, Sean Christopherson wrote:
> Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
> of the same name defined by linux/overflow.h.  Note, overflow.h's version
> takes a type as the input, whereas the harness's version takes a variable!

This patch is in -next and is causing widespread breakage in the
selftests -next on at least arm and arm64 due to:

make --silent --keep-going --jobs=15 O=/build/stage/build-work INSTALL_PATH=/build/stage/build-work/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install

...

In file included from test-pcmtest-driver.c:10:
../kselftest_harness.h:59:10: fatal error: linux/overflow.h: No such file or directory
   59 | #include <linux/overflow.h>
      |          ^~~~~~~~~~~~~~~~~~
compilation terminated.

Sample build log:

   https://builds.sirena.org.uk/5303936d609e09665deda94eaedf26a0e5c3a087/arm64/defconfig/build.log

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Sean Christopherson]

On Wed, Aug 20, 2025, Mark Brown wrote:
> On Tue, Jun 24, 2025 at 04:19:30PM -0700, Sean Christopherson wrote:
> > Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
> > of the same name defined by linux/overflow.h.  Note, overflow.h's version
> > takes a type as the input, whereas the harness's version takes a variable!
> 
> This patch is in -next and is causing widespread breakage in the
> selftests -next on at least arm and arm64 due to:
> 
> make --silent --keep-going --jobs=15 O=/build/stage/build-work INSTALL_PATH=/build/stage/build-work/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install
> 
> ...
> 
> In file included from test-pcmtest-driver.c:10:
> ../kselftest_harness.h:59:10: fatal error: linux/overflow.h: No such file or directory
>    59 | #include <linux/overflow.h>
>       |          ^~~~~~~~~~~~~~~~~~
> compilation terminated.

Argh, many selftests don't add tools/include to their include path.  The least
awful thing I can think of is to go with a minimal fix to avoid the collision.
AFAICT, nothing outside of kselftest_harness.h uses is_signed_type(), so this
shouldn't cause a different flavor of breakage?

--
From: Sean Christopherson <seanjc@google.com>
Date: Tue, 24 Jun 2025 16:19:30 -0700
Subject: [PATCH] selftests: harness: Rework is_signed_type() to avoid
 collision with overflow.h

Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
of the same name defined by tools' linux/overflow.h.  This fixes warnings
(and presumably potential test failures) in tests that utilize the
selftests harness and happen to (indirectly) include overflow.h.

  In file included from tools/include/linux/bits.h:34,
                   from tools/include/linux/bitops.h:14,
                   from tools/include/linux/hashtable.h:13,
                   from include/kvm_util.h:11,
                   from x86/userspace_msr_exit_test.c:11:
  tools/include/linux/overflow.h:31:9: error: "is_signed_type" redefined [-Werror]
     31 | #define is_signed_type(type)       (((type)(-1)) < (type)1)
        |         ^~~~~~~~~~~~~~
  In file included from include/kvm_test_harness.h:11,
                   from x86/userspace_msr_exit_test.c:9:
  ../kselftest_harness.h:754:9: note: this is the location of the previous definition
    754 | #define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
        |         ^~~~~~~~~~~~~~

Use a separate definition, at least for now, as many selftests build
without tools/include in their include path.

Fixes: fc92099902fb ("tools headers: Synchronize linux/bits.h with the kernel sources")
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Link: https://lore.kernel.org/r/20250624231930.583689-1-seanjc@google.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kselftest_harness.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 2925e47db995..8516e8434bc4 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -751,7 +751,7 @@
 	for (; _metadata->trigger; _metadata->trigger = \
 			__bail(_assert, _metadata))
 
-#define is_signed_type(var)       (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
+#define is_signed_var(var)	(!!(((__typeof__(var))(-1)) < (__typeof__(var))1))
 
 #define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
 	/* Avoid multiple evaluation of the cases */ \
@@ -759,7 +759,7 @@
 	__typeof__(_seen) __seen = (_seen); \
 	if (!(__exp _t __seen)) { \
 		/* Report with actual signedness to avoid weird output. */ \
-		switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { \
+		switch (is_signed_var(__exp) * 2 + is_signed_var(__seen)) { \
 		case 0: { \
 			uintmax_t __exp_print = (uintmax_t)__exp; \
 			uintmax_t __seen_print = (uintmax_t)__seen; \

base-commit: 923fcb3dbc0246fc5207093c0049af4c56f20e41
--

Re: [PATCH] selftests: harness: Rework is_signed_type() to avoid collision with overflow.h

[Sean Christopherson]

On Wed, Aug 20, 2025, Sean Christopherson wrote:
> On Wed, Aug 20, 2025, Mark Brown wrote:
> > On Tue, Jun 24, 2025 at 04:19:30PM -0700, Sean Christopherson wrote:
> > > Rename is_signed_type() to is_signed_var() to avoid colliding with a macro
> > > of the same name defined by linux/overflow.h.  Note, overflow.h's version
> > > takes a type as the input, whereas the harness's version takes a variable!
> > 
> > This patch is in -next and is causing widespread breakage in the
> > selftests -next on at least arm and arm64 due to:
> > 
> > make --silent --keep-going --jobs=15 O=/build/stage/build-work INSTALL_PATH=/build/stage/build-work/kselftest_install ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install
> > 
> > ...
> > 
> > In file included from test-pcmtest-driver.c:10:
> > ../kselftest_harness.h:59:10: fatal error: linux/overflow.h: No such file or directory
> >    59 | #include <linux/overflow.h>
> >       |          ^~~~~~~~~~~~~~~~~~
> > compilation terminated.
> 
> Argh, many selftests don't add tools/include to their include path.  The least
> awful thing I can think of is to go with a minimal fix to avoid the collision.
> AFAICT, nothing outside of kselftest_harness.h uses is_signed_type(), so this
> shouldn't cause a different flavor of breakage?

I force-pushed a straight rename.  Please holler if that somehow still breaks
builds.

[1/1] selftests: harness: Rename is_signed_type() to avoid collision with overflow.h
      https://github.com/kvm-x86/linux/commit/dce1b33ed743