Fix: hlist iteration relies on undefined behavior

Comparing an offset from an object with NULL is undefined behavior
and the compiler may assume that this is never true.

This is indeed what is observed with gcc-10 miscompiling
cds_hlist_for_each_entry_rcu_2().

Fix this by introducing cds_hlist_entry_safe() rather than open-coding
the NULL check comparisons, and move cds_hlist_for_each_entry_2()
and cds_hlist_for_each_entry_safe_2() to this scheme as well.

Fixes: #1308
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ief3531cf04e54b6dae05eb28a6822adfa141fdeb

diff --git a/include/urcu/hlist.h b/include/urcu/hlist.h
index 344481133eac2051f4d7ab85e6188f51887e7dd7..5e3c27d02025bd4445efd65606f60374d8326e88 100644 (file)
--- a/include/urcu/hlist.h
+++ b/include/urcu/hlist.h
@@ -17,6 +17,7 @@
  */
 
 #include <stddef.h>
+#include <urcu/compiler.h>
 
 struct cds_hlist_head {
        struct cds_hlist_node *next;
@@ -40,8 +41,14 @@ void CDS_INIT_HLIST_HEAD(struct cds_hlist_head *ptr)
        { .next = NULL }
 
 /* Get typed element from list at a given position. */
-#define cds_hlist_entry(ptr, type, member) \
-       ((type *) ((char *) (ptr) - (unsigned long) (&((type *) 0)->member)))
+#define cds_hlist_entry(ptr, type, member)     caa_container_of(ptr, type, member)
+
+/* Get typed element from list at a given position, keeping NULL pointers. */
+#define cds_hlist_entry_safe(ptr, type, member)                                        \
+       ({                                                                      \
+               __typeof__(ptr) ____ret = ptr;                                  \
+               ____ret ? cds_hlist_entry(____ret, type, member) : NULL;        \
+       })
 
 /* Add new element at the head of the list. */
 static inline
@@ -93,17 +100,13 @@ void cds_hlist_del(struct cds_hlist_node *elem)
                        entry = cds_hlist_entry(pos, __typeof__(*entry), member))
 
 #define cds_hlist_for_each_entry_2(entry, head, member) \
-       for (entry = ((head)->next == NULL ? NULL \
-                       : cds_hlist_entry((head)->next, __typeof__(*entry), member)); \
+       for (entry = cds_hlist_entry_safe((head)->next, __typeof__(*entry), member); \
                entry != NULL; \
-               entry = (entry->member.next == NULL ? NULL \
-                       : cds_hlist_entry(entry->member.next, __typeof__(*entry), member)))
+               entry = cds_hlist_entry_safe(entry->member.next, __typeof__(*entry), member))
 
 #define cds_hlist_for_each_entry_safe_2(entry, e, head, member) \
-       for (entry = ((head)->next == NULL ? NULL \
-                       : cds_hlist_entry((head)->next, __typeof__(*entry), member)); \
-               (entry != NULL) && (e = (entry->member.next == NULL ? NULL \
-                                       : cds_hlist_entry(entry->member.next, \
+       for (entry = cds_hlist_entry_safe((head)->next, __typeof__(*entry), member); \
+               (entry != NULL) && (e = (cds_hlist_entry_safe(entry->member.next, \
                                                __typeof__(*entry), member)), 1); \
                entry = e)
 

diff --git a/include/urcu/rcuhlist.h b/include/urcu/rcuhlist.h
index 69c4d3184fd50d7f38a730e17447678f5fec3991..ca1da068b1fa6fdbe95db84dae922fc44e66bf84 100644 (file)
--- a/include/urcu/rcuhlist.h
+++ b/include/urcu/rcuhlist.h
@@ -72,10 +72,10 @@ void cds_hlist_del_rcu(struct cds_hlist_node *elem)
                        entry = cds_hlist_entry(pos, __typeof__(*entry), member))
 
 #define cds_hlist_for_each_entry_rcu_2(entry, head, member) \
-       for (entry = cds_hlist_entry(rcu_dereference((head)->next), \
+       for (entry = cds_hlist_entry_safe(rcu_dereference((head)->next), \
                        __typeof__(*entry), member); \
-               &entry->member != NULL; \
-               entry = cds_hlist_entry(rcu_dereference(entry->member.next), \
+               entry != NULL; \
+               entry = cds_hlist_entry_safe(rcu_dereference(entry->member.next), \
                        __typeof__(*entry), member))
 
 #endif /* _URCU_RCUHLIST_H */