[PATCH] make sparse happy with gfp.h

[PATCH] make sparse happy with gfp.h

[Dave Hansen]

Running sparse on page_alloc.c today, it errors out:
        include/linux/gfp.h:254:17: error: bad constant expression
        include/linux/gfp.h:254:17: error: cannot size expression

which is a line in gfp_zone():

        BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);

That's really unfortunate, because it ends up hiding all of the other
legitimate sparse messages like this:
        mm/page_alloc.c:5315:59: warning: incorrect type in argument 1 (different base types)
        mm/page_alloc.c:5315:59:    expected unsigned long [unsigned] [usertype] size
        mm/page_alloc.c:5315:59:    got restricted gfp_t [usertype] <noident>
...

Having sparse be able to catch these very oopsable bugs is a lot more
important than keeping a BUILD_BUG_ON().  Kill the BUILD_BUG_ON().

Compiles on x86_64 with and without CONFIG_DEBUG_VM=y.  defconfig
boots fine for me.

Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
---

 linux-2.6.git-dave/include/linux/gfp.h |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
--- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
+++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
@@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
 
 	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
 					 ((1 << ZONES_SHIFT) - 1);
-
-	if (__builtin_constant_p(bit))
-		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
-	else {
 #ifdef CONFIG_DEBUG_VM
-		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
+	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
 #endif
-	}
 	return z;
 }
 
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] make sparse happy with gfp.h

[KOSAKI Motohiro]

Hello,

> diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
> --- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
> +++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
> @@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
>  
>  	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
>  					 ((1 << ZONES_SHIFT) - 1);
> -
> -	if (__builtin_constant_p(bit))
> -		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> -	else {
>  #ifdef CONFIG_DEBUG_VM
> -		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> +	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
>  #endif
> -	}
>  	return z;

Why don't you use VM_BUG_ON?


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] make sparse happy with gfp.h

[Dave Hansen]

On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> >  #ifdef CONFIG_DEBUG_VM
> > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> >  #endif
> > -     }
> >       return z;
> 
> Why don't you use VM_BUG_ON?

I was just trying to make a minimal patch that did a single thing.

Feel free to submit another one that does that.  I'm sure there are a
couple more places that could use similar love.

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] define dummy BUILD_BUG_ON definition for sparse

[KOSAKI Motohiro]

> Hello,
> 
> > diff -puN include/linux/gfp.h~make-sparse-happy-with-gfp_h include/linux/gfp.h
> > --- linux-2.6.git/include/linux/gfp.h~make-sparse-happy-with-gfp_h	2011-04-14 14:47:02.629275904 -0700
> > +++ linux-2.6.git-dave/include/linux/gfp.h	2011-04-14 14:47:38.813272674 -0700
> > @@ -249,14 +249,9 @@ static inline enum zone_type gfp_zone(gf
> >  
> >  	z = (GFP_ZONE_TABLE >> (bit * ZONES_SHIFT)) &
> >  					 ((1 << ZONES_SHIFT) - 1);
> > -
> > -	if (__builtin_constant_p(bit))
> > -		BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > -	else {
> >  #ifdef CONFIG_DEBUG_VM
> > -		BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > +	BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> >  #endif
> > -	}
> >  	return z;
> 
> Why don't you use VM_BUG_ON?

After while thinking, I decided to make another patch. If we take your
approach we will remove all BUILD_BUG_ON eventually. It's no happy result.


From 2da32b2875a6bd0bb0166993b4663eac0c5d1d6d Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:37:24 +0900
Subject: [PATCH] define dummy BUILD_BUG_ON definition for sparse

BUILD_BUG_ON() makes syntax error to detect coding error. Then it
naturally makes sparse error too. It reduce sparse usefulness.

Then, this patch makes dummy BUILD_BUG_ON() definition for sparse.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 include/linux/kernel.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 00cec4d..9ac44b8 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -637,6 +637,14 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
+#ifdef __CHECKER__
+#define BUILD_BUG_ON_NOT_POWER_OF_2(n)
+#define BUILD_BUG_ON_ZERO(e)
+#define BUILD_BUG_ON_NULL(e)
+#define BUILD_BUG_ON(condition)
+#else /* __CHECKER__ */
+
 /* Force a compilation error if a constant expression is not a power of 2 */
 #define BUILD_BUG_ON_NOT_POWER_OF_2(n)			\
 	BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
@@ -673,6 +681,7 @@ extern int __build_bug_on_failed;
 		if (condition) __build_bug_on_failed = 1;	\
 	} while(0)
 #endif
