* Mathieu Desnoyers ([email protected]) wrote:
> Passing an unsigned int to uatomic_sub does not honor sign extend to
> long, as we should be allowed by assume.
> 
> Fix this by introducing caa_cast_long_keep_sign(), which casts either to
> long or unsigned long depending on the signedness of the argument
> received. It is used in uatomic_sub before applying the "-" operator,
> since this operator needs to operate on the "long" type size (since sign
> extension might not be performed if the argument received is unsigned).

FYI, this fix is now committed to urcu master branch as:

commit e56d99bf2046a163875df80bab5195f38606dfde

Thanks,

Mathieu

> 
> Changelog since v1:
> - cast_long_keep_sign -> caa_cast_long_keep_sign
> 
> Signed-off-by: Mathieu Desnoyers <[email protected]>
> ---
>  tests/test_uatomic.c   |    4 +++
>  urcu/compiler.h        |    5 ++++
>  urcu/uatomic/generic.h |   59 
> ++++++++++++++++++++++++++-----------------------
>  urcu/uatomic/ppc.h     |   18 ++++++++------
>  urcu/uatomic/s390.h    |   13 +++++-----
>  urcu/uatomic/sparc64.h |    7 +++--
>  urcu/uatomic/x86.h     |   42 ++++++++++++++++++----------------
>  7 files changed, 84 insertions(+), 64 deletions(-)
> 
> Index: userspace-rcu/tests/test_uatomic.c
> ===================================================================
> --- userspace-rcu.orig/tests/test_uatomic.c
> +++ userspace-rcu/tests/test_uatomic.c
> @@ -62,6 +62,10 @@ do {                                               \
>       assert(uatomic_read(ptr) == 122);       \
>       v = uatomic_sub_return(ptr, 1);         \
>       assert(v == 121);                       \
> +     uatomic_sub(ptr, (unsigned int) 2);     \
> +     assert(uatomic_read(ptr) == 119);       \
> +     uatomic_inc(ptr);                       \
> +     uatomic_inc(ptr);                       \
>       assert(uatomic_read(ptr) == 121);       \
>       uatomic_and(ptr, 129);                  \
>       assert(uatomic_read(ptr) == 1);         \
> Index: userspace-rcu/urcu/compiler.h
> ===================================================================
> --- userspace-rcu.orig/urcu/compiler.h
> +++ userspace-rcu/urcu/compiler.h
> @@ -86,4 +86,9 @@
>  #define URCU_FORCE_CAST(type, arg)   ((type) (arg))
>  #endif
>  
> +#define caa_is_signed_type(type)     (((type) (-1)) < 0)
> +
> +#define caa_cast_long_keep_sign(v)   \
> +     (caa_is_signed_type(__typeof__(v)) ? (long) (v) : (unsigned long) (v))
> +
>  #endif /* _URCU_COMPILER_H */
> Index: userspace-rcu/urcu/uatomic/generic.h
> ===================================================================
> --- userspace-rcu.orig/urcu/uatomic/generic.h
> +++ userspace-rcu/urcu/uatomic/generic.h
> @@ -81,9 +81,10 @@ unsigned long _uatomic_cmpxchg(void *add
>  }
>  
>  
> -#define uatomic_cmpxchg(addr, old, _new)                                 \
> -     ((__typeof__(*(addr))) _uatomic_cmpxchg((addr), (unsigned long)(old),\
> -                                             (unsigned long)(_new),      \
> +#define uatomic_cmpxchg(addr, old, _new)                                   \
> +     ((__typeof__(*(addr))) _uatomic_cmpxchg((addr),                       \
> +                                             caa_cast_long_keep_sign(old), \
> +                                             caa_cast_long_keep_sign(_new),\
>                                               sizeof(*(addr))))
>  
>  
> @@ -119,8 +120,8 @@ void _uatomic_and(void *addr, unsigned l
>  
>  #define uatomic_and(addr, v)                 \
>       (_uatomic_and((addr),                   \
> -                   (unsigned long)(v),       \
> -                   sizeof(*(addr))))
> +             caa_cast_long_keep_sign(v),     \
> +             sizeof(*(addr))))
>  #endif
>  
>  /* uatomic_or */
> @@ -156,8 +157,8 @@ void _uatomic_or(void *addr, unsigned lo
>  
>  #define uatomic_or(addr, v)                  \
>       (_uatomic_or((addr),                    \
> -                  (unsigned long)(v),        \
> -                  sizeof(*(addr))))
> +             caa_cast_long_keep_sign(v),     \
> +             sizeof(*(addr))))
>  #endif
>  
>  /* uatomic_add_return */
> @@ -188,10 +189,10 @@ unsigned long _uatomic_add_return(void *
>  }
>  
>  
> -#define uatomic_add_return(addr, v)                                  \
> -     ((__typeof__(*(addr))) _uatomic_add_return((addr),              \
> -                                               (unsigned long)(v),   \
> -                                               sizeof(*(addr))))
> +#define uatomic_add_return(addr, v)                                      \
> +     ((__typeof__(*(addr))) _uatomic_add_return((addr),                  \
> +                                             caa_cast_long_keep_sign(v), \
> +                                             sizeof(*(addr))))
>  #endif /* #ifndef uatomic_add_return */
>  
>  #ifndef uatomic_xchg
> @@ -253,7 +254,8 @@ unsigned long _uatomic_exchange(void *ad
>  }
>  
>  #define uatomic_xchg(addr, v)                                                
>     \
> -     ((__typeof__(*(addr))) _uatomic_exchange((addr), (unsigned long)(v), \
> +     ((__typeof__(*(addr))) _uatomic_exchange((addr),                    \
> +                                             caa_cast_long_keep_sign(v), \
>                                               sizeof(*(addr))))
>  #endif /* #ifndef uatomic_xchg */
>  
> @@ -322,10 +324,10 @@ void _uatomic_and(void *addr, unsigned l
>       _uatomic_link_error();
>  }
>  
> -#define uatomic_and(addr, v)         \
> -     (_uatomic_and((addr),           \
> -                 (unsigned long)(v), \
> -                 sizeof(*(addr))))
> +#define uatomic_and(addr, v)                 \
> +     (_uatomic_and((addr),                   \
> +             caa_cast_long_keep_sign(v),     \
> +             sizeof(*(addr))))
>  #endif /* #ifndef uatomic_and */
>  
>  #ifndef uatomic_or
> @@ -393,10 +395,10 @@ void _uatomic_or(void *addr, unsigned lo
>       _uatomic_link_error();
>  }
>  
> -#define uatomic_or(addr, v)          \
> -     (_uatomic_or((addr),            \
> -                  (unsigned long)(v),\
> -                  sizeof(*(addr))))
> +#define uatomic_or(addr, v)                  \
> +     (_uatomic_or((addr),                    \
> +             caa_cast_long_keep_sign(v),     \
> +             sizeof(*(addr))))
>  #endif /* #ifndef uatomic_or */
>  
>  #ifndef uatomic_add_return
> @@ -469,10 +471,10 @@ unsigned long _uatomic_add_return(void *
>       return 0;
>  }
>  
> -#define uatomic_add_return(addr, v)                                  \
> -     ((__typeof__(*(addr))) _uatomic_add_return((addr),              \
> -                                               (unsigned long)(v),   \
> -                                               sizeof(*(addr))))
> +#define uatomic_add_return(addr, v)                                      \
> +     ((__typeof__(*(addr))) _uatomic_add_return((addr),                  \
> +                                             caa_cast_long_keep_sign(v), \
> +                                             sizeof(*(addr))))
>  #endif /* #ifndef uatomic_add_return */
>  
>  #ifndef uatomic_xchg
> @@ -546,7 +548,8 @@ unsigned long _uatomic_exchange(void *ad
>  }
>  
>  #define uatomic_xchg(addr, v)                                                
>     \
> -     ((__typeof__(*(addr))) _uatomic_exchange((addr), (unsigned long)(v), \
> +     ((__typeof__(*(addr))) _uatomic_exchange((addr),                    \
> +                                             caa_cast_long_keep_sign(v), \
>                                               sizeof(*(addr))))
>  #endif /* #ifndef uatomic_xchg */
>  
> @@ -558,8 +561,10 @@ unsigned long _uatomic_exchange(void *ad
>  #define uatomic_add(addr, v)         (void)uatomic_add_return((addr), (v))
>  #endif
>  
> -#define uatomic_sub_return(addr, v)  uatomic_add_return((addr), -(v))
> -#define uatomic_sub(addr, v)         uatomic_add((addr), -(v))
> +#define uatomic_sub_return(addr, v)  \
> +     uatomic_add_return((addr), -(caa_cast_long_keep_sign(v)))
> +#define uatomic_sub(addr, v)         \
> +     uatomic_add((addr), -(caa_cast_long_keep_sign(v)))
>  
>  #ifndef uatomic_inc
>  #define uatomic_inc(addr)            uatomic_add((addr), 1)
> Index: userspace-rcu/urcu/uatomic/ppc.h
> ===================================================================
> --- userspace-rcu.orig/urcu/uatomic/ppc.h
> +++ userspace-rcu/urcu/uatomic/ppc.h
> @@ -100,7 +100,8 @@ unsigned long _uatomic_exchange(void *ad
>  }
>  
>  #define uatomic_xchg(addr, v)                                                
>     \
> -     ((__typeof__(*(addr))) _uatomic_exchange((addr), (unsigned long)(v), \
> +     ((__typeof__(*(addr))) _uatomic_exchange((addr),                    \
> +                                             caa_cast_long_keep_sign(v), \
>                                               sizeof(*(addr))))
>  /* cmpxchg */
>  
> @@ -159,9 +160,10 @@ unsigned long _uatomic_cmpxchg(void *add
>  }
>  
>  
> -#define uatomic_cmpxchg(addr, old, _new)                                 \
> -     ((__typeof__(*(addr))) _uatomic_cmpxchg((addr), (unsigned long)(old),\
> -                                             (unsigned long)(_new),      \
> +#define uatomic_cmpxchg(addr, old, _new)                                   \
> +     ((__typeof__(*(addr))) _uatomic_cmpxchg((addr),                       \
> +                                             caa_cast_long_keep_sign(old), \
> +                                             caa_cast_long_keep_sign(_new),\
>                                               sizeof(*(addr))))
>  
>  /* uatomic_add_return */
> @@ -215,10 +217,10 @@ unsigned long _uatomic_add_return(void *
>  }
>  
>  
> -#define uatomic_add_return(addr, v)                                  \
> -     ((__typeof__(*(addr))) _uatomic_add_return((addr),              \
> -                                               (unsigned long)(v),   \
> -                                               sizeof(*(addr))))
> +#define uatomic_add_return(addr, v)                                      \
> +     ((__typeof__(*(addr))) _uatomic_add_return((addr),                  \
> +                                             caa_cast_long_keep_sign(v), \
> +                                             sizeof(*(addr))))
>  
>  #ifdef __cplusplus 
>  }
> Index: userspace-rcu/urcu/uatomic/s390.h
> ===================================================================
> --- userspace-rcu.orig/urcu/uatomic/s390.h
> +++ userspace-rcu/urcu/uatomic/s390.h
> @@ -106,8 +106,9 @@ unsigned long _uatomic_exchange(volatile
>  }
>  
>  #define uatomic_xchg(addr, v)                                                
>     \
> -     (__typeof__(*(addr))) _uatomic_exchange((addr), (unsigned long)(v), \
> -                                            sizeof(*(addr)))
> +     (__typeof__(*(addr))) _uatomic_exchange((addr),                     \
> +                                             caa_cast_long_keep_sign(v), \
> +                                             sizeof(*(addr)))
>  
>  /* cmpxchg */
>  
> @@ -145,10 +146,10 @@ unsigned long _uatomic_cmpxchg(void *add
>       return 0;
>  }
>  
> -#define uatomic_cmpxchg(addr, old, _new)                             \
> -     (__typeof__(*(addr))) _uatomic_cmpxchg((addr),                  \
> -                                            (unsigned long)(old),    \
> -                                            (unsigned long)(_new),   \
> +#define uatomic_cmpxchg(addr, old, _new)                                  \
> +     (__typeof__(*(addr))) _uatomic_cmpxchg((addr),                       \
> +                                            caa_cast_long_keep_sign(old), \
> +                                            caa_cast_long_keep_sign(_new),\
>                                              sizeof(*(addr)))
>  
>  #ifdef __cplusplus 
> Index: userspace-rcu/urcu/uatomic/sparc64.h
> ===================================================================
> --- userspace-rcu.orig/urcu/uatomic/sparc64.h
> +++ userspace-rcu/urcu/uatomic/sparc64.h
> @@ -66,9 +66,10 @@ unsigned long _uatomic_cmpxchg(void *add
>  }
>  
>  
> -#define uatomic_cmpxchg(addr, old, _new)                                 \
> -     ((__typeof__(*(addr))) _uatomic_cmpxchg((addr), (unsigned long)(old),\
> -                                             (unsigned long)(_new),      \
> +#define uatomic_cmpxchg(addr, old, _new)                                    \
> +     ((__typeof__(*(addr))) _uatomic_cmpxchg((addr),                        \
> +                                             caa_cast_long_keep_sign(old),  \
> +                                             caa_cast_long_keep_sign(_new), \
>                                               sizeof(*(addr))))
>  
>  #ifdef __cplusplus 
> Index: userspace-rcu/urcu/uatomic/x86.h
> ===================================================================
> --- userspace-rcu.orig/urcu/uatomic/x86.h
> +++ userspace-rcu/urcu/uatomic/x86.h
> @@ -102,8 +102,9 @@ unsigned long __uatomic_cmpxchg(void *ad
>  }
>  
>  #define _uatomic_cmpxchg(addr, old, _new)                                  \
> -     ((__typeof__(*(addr))) __uatomic_cmpxchg((addr), (unsigned long)(old),\
> -                                             (unsigned long)(_new),        \
> +     ((__typeof__(*(addr))) __uatomic_cmpxchg((addr),                      \
> +                                             caa_cast_long_keep_sign(old), \
> +                                             caa_cast_long_keep_sign(_new),\
>                                               sizeof(*(addr))))
>  
>  /* xchg */
> @@ -163,7 +164,8 @@ unsigned long __uatomic_exchange(void *a
>  }
>  
>  #define _uatomic_xchg(addr, v)                                               
>       \
> -     ((__typeof__(*(addr))) __uatomic_exchange((addr), (unsigned long)(v), \
> +     ((__typeof__(*(addr))) __uatomic_exchange((addr),                     \
> +                                             caa_cast_long_keep_sign(v),   \
>                                               sizeof(*(addr))))
>  
>  /* uatomic_add_return */
> @@ -226,10 +228,10 @@ unsigned long __uatomic_add_return(void
>       return 0;
>  }
>  
> -#define _uatomic_add_return(addr, v)                                 \
> -     ((__typeof__(*(addr))) __uatomic_add_return((addr),             \
> -                                               (unsigned long)(v),   \
> -                                               sizeof(*(addr))))
> +#define _uatomic_add_return(addr, v)                                     \
> +     ((__typeof__(*(addr))) __uatomic_add_return((addr),                 \
> +                                             caa_cast_long_keep_sign(v), \
> +                                             sizeof(*(addr))))
>  
>  /* uatomic_and */
>  
> @@ -283,7 +285,7 @@ void __uatomic_and(void *addr, unsigned
>  }
>  
>  #define _uatomic_and(addr, v)                                                
>    \
> -     (__uatomic_and((addr), (unsigned long)(v), sizeof(*(addr))))
> +     (__uatomic_and((addr), caa_cast_long_keep_sign(v), sizeof(*(addr))))
>  
>  /* uatomic_or */
>  
> @@ -337,7 +339,7 @@ void __uatomic_or(void *addr, unsigned l
>  }
>  
>  #define _uatomic_or(addr, v)                                            \
> -     (__uatomic_or((addr), (unsigned long)(v), sizeof(*(addr))))
> +     (__uatomic_or((addr), caa_cast_long_keep_sign(v), sizeof(*(addr))))
>  
>  /* uatomic_add */
>  
> @@ -391,7 +393,7 @@ void __uatomic_add(void *addr, unsigned
>  }
>  
>  #define _uatomic_add(addr, v)                                                
>    \
> -     (__uatomic_add((addr), (unsigned long)(v), sizeof(*(addr))))
> +     (__uatomic_add((addr), caa_cast_long_keep_sign(v), sizeof(*(addr))))
>  
>  
>  /* uatomic_inc */
> @@ -517,7 +519,7 @@ extern unsigned long _compat_uatomic_set
>                                        unsigned long _new, int len);
>  #define compat_uatomic_set(addr, _new)                                       
>        \
>       ((__typeof__(*(addr))) _compat_uatomic_set((addr),                     \
> -                                             (unsigned long)(_new),         \
> +                                             caa_cast_long_keep_sign(_new), \
>                                               sizeof(*(addr))))
>  
>  
> @@ -525,35 +527,35 @@ extern unsigned long _compat_uatomic_xch
>                                         unsigned long _new, int len);
>  #define compat_uatomic_xchg(addr, _new)                                      
>        \
>       ((__typeof__(*(addr))) _compat_uatomic_xchg((addr),                    \
> -                                             (unsigned long)(_new),         \
> +                                             caa_cast_long_keep_sign(_new), \
>                                               sizeof(*(addr))))
>  
>  extern unsigned long _compat_uatomic_cmpxchg(void *addr, unsigned long old,
>                                            unsigned long _new, int len);
>  #define compat_uatomic_cmpxchg(addr, old, _new)                              
>        \
>       ((__typeof__(*(addr))) _compat_uatomic_cmpxchg((addr),                 \
> -                                             (unsigned long)(old),          \
> -                                             (unsigned long)(_new),         \
> +                                             caa_cast_long_keep_sign(old),  \
> +                                             caa_cast_long_keep_sign(_new), \
>                                               sizeof(*(addr))))
>  
>  extern void _compat_uatomic_and(void *addr, unsigned long _new, int len);
>  #define compat_uatomic_and(addr, v)                                 \
>       (_compat_uatomic_and((addr),                                   \
> -                     (unsigned long)(v),                            \
> +                     caa_cast_long_keep_sign(v),                    \
>                       sizeof(*(addr))))
>  
>  extern void _compat_uatomic_or(void *addr, unsigned long _new, int len);
>  #define compat_uatomic_or(addr, v)                                  \
>       (_compat_uatomic_or((addr),                                    \
> -                       (unsigned long)(v),                          \
> +                       caa_cast_long_keep_sign(v),                  \
>                         sizeof(*(addr))))
>  
>  extern unsigned long _compat_uatomic_add_return(void *addr,
>                                               unsigned long _new, int len);
> -#define compat_uatomic_add_return(addr, v)                          \
> -     ((__typeof__(*(addr))) _compat_uatomic_add_return((addr),      \
> -                                                (unsigned long)(v), \
> -                                                sizeof(*(addr))))
> +#define compat_uatomic_add_return(addr, v)                               \
> +     ((__typeof__(*(addr))) _compat_uatomic_add_return((addr),           \
> +                                             caa_cast_long_keep_sign(v), \
> +                                             sizeof(*(addr))))
>  
>  #define compat_uatomic_add(addr, v)                                         \
>               ((void)compat_uatomic_add_return((addr), (v)))
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
> 
> _______________________________________________
> lttng-dev mailing list
> [email protected]
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to