sparse breakage triggered by rcu_read_lock() lockdep annotations

sparse breakage triggered by rcu_read_lock() lockdep annotations

[Alexey Dobriyan]

FWIW, commit 851a67b825540a8e00c0be3ee25e4627ba8b133b
aka "lockdep: annotate rcu_read_{,un}lock{,_bh}"
causes sparse to trigger internal assertion in quite a few places over
allyesconfig run.

	sparse: flow.c:805: rewrite_parent_branch: Assertion `changed' failed.

Trimmed down testcase:

	void f(unsigned long ip);
	static void g(void)
	{
	        if (1) {
	              f(({ __label__ x; x: (unsigned long)&&x; }));
	        }
	        f(({ __label__ x; x: (unsigned long)&&x; }));
	}

#0  0x4001c410 in __kernel_vsyscall ()
(gdb) bt
#0  0x4001c410 in __kernel_vsyscall ()
#1  0x40050701 in raise () from /lib/libc.so.6
#2  0x40051e38 in abort () from /lib/libc.so.6
#3  0x40049fcc in __assert_fail () from /lib/libc.so.6
#4  0x08064947 in pack_basic_blocks (ep=0x411a1c6c) at flow.c:812
#5  0x0805ffbf in linearize_symbol (sym=0x4103ec8c) at linearize.c:2154
#6  0x080492a3 in main (argc=Cannot access memory at address 0x274d) at sparse.c:266

Re: sparse breakage triggered by rcu_read_lock() lockdep annotations

[Josh Triplett]

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

Alexey Dobriyan wrote:
> FWIW, commit 851a67b825540a8e00c0be3ee25e4627ba8b133b
> aka "lockdep: annotate rcu_read_{,un}lock{,_bh}"
> causes sparse to trigger internal assertion in quite a few places over
> allyesconfig run.
> 
> 	sparse: flow.c:805: rewrite_parent_branch: Assertion `changed' failed.
> 
> Trimmed down testcase:
> 
> 	void f(unsigned long ip);
> 	static void g(void)
> 	{
> 	        if (1) {
> 	              f(({ __label__ x; x: (unsigned long)&&x; }));
> 	        }
> 	        f(({ __label__ x; x: (unsigned long)&&x; }));
> 	}
> 
> #0  0x4001c410 in __kernel_vsyscall ()
> (gdb) bt
> #0  0x4001c410 in __kernel_vsyscall ()
> #1  0x40050701 in raise () from /lib/libc.so.6
> #2  0x40051e38 in abort () from /lib/libc.so.6
> #3  0x40049fcc in __assert_fail () from /lib/libc.so.6
> #4  0x08064947 in pack_basic_blocks (ep=0x411a1c6c) at flow.c:812
> #5  0x0805ffbf in linearize_symbol (sym=0x4103ec8c) at linearize.c:2154
> #6  0x080492a3 in main (argc=Cannot access memory at address 0x274d) at sparse.c:266

Thanks for the detailed report.  Looking into it now.

- Josh Triplett



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: sparse breakage triggered by rcu_read_lock() lockdep annotations

[Chris Li]

Err,

Sparse does not support the local label syntax yet. It just treats the
second label "x:" as the same as the first one. Then the linearize
code gets serious confused when it saw one label get define in two
places.

The fix seems not trivial from the first look.

Chris

On 10/16/07, Alexey Dobriyan <adobriyan@sw.ru> wrote:
> FWIW, commit 851a67b825540a8e00c0be3ee25e4627ba8b133b
> aka "lockdep: annotate rcu_read_{,un}lock{,_bh}"
> causes sparse to trigger internal assertion in quite a few places over
> allyesconfig run.
>
>         sparse: flow.c:805: rewrite_parent_branch: Assertion `changed' failed.
>
> Trimmed down testcase:
>
>         void f(unsigned long ip);
>         static void g(void)
>         {
>                 if (1) {
>                       f(({ __label__ x; x: (unsigned long)&&x; }));
>                 }
>                 f(({ __label__ x; x: (unsigned long)&&x; }));
>         }
>
> #0  0x4001c410 in __kernel_vsyscall ()
> (gdb) bt
> #0  0x4001c410 in __kernel_vsyscall ()
> #1  0x40050701 in raise () from /lib/libc.so.6
> #2  0x40051e38 in abort () from /lib/libc.so.6
> #3  0x40049fcc in __assert_fail () from /lib/libc.so.6
> #4  0x08064947 in pack_basic_blocks (ep=0x411a1c6c) at flow.c:812
> #5  0x0805ffbf in linearize_symbol (sym=0x4103ec8c) at linearize.c:2154
> #6  0x080492a3 in main (argc=Cannot access memory at address 0x274d) at sparse.c:266
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH] Re: sparse breakage triggered by rcu_read_lock() lockdep annotations

[Christopher Li]

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

OK, I get a trivial fix after all. The test case is fixed now.
I haven't done much test otherwise.

See the patch attached.

Chris


On 10/19/07, Chris Li <christ.li@gmail.com> wrote:
> Err,
>
> Sparse does not support the local label syntax yet. It just treats the
> second label "x:" as the same as the first one. Then the linearize
> code gets serious confused when it saw one label get define in two
> places.
>
> The fix seems not trivial from the first look.
>
> Chris
>
> On 10/16/07, Alexey Dobriyan <adobriyan@sw.ru> wrote:
> > FWIW, commit 851a67b825540a8e00c0be3ee25e4627ba8b133b
> > aka "lockdep: annotate rcu_read_{,un}lock{,_bh}"
> > causes sparse to trigger internal assertion in quite a few places over
> > allyesconfig run.
> >
> >         sparse: flow.c:805: rewrite_parent_branch: Assertion `changed' failed.
> >
> > Trimmed down testcase:
> >
> >         void f(unsigned long ip);
> >         static void g(void)
> >         {
> >                 if (1) {
> >                       f(({ __label__ x; x: (unsigned long)&&x; }));
> >                 }
> >                 f(({ __label__ x; x: (unsigned long)&&x; }));
> >         }
> >
> > #0  0x4001c410 in __kernel_vsyscall ()
> > (gdb) bt
> > #0  0x4001c410 in __kernel_vsyscall ()
> > #1  0x40050701 in raise () from /lib/libc.so.6
> > #2  0x40051e38 in abort () from /lib/libc.so.6
> > #3  0x40049fcc in __assert_fail () from /lib/libc.so.6
> > #4  0x08064947 in pack_basic_blocks (ep=0x411a1c6c) at flow.c:812
> > #5  0x0805ffbf in linearize_symbol (sym=0x4103ec8c) at linearize.c:2154
> > #6  0x080492a3 in main (argc=Cannot access memory at address 0x274d) at sparse.c:266
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
>

[-- Attachment #2: local-label --]
[-- Type: application/octet-stream, Size: 1678 bytes --]

Perform local label lookup

This patch fix the sparse breakage triggered by
rcu_read_lock() lockdep annotations.

Now sparse look up the local label in symbol node
name space as well, just like looking up a normal
symbol node. Now a lable symbol can be both
type SYM_LABEL or SYM_NODE with MOD_LABEL.

Singed-Off-By: Christopher Li <sparse@chrisli.org>

Index: sparse/parse.c
===================================================================
--- sparse.orig/parse.c	2007-10-19 13:52:37.000000000 -0700
+++ sparse/parse.c	2007-10-19 13:55:23.000000000 -0700
@@ -459,6 +459,16 @@ static struct symbol *lookup_or_create_s
 	return sym;
 }
 
+static struct symbol * local_label(struct token *token)
+{
+	struct symbol *sym = lookup_symbol(token->ident, NS_SYMBOL);
+
+	if (sym && sym->ctype.modifiers & MOD_LABEL)
+		return sym;
+
+	return NULL;
+}
+
 /*
  * NOTE! NS_LABEL is not just a different namespace,
  * it also ends up using function scope instead of the
@@ -466,6 +476,9 @@ static struct symbol *lookup_or_create_s
  */
 struct symbol *label_symbol(struct token *token)
 {
+	struct symbol *sym = local_label(token);
+	if (sym)
+		return sym;
 	return lookup_or_create_symbol(NS_LABEL, SYM_LABEL, token);
 }
 
Index: sparse/validation/local-label.c
===================================================================
--- sparse.orig/validation/local-label.c	2007-10-19 13:52:40.000000000 -0700
+++ sparse/validation/local-label.c	2007-10-19 13:52:40.000000000 -0700
@@ -0,0 +1,8 @@
+void f(unsigned long ip);
+static void g(void)
+{
+       if (1) {
+	     f(({ __label__ x; x: (unsigned long)&&x; }));
+       }
+       f(({ __label__ x; x: (unsigned long)&&x; }));
+}

Re: [PATCH] Re: sparse breakage triggered by rcu_read_lock() lockdep annotations

[Josh Triplett]

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

Christopher Li wrote:
> OK, I get a trivial fix after all. The test case is fixed now.
> I haven't done much test otherwise.
> 
> See the patch attached.

Nice work Chris!  Patch applied and pushed out.

I may roll an 0.4.1 release in the near future to fix kernel
builds with Sparse.

- Josh Triplett



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]