[PATCH] D12297: [libcxx] Fix most undefined behavior in <list>

[Eric Fiselier via cfe-commits]

EricWF created this revision.
EricWF added reviewers: mclow.lists, danalbert, rsmith.
EricWF added subscribers: cfe-commits, awi, rsmith.

This patch partially fixes the undefined behavior in `<list>`. The undefined behavior is caused by casting a pointer to `__node_base __end_` to the derived type `__node` even though `__end_` is not a sub-object of type `__node`.

See C++14 [expr.static.cast]p11

> A prvalue of type “pointer to cv1 B,” where B is a class type, can be converted to a prvalue of type “pointer to cv2 D,” where D is a class derived (Clause 10) from B, if a valid standard conversion from “pointer to D” to “pointer to B” exists (4.10), cv2 is the same cv-qualification as, or greater cv-qualification than, cv1, and B is neither a virtual base class of D nor a base class of a virtual base class of D. The null pointer value (4.10) is converted to the null pointer value of the destination type. If the prvalue of type “pointer to cv1 B” points to a B that is actually a subobject of an object of type D, the resulting pointer points to the enclosing object of type D. Otherwise, the behavior is undefined.

This patch fixes the problem by adding an intermediary void cast so that the conversion acts as if it were a reinterpret_cast. A reinterpret cast between pointer types is not used because this will not work with fancy pointers. This depends on the wording in:

C++14 [expr.reinterpret.cast]p7

> An object pointer can be explicitly converted to an object pointer of a different type. When a prvalue v of object pointer type is converted to the object pointer type “pointer to cv T”, the result is static_cast<cv T*>(static_cast<cv void*>(v)). Converting a prvalue of type “pointer to T1” to the type “pointer to T2” (where T1 and T2 are object types and where the alignment requirements of T2 are no stricter than those of T1) and back to its original type yields the original pointer value.

However, as @rsmith pointed out, this still has undefined behavior when the `element_type` stored in the list has a higher alignment requirement than that of a pointer.

I will post a patch that completely fixes this problem but that likely involves an ABI break.

http://reviews.llvm.org/D12297

Files:
  include/list

Index: include/list
===================================================================
--- include/list
+++ include/list
@@ -205,7 +205,8 @@
     _LIBCPP_INLINE_VISIBILITY
     pointer __self()
     {
-        return static_cast<pointer>(pointer_traits<__base_pointer>::pointer_to(*this));
+        return static_cast<pointer>(static_cast<_VoidPtr>(
+            pointer_traits<__base_pointer>::pointer_to(*this)));
     }
 };
 
@@ -543,22 +544,18 @@
     iterator end() _NOEXCEPT
     {
 #if _LIBCPP_DEBUG_LEVEL >= 2
-        return iterator(static_cast<__node_pointer>(
-                pointer_traits<__node_base_pointer>::pointer_to(__end_)), this);
+        return iterator(__end_.__self(), this);
 #else
-        return iterator(static_cast<__node_pointer>(
-                      pointer_traits<__node_base_pointer>::pointer_to(__end_)));
+        return iterator(__end_.__self());
 #endif
     }
     _LIBCPP_INLINE_VISIBILITY
     const_iterator end() const _NOEXCEPT
     {
 #if _LIBCPP_DEBUG_LEVEL >= 2
-        return const_iterator(static_cast<__node_const_pointer>(
-        pointer_traits<__node_base_pointer>::pointer_to(const_cast<__node_base&>(__end_))), this);
+        return const_iterator(const_cast<__node_base&>(__end_).__self(), this);
 #else
-        return const_iterator(static_cast<__node_const_pointer>(
-        pointer_traits<__node_base_pointer>::pointer_to(const_cast<__node_base&>(__end_))));
+        return const_iterator(const_cast<__node_base&>(__end_).__self());
 #endif
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D12297.33000.patch
Type: text/x-patch
Size: 1521 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150824/7bcbfc31/attachment.bin>