lkml.org 
[lkml]   [2008]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Using sparse to catch invalid RCU dereferences?
From
Date

> It might be. There are a number of places where it is legal to access
> RCU-protected pointers directly, and all of these would need to be
> changed. For example, in the example above, one could do:
>
> foo = NULL;

Ok, that I understand, but sparse always treats NULL specially anyway.

> I recently tried to modify rcu_assign_pointer() to issue the memory
> memory barrier only when the pointer was non-NULL, but this ended badly.

Hm? I thought that's in the current tree.

> Probably because I am not the greatest gcc expert around... We ended
> up having to define an rcu_assign_index() to handle the possibility of
> assigning a zero-value array index, but my attempts to do type-checking
> backfired, and I eventually gave it up. Again, someone a bit more clued
> in to gcc than I am could probably pull it off.

Ah, ok.

> In addition, it is legal to omit rcu_dereference() and rcu_assign_pointer()
> when holding the update-side lock.

That I don't understand. Well, I do understand that omitting
rcu_dereference() is ok, but it seems to me that the memory and compiler
barrier in rcu_assign_pointer() is actually needed.

I've been playing a bit, see below for my play rcupdate.h and test.c
test program.

Unfortunately, sparse doesn't have the ability to declare
"__attribute__((force_bitwise)) typeof(p)" or even
"__attribute__((force)) typeof(p)" which makes this force more than
necessary and causes it to not catch when incompatible pointers are
used. gcc notices that because I only do a cast at all for sparse, but
that doesn't help, since e.g. list_for_each_entry_rcu() requires that
the correct type is returned. So without sparse supporting the latter
notation, we don't stand a chance.

Also, I wouldn't know how to declare that an array or so needs
rcu-access to the members.

johannes


rcupdate.h:

#define USE_BITWISE

#ifdef __CHECKER__
#ifdef USE_BITWISE
#define __rcu __attribute__((bitwise))
#define __force_rcu_cast(p) (*((__attribute__((force)) void **)&(p)))
// would like instead:
//#define __force_rcu_cast(p) ((__attribute__((force_bitwise)) typeof(p)) (p))
#else /* not bitwise */
#define __rcu __attribute__((address_space(3)))
#define __force_rcu_cast(p) (*((__attribute__((force)) void **)&(p)))
// would like instead:
//#define __force_rcu_cast(p) ((__attribute__((force_address_space)) typeof(p)) (p))
#endif

#else /* not checker */
#define __rcu
#define __force_rcu_cast(p) (p)
#endif

#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))

#define rcu_dereference(p) ({ \
typeof(p) _________p1 = ACCESS_ONCE(p); \
smp_read_barrier_depends(); \
__force_rcu_cast(_________p1); \
})

/**
* rcu_fetch - fetch an RCU-protected pointer in the update-locked
* critical section.
*
* This macro exists for documentation and code checking purposes.
*/
#define rcu_fetch(p) __force_rcu_cast(p);

#define rcu_assign_pointer(p, v) \
({ \
if (!__builtin_constant_p(v) || \
((v) != NULL)) \
smp_wmb(); \
__force_rcu_cast(p) = (v); \
})


test.c:

#include <stdlib.h>
#include "rcupdate.h"

/* my rcu protected variables */
static unsigned int __rcu *prot;
static unsigned int __rcu *prot_same;
static unsigned char __rcu *prot2;

// dummies
static smp_read_barrier_depends(void) {}
static smp_wmb(void) {}

int main(void)
{
unsigned int *tmp;

// no warnings from sparse due to forced cast
rcu_assign_pointer(prot, tmp);
// but gcc warns
rcu_assign_pointer(prot2, tmp);

// no warnings
rcu_assign_pointer(prot, NULL);
rcu_assign_pointer(prot2, NULL);

// no warnings
prot = NULL;
prot2 = NULL;

// no warnings from sparse due to forced cast
tmp = rcu_dereference(prot);
// but gcc warns
tmp = rcu_dereference(prot2);

/* now within locked section rcu_dereference isn't required */

// no warnings from sparse due to forced cast
tmp = rcu_fetch(prot);
// but gcc warns
tmp = rcu_fetch(prot2);

/* not caught with address_space, but is caught with bitwise */
prot = prot_same;
}

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2008-04-10 16:43    [W:0.069 / U:0.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site