+#endif /* __CHECKER__ */
 
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
-- 
1.7.3.1





--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] define __must_be_array() for __CHECKER__

[KOSAKI Motohiro]

This fixes another sparse splat.


From 711131e2e16925970a67103156af1296993dbc93 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:28:16 +0900
Subject: [PATCH] define __must_be_array() for __CHECKER__

commit c5e631cf65f (ARRAY_SIZE: check for type) added __must_be_array().
but sparse can't parse this gcc extention.

Then, now make C=2 makes following sparse errors a lot.

  kernel/futex.c:2699:25: error: No right hand side of '+'-expression

Because __must_be_array() is used for ARRAY_SIZE() macro and it is
used very widely.

This patch fixes it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 include/linux/compiler-gcc.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index cb4c1eb..59e4028 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -34,8 +34,12 @@
     __asm__ ("" : "=r"(__ptr) : "0"(ptr));		\
     (typeof(ptr)) (__ptr + (off)); })
 
+#ifdef __CHECKER__
+#define __must_be_array(arr) 0
+#else
 /* &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]))
+#endif
 
 /*
  * Force always-inline if the user requests it so via the .config,
-- 
1.7.3.1



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] Undef __compiletime_{warning,error} if __CHECKER__ is defined

[KOSAKI Motohiro]

From 4560a800c056714a7b5f9dc813460698717e6998 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Fri, 15 Apr 2011 13:58:07 +0900
Subject: [PATCH] Undef __compiletime_{warning,error} if __CHECKER__ is defined

sparse can't parse warning and error attribute. then they should
be hidden from sparse.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Arjan van de Ven <arjan@infradead.org>
---
 include/linux/compiler-gcc4.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 64b7c00..dfadc96 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -51,7 +51,7 @@
 #if __GNUC_MINOR__ > 0
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 #endif
-#if __GNUC_MINOR__ >= 4
+#if __GNUC_MINOR__ >= 4 && !defined(__CHECKER__)
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 #endif
-- 
1.7.3.1



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] make sparse happy with gfp.h

[KOSAKI Motohiro]

Hello,

> On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> > >  #ifdef CONFIG_DEBUG_VM
> > > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > >  #endif
> > > -     }
> > >       return z;
> > 
> > Why don't you use VM_BUG_ON?
> 
> I was just trying to make a minimal patch that did a single thing.
> 
> Feel free to submit another one that does that.  I'm sure there are a
> couple more places that could use similar love.

I posted another approach patches a second ago. Could you please see it?



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] fix sparse happy borkage when including gfp.h

[Dave Hansen]

On Fri, 2011-04-15 at 14:33 +0900, KOSAKI Motohiro wrote:
> Hello,
> > On Fri, 2011-04-15 at 12:14 +0900, KOSAKI Motohiro wrote:
> > > >  #ifdef CONFIG_DEBUG_VM
> > > > -             BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > > +     BUG_ON((GFP_ZONE_BAD >> bit) & 1);
> > > >  #endif
> > > > -     }
> > > >       return z;
> > > 
> > > Why don't you use VM_BUG_ON?
> > 
> > I was just trying to make a minimal patch that did a single thing.
> > 
> > Feel free to submit another one that does that.  I'm sure there are a
> > couple more places that could use similar love.
> 
> I posted another approach patches a second ago. Could you please see it?

Those both look sane to me.  Those weren't biting me in particular, and
they don't fix the issue I was seeing.  But, they do seem necessary to
reduce some of the noise.

CC'ing the sparse mailing list.  We're seeing a couple of cases where
some gcc-isms are either stopping sparse from finding real bugs:

	http://marc.info/?l=linux-mm&m=130282454732689&w=2

or creating a lot of noise on some builds:

	http://marc.info/?l=linux-mm&m=130284428614058&w=2
	http://marc.info/?l=linux-mm&m=130284431014077&w=2

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>