[PATCH] Remove undefined behavior from list::push_back/front, emplace_back/front.
Marshall Clow
mclow.lists at gmail.com
Wed Mar 26 20:45:44 PDT 2014
Hi rsmith, howard.hinnant,
This is an attempt to fix http://llvm.org/bugs/show_bug.cgi?id=18488, where std::list shows undefined behavior by casting a pointer to a __list_node_base to a __list_node.
I have added two private routines to list: __link_nodes_at_front and __link_nodes_at_back, to deal with these cases.
For simplicity, I added a method __self to __list_node_base, because that expression was used all over the place.
This passes all tests for C++03/11/14, and with ASAN. However, the undefined behavior was observed only with gcc 4.7.2, which I don't have. I'll work on getting that set up.
Also, I am not 100% sure that there isn't a similar set of UB lurking in the insert* routines.
http://llvm-reviews.chandlerc.com/D3197
Files:
include/list
Index: include/list
===================================================================
--- include/list
+++ include/list
@@ -214,10 +214,13 @@
pointer __next_;
_LIBCPP_INLINE_VISIBILITY
- __list_node_base()
- : __prev_(static_cast<pointer>(pointer_traits<__base_pointer>::pointer_to(*this))),
- __next_(static_cast<pointer>(pointer_traits<__base_pointer>::pointer_to(*this)))
- {}
+ __list_node_base() : __prev_(__self()), __next_(__self()) {}
+
+ _LIBCPP_INLINE_VISIBILITY
+ pointer __self()
+ {
+ return static_cast<pointer>(pointer_traits<__base_pointer>::pointer_to(*this));
+ }
};
template <class _Tp, class _VoidPtr>
@@ -753,20 +756,14 @@
swap(__sz(), __c.__sz());
swap(__end_, __c.__end_);
if (__sz() == 0)
- __end_.__next_ = __end_.__prev_ = static_cast<__node_pointer>(
- pointer_traits<__node_base_pointer>::pointer_to(__end_));
+ __end_.__next_ = __end_.__prev_ = __end_.__self();
else
- __end_.__prev_->__next_ = __end_.__next_->__prev_
- = static_cast<__node_pointer>(
- pointer_traits<__node_base_pointer>::pointer_to(__end_));
+ __end_.__prev_->__next_ = __end_.__next_->__prev_ = __end_.__self();
if (__c.__sz() == 0)
- __c.__end_.__next_ = __c.__end_.__prev_
- = static_cast<__node_pointer>(
- pointer_traits<__node_base_pointer>::pointer_to(__c.__end_));
+ __c.__end_.__next_ = __c.__end_.__prev_ = __c.__end_.__self();
else
- __c.__end_.__prev_->__next_ = __c.__end_.__next_->__prev_
- = static_cast<__node_pointer>(
- pointer_traits<__node_base_pointer>::pointer_to(__c.__end_));
+ __c.__end_.__prev_->__next_ = __c.__end_.__next_->__prev_ = __c.__end_.__self();
+
#if _LIBCPP_DEBUG_LEVEL >= 2
__libcpp_db* __db = __get_db();
__c_node* __cn1 = __db->__find_c_and_lock(this);
@@ -1059,7 +1056,9 @@
#endif // _LIBCPP_DEBUG_LEVEL >= 2
private:
- static void __link_nodes(__node_pointer __p, __node_pointer __f, __node_pointer __l);
+ static void __link_nodes (__node_pointer __p, __node_pointer __f, __node_pointer __l);
+ void __link_nodes_at_front(__node_pointer __f, __node_pointer __l);
+ void __link_nodes_at_back (__node_pointer __f, __node_pointer __l);
iterator __iterator(size_type __n);
template <class _Comp>
static iterator __sort(iterator __f1, iterator __e2, size_type __n, _Comp& __comp);
@@ -1081,8 +1080,33 @@
__l->__next_ = __p;
}
+// Link in nodes [__f, __l] at the front of the list
template <class _Tp, class _Alloc>
inline _LIBCPP_INLINE_VISIBILITY
+void
+list<_Tp, _Alloc>::__link_nodes_at_front(__node_pointer __f, __node_pointer __l)
+{
+ __f->__prev_ = base::__end_.__self();
+ __l->__next_ = base::__end_.__next_;
+ __l->__next_->__prev_ = __l;
+ base::__end_.__next_ = __f;
+}
+
+// Link in nodes [__f, __l] at the front of the list
+template <class _Tp, class _Alloc>
+inline _LIBCPP_INLINE_VISIBILITY
+void
+list<_Tp, _Alloc>::__link_nodes_at_back(__node_pointer __f, __node_pointer __l)
+{
+ __l->__next_ = base::__end_.__self();
+ __f->__prev_ = base::__end_.__prev_;
+ __f->__prev_->__next_ = __f;
+ base::__end_.__prev_ = __l;
+}
+
+
+template <class _Tp, class _Alloc>
+inline _LIBCPP_INLINE_VISIBILITY
typename list<_Tp, _Alloc>::iterator
list<_Tp, _Alloc>::__iterator(size_type __n)
{
@@ -1502,7 +1526,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
- __link_nodes(base::__end_.__next_, __hold.get(), __hold.get());
+ __link_nodes_at_front(__hold.get(), __hold.get());
++base::__sz();
__hold.release();
}
@@ -1515,8 +1539,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), __x);
- __link_nodes(static_cast<__node_pointer>(pointer_traits<__node_base_pointer>::
- pointer_to(base::__end_)), __hold.get(), __hold.get());
+ __link_nodes_at_back(__hold.get(), __hold.get());
++base::__sz();
__hold.release();
}
@@ -1531,7 +1554,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::move(__x));
- __link_nodes(base::__end_.__next_, __hold.get(), __hold.get());
+ __link_nodes_at_front(__hold.get(), __hold.get());
++base::__sz();
__hold.release();
}
@@ -1544,8 +1567,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::move(__x));
- __link_nodes(static_cast<__node_pointer>(pointer_traits<__node_base_pointer>::
- pointer_to(base::__end_)), __hold.get(), __hold.get());
+ __link_nodes_at_back(__hold.get(), __hold.get());
++base::__sz();
__hold.release();
}
@@ -1561,7 +1583,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::forward<_Args>(__args)...);
- __link_nodes(base::__end_.__next_, __hold.get(), __hold.get());
+ __link_nodes_at_front(__hold.get(), __hold.get());
++base::__sz();
__hold.release();
}
@@ -1575,8 +1597,7 @@
typedef __allocator_destructor<__node_allocator> _Dp;
unique_ptr<__node, _Dp> __hold(__node_alloc_traits::allocate(__na, 1), _Dp(__na, 1));
__node_alloc_traits::construct(__na, _VSTD::addressof(__hold->__value_), _VSTD::forward<_Args>(__args)...);
- __link_nodes(static_cast<__node_pointer>(pointer_traits<__node_base_pointer>::
- pointer_to(base::__end_)), __hold.get(), __hold.get());
+ __link_nodes_at_back(__hold.get(), __hold.get());
++base::__sz();
__hold.release();
}
@@ -1826,8 +1847,7 @@
throw;
}
#endif // _LIBCPP_NO_EXCEPTIONS
- __link_nodes(static_cast<__node_pointer>(pointer_traits<__node_base_pointer>::
- pointer_to(base::__end_)), __r.__ptr_, __e.__ptr_);
+ __link_nodes_at_back(__r.__ptr_, __e.__ptr_);
base::__sz() += __ds;
}
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3197.1.patch
Type: text/x-patch
Size: 6819 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140326/256c0213/attachment.bin>
More information about the cfe-commits
mailing list