[PATCH v2 1/3] compiler-gcc: hide COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW from sparse

[PATCH v2 1/3] compiler-gcc: hide COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW from sparse

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Sparse doesn't support all the overflow builtins, so just hide
them from it to avoid lots of warnings/errors reported by it.

We could try to teach them to sparse, but the passed types are
variable, and sparse doesn't seem to handle that well.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/compiler-gcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 2010493e1040..3154f2a84571 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -143,7 +143,7 @@
 #define KASAN_ABI_VERSION 3
 #endif
 
-#if GCC_VERSION >= 50100
+#if GCC_VERSION >= 50100 && !defined(__CHECKER__)
 #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
 #endif
 
-- 
2.17.2

[PATCH v2 2/3] kernel.h: hide __is_constexpr() from sparse

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

__is_constexpr() is a work of art, understandable only to the most
respected wizards of C. Even sparse doesn't seem to belong to that
group and warns that there's an "expression using sizeof(void)".

Just hide the definition from sparse and pretend it's always true.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/kernel.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d6aac75b51ba..d4d2233f95c9 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -844,6 +844,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #define __typecheck(x, y) \
 		(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
 
+#ifndef __CHECKER__
 /*
  * This returns a constant expression while determining if an argument is
  * a constant expression, most importantly without evaluating the argument.
@@ -851,6 +852,13 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
  */
 #define __is_constexpr(x) \
 	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
+#else
+/*
+ * We don't really care about the check when running sparse and the
+ * above expression causes a warning due to sizeof(void).
+ */
+#define __is_constexpr(x) 1
+#endif
 
 #define __no_side_effects(x, y) \
 		(__is_constexpr(x) && __is_constexpr(y))
-- 
2.17.2

Re: [PATCH v2 2/3] kernel.h: hide __is_constexpr() from sparse

[Luc Van Oostenryck]

On Fri, Nov 09, 2018 at 10:35:33AM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> __is_constexpr() is a work of art, understandable only to the most
> respected wizards of C. Even sparse doesn't seem to belong to that
> group and warns that there's an "expression using sizeof(void)".
> 
> Just hide the definition from sparse and pretend it's always true.

The development version of sparse doesn't issues a warning for
sizeof(void) soon after the introduction of __is_constexpr()
and sparse's main tree have been updated (very recently).

I strongly believe this patch shouldn't be.

Regards,
-- Luc

[PATCH v2 3/3] slab: use logical instead of bitwise operation in kmalloc_type()

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The operation here really is more logical than bitwise, even if
due to the setup the bitwise operation works fine. However, this
causes a complaint from sparse that the operation doesn't really
make sense due to the not.

Use a logical and instead of bitwise.

In my (somewhat unscientific) test this caused no differences in
the generated code.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/slab.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 918f374e7156..d395c7366312 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -329,7 +329,7 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
 	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
 	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
 	 */
-	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
+	return type_dma + (is_reclaimable && !is_dma) * KMALLOC_RECLAIM;
 }
 
 /*
-- 
2.17.2

Re: [PATCH v2 3/3] slab: use logical instead of bitwise operation in kmalloc_type()

[Andrew Morton]

On Fri,  9 Nov 2018 10:35:34 +0100 Johannes Berg <johannes@sipsolutions.net> wrote:

> The operation here really is more logical than bitwise, even if
> due to the setup the bitwise operation works fine. However, this
> causes a complaint from sparse that the operation doesn't really
> make sense due to the not.
> 
> Use a logical and instead of bitwise.
> 
> In my (somewhat unscientific) test this caused no differences in
> the generated code.
> 
> ...
>
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -329,7 +329,7 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
>  	 * If an allocation is both __GFP_DMA and __GFP_RECLAIMABLE, return
>  	 * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
>  	 */
> -	return type_dma + (is_reclaimable & !is_dma) * KMALLOC_RECLAIM;
> +	return type_dma + (is_reclaimable && !is_dma) * KMALLOC_RECLAIM;
>  }
>  
>  /*

Thanks.  There's presently a bit of head-scratching going on over this.
http://lkml.kernel.org/r/20181105204000.129023-1-bvanassche@acm.org -
please feel free to weigh in ;)

Re: [PATCH v2 1/3] compiler-gcc: hide COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW from sparse

[Luc Van Oostenryck]

On Fri, Nov 09, 2018 at 10:35:32AM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Sparse doesn't support all the overflow builtins, so just hide
> them from it to avoid lots of warnings/errors reported by it.

The development version of sparse support these builtins
since their introduction in the kernel and sparse's main tree
have been updated (very recently).

I strongly believe this patch shouldn't be.

Regards,
-- Luc

Re: [PATCH v2 1/3] compiler-gcc: hide COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW from sparse

[Johannes Berg]

On Sat, 2018-11-17 at 01:42 +0100, Luc Van Oostenryck wrote:
> On Fri, Nov 09, 2018 at 10:35:32AM +0100, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > Sparse doesn't support all the overflow builtins, so just hide
> > them from it to avoid lots of warnings/errors reported by it.
> 
> The development version of sparse support these builtins
> since their introduction in the kernel and sparse's main tree
> have been updated (very recently).

All the better, but I certainly checked sparse git before even
considering this patch, so saying "but it's all there" feels somewhat
dishonest. Also, the sparse repo is 'backdated' to the end of October.
I'm almost certain this wasn't present when I sent the patch.

However, it would be nice to be able to use distro sparse versions, so
not sure I fully agree that we shouldn't apply such patches.

johannes