[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?

    > 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.



    #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))

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

    #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); \


    #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.027 / U:10.196 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site