tech-userlevel archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: in which we present an ugly hack to make sys/queue.h CIRCLEQ work
+ * this by changing the
the what? :-)
i recently moved bionic from CIRCLEQ
(https://android-review.googlesource.com/#/c/69028/4/libc/bionic/pthread-atfork.c)
to just managing its own list
(https://android-review.googlesource.com/#/c/69028/4/libc/bionic/pthread_atfork.cpp)
because you can't use the existing CIRCLEQ macros in C++.
On Wed, Nov 20, 2013 at 1:54 AM, matthew green <mrg%eterna.com.au@localhost>
wrote:
>
> hi folks.
>
>
> while preparing to update to GCC 4.8 i discovered that our sys/queue.h
> CIRCLEQ macros violate C aliasing rules, ultimately leading to the
> compiler eliding comparisons it declared as always false.
>
> unfortunately, changing these macros to be strictly C requires changing
> the ABI of them, and while we are going to consider such a change, in
> the interest of getting things working we present the following hack.
> it was inspired by David Holland, and we considered placing it in
> sys/cdefs.h for other consumers, but as queue.h currently only relies
> on the presence of "NULL" combined with the need for "<sys/queue.h>"
> to work correctly for the tools build (which may be on non-netbsd
> platforms.)
>
> the visible problems are that the libc DB routines often fail, and
> that nvi locks up all the time.
>
> i'm going to commit this soon, but if anyone has more useful hacks or
> real fixes, please let me/these lists know and we'll consider them.
>
> i'm also going to purge the tree of several copies of sys/queue.h
> present in 3rd party software as a follow change.
>
> thanks to dholland, apb, joerg, martin, matt, and skrll for this
> least-worst-so-far solution.
>
>
> .mrg.
>
>
> Index: queue.h
> ===================================================================
> RCS file: /cvsroot/src/sys/sys/queue.h,v
> retrieving revision 1.55
> diff -p -r1.55 queue.h
> --- queue.h 17 Jul 2013 15:50:59 -0000 1.55
> +++ queue.h 20 Nov 2013 09:33:42 -0000
> @@ -602,18 +602,41 @@
> /*
> * Circular queue definitions.
> */
> +
> +/*
> + * __launder_type(): We use this ugly hack to work around the the compiler
> + * noticing that two types may not alias each other and elide tests in code.
> + * We hit this in the CIRCLEQ macros when comparing 'struct name *' and
> + * 'struct type *' (see CIRCLEQ_HEAD()). Modern compilers (such as GCC
> + * 4.8) declare these comparisons as always false, causing the code to
> + * not run as designed.
> + *
> + * This hack is only to be used for comparisons and thus can be fully const.
> + * Do not use for assignment.
> + *
> + * If we ever choose to change the ABI of the CIRCLEQ macros, we could fix
> + * this by changing the
> + */
> +static inline const void * __launder_type(const void *);
> +static inline const void *
> +__launder_type(const void *__x)
> +{
> + __asm __volatile("" : "+r" (__x));
> + return __x;
> +}
> +
> #if defined(_KERNEL) && defined(QUEUEDEBUG)
> #define QUEUEDEBUG_CIRCLEQ_HEAD(head, field) \
> - if ((head)->cqh_first != (void *)(head) && \
> - (head)->cqh_first->field.cqe_prev != (void *)(head)) \
> + if ((head)->cqh_first != __launder_type(head) && \
> + (head)->cqh_first->field.cqe_prev != __launder_type(head)) \
> panic("CIRCLEQ head forw %p %s:%d", (head), \
> __FILE__, __LINE__); \
> - if ((head)->cqh_last != (void *)(head) && \
> - (head)->cqh_last->field.cqe_next != (void *)(head)) \
> + if ((head)->cqh_last != __launder_type(head) && \
> + (head)->cqh_last->field.cqe_next != __launder_type(head)) \
> panic("CIRCLEQ head back %p %s:%d", (head), \
> __FILE__, __LINE__);
> #define QUEUEDEBUG_CIRCLEQ_ELM(head, elm, field) \
> - if ((elm)->field.cqe_next == (void *)(head)) { \
> + if ((elm)->field.cqe_next == __launder_type(head)) { \
> if ((head)->cqh_last != (elm)) \
> panic("CIRCLEQ elm last %p %s:%d", (elm), \
> __FILE__, __LINE__); \
> @@ -622,7 +645,7 @@
> panic("CIRCLEQ elm forw %p %s:%d", (elm), \
> __FILE__, __LINE__); \
> } \
> - if ((elm)->field.cqe_prev == (void *)(head)) { \
> + if ((elm)->field.cqe_prev == __launder_type(head)) { \
> if ((head)->cqh_first != (elm)) \
> panic("CIRCLEQ elm first %p %s:%d", (elm), \
> __FILE__, __LINE__); \
> @@ -668,7 +691,7 @@
> QUEUEDEBUG_CIRCLEQ_ELM((head), (listelm), field) \
> (elm)->field.cqe_next = (listelm)->field.cqe_next; \
> (elm)->field.cqe_prev = (listelm); \
> - if ((listelm)->field.cqe_next == (void *)(head)) \
> + if ((listelm)->field.cqe_next == __launder_type(head)) \
> (head)->cqh_last = (elm); \
> else \
> (listelm)->field.cqe_next->field.cqe_prev = (elm); \
> @@ -680,7 +703,7 @@
> QUEUEDEBUG_CIRCLEQ_ELM((head), (listelm), field) \
> (elm)->field.cqe_next = (listelm); \
> (elm)->field.cqe_prev = (listelm)->field.cqe_prev; \
> - if ((listelm)->field.cqe_prev == (void *)(head)) \
> + if ((listelm)->field.cqe_prev == __launder_type(head)) \
> (head)->cqh_first = (elm); \
> else \
> (listelm)->field.cqe_prev->field.cqe_next = (elm); \
> @@ -691,7 +714,7 @@
> QUEUEDEBUG_CIRCLEQ_HEAD((head), field) \
> (elm)->field.cqe_next = (head)->cqh_first; \
> (elm)->field.cqe_prev = (void *)(head); \
> - if ((head)->cqh_last == (void *)(head)) \
> + if ((head)->cqh_last == __launder_type(head)) \
> (head)->cqh_last = (elm); \
> else \
> (head)->cqh_first->field.cqe_prev = (elm); \
> @@ -702,7 +725,7 @@
> QUEUEDEBUG_CIRCLEQ_HEAD((head), field) \
> (elm)->field.cqe_next = (void *)(head); \
> (elm)->field.cqe_prev = (head)->cqh_last; \
> - if ((head)->cqh_first == (void *)(head)) \
> + if ((head)->cqh_first == __launder_type(head)) \
> (head)->cqh_first = (elm); \
> else \
> (head)->cqh_last->field.cqe_next = (elm); \
> @@ -712,12 +735,12 @@
> #define CIRCLEQ_REMOVE(head, elm, field) do {
> \
> QUEUEDEBUG_CIRCLEQ_HEAD((head), field) \
> QUEUEDEBUG_CIRCLEQ_ELM((head), (elm), field) \
> - if ((elm)->field.cqe_next == (void *)(head)) \
> + if ((elm)->field.cqe_next == __launder_type(head)) \
> (head)->cqh_last = (elm)->field.cqe_prev; \
> else \
> (elm)->field.cqe_next->field.cqe_prev = \
> (elm)->field.cqe_prev; \
> - if ((elm)->field.cqe_prev == (void *)(head)) \
> + if ((elm)->field.cqe_prev == __launder_type(head)) \
> (head)->cqh_first = (elm)->field.cqe_next; \
> else \
> (elm)->field.cqe_prev->field.cqe_next = \
> @@ -727,29 +750,29 @@
>
> #define CIRCLEQ_FOREACH(var, head, field)
> \
> for ((var) = ((head)->cqh_first); \
> - (var) != (const void *)(head); \
> + (var) != (const void *)__launder_type(head); \
> (var) = ((var)->field.cqe_next))
>
> #define CIRCLEQ_FOREACH_REVERSE(var, head, field)
> \
> for ((var) = ((head)->cqh_last); \
> - (var) != (const void *)(head); \
> + (var) != (const void *)__launder_type(head); \
> (var) = ((var)->field.cqe_prev))
>
> /*
> * Circular queue access methods.
> */
> -#define CIRCLEQ_EMPTY(head) ((head)->cqh_first == (void
> *)(head))
> +#define CIRCLEQ_EMPTY(head) ((head)->cqh_first ==
> __launder_type(head))
> #define CIRCLEQ_FIRST(head) ((head)->cqh_first)
> #define CIRCLEQ_LAST(head) ((head)->cqh_last)
> #define CIRCLEQ_NEXT(elm, field) ((elm)->field.cqe_next)
> #define CIRCLEQ_PREV(elm, field) ((elm)->field.cqe_prev)
>
> #define CIRCLEQ_LOOP_NEXT(head, elm, field) \
> - (((elm)->field.cqe_next == (void *)(head)) \
> + (((elm)->field.cqe_next == __launder_type(head))
> \
> ? ((head)->cqh_first) \
> : (elm->field.cqe_next))
> #define CIRCLEQ_LOOP_PREV(head, elm, field) \
> - (((elm)->field.cqe_prev == (void *)(head)) \
> + (((elm)->field.cqe_prev == __launder_type(head))
> \
> ? ((head)->cqh_last) \
> : (elm->field.cqe_prev))
>
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Java i18n/JNI/NIO, or bionic questions? Mail me/drop by/add me as a reviewer.
Home |
Main Index |
Thread Index |
Old Index