mirror of
https://github.com/RPCS3/llvm-mirror.git
synced 2024-10-19 11:02:59 +02:00
9ac8992a85
This removes the undefined behaviour (UB) in ilist/ilist_node/etc., mainly by removing (gutting) the ilist_sentinel_traits customization point and canonicalizing on a single, efficient memory layout. This fixes PR26753. The new ilist is a doubly-linked circular list. - ilist_node_base has two ilist_node_base*: Next and Prev. Size-of: two pointers. - ilist_node<T> (size-of: two pointers) is a type-safe wrapper around ilist_node_base. - ilist_iterator<T> (size-of: two pointers) operates on an ilist_node<T>*, and downcasts to T* on dereference. - ilist_sentinel<T> (size-of: two pointers) is a wrapper around ilist_node<T> that has some extra API for list management. - ilist<T> (size-of: two pointers) has an ilist_sentinel<T>, whose address is returned for end(). The new memory layout matches ilist_half_embedded_sentinel_traits<T> exactly. The Head pointer that previously lived in ilist<T> is effectively glued to the ilist_half_node<T> that lived in ilist_half_embedded_sentinel_traits<T>, becoming the Next and Prev in the ilist_sentinel_node<T>, respectively. sizeof(ilist<T>) is now the size of two pointers, and there is never any additional storage for a sentinel. This is a much simpler design for a doubly-linked list, removing most of the corner cases of list manipulation (add, remove, etc.). In follow-up commits, I intend to move as many algorithms as possible into a non-templated base class (ilist_base) to reduce code size. Moreover, this fixes the UB in ilist_iterator/getNext/getPrev operations. Previously, ilist_iterator<T> operated on a T*, even when the sentinel was not of type T (i.e., ilist_embedded_sentinel_traits and ilist_half_embedded_sentinel_traits). This added UB to all operations involving end(). Now, ilist_iterator<T> operates on an ilist_node<T>*, and only downcasts when the full type is guaranteed to be T*. What did we lose? There used to be a crash (in some configurations) on ++end(). Curiously (via UB), ++end() would return begin() for users of ilist_half_embedded_sentinel_traits<T>, but otherwise ++end() would cause a nice dependable nullptr dereference, crashing instead of a possible infinite loop. Options: 1. Lose that behaviour. 2. Keep it, by stealing a bit from Prev in asserts builds. 3. Crash on dereference instead, using the same technique. Hans convinced me (because of the number of problems this and r278532 exposed on Windows) that we really need some assertion here, at least in the short term. I've opted for #3 since I think it catches more bugs. I added only a couple of unit tests to root out specific bugs I hit during bring-up, but otherwise this is tested implicitly via the extensive usage throughout LLVM. Planned follow-ups: - Remove ilist_*sentinel_traits<T>. Here I've just gutted them to prevent build failures in sub-projects. Once I stop referring to them in sub-projects, I'll come back and delete them. - Add ilist_base and move algorithms there. - Check and fix move construction and assignment. Eventually, there are other interesting directions: - Rewrite reverse iterators, so that rbegin().getNodePtr()==&*rbegin(). This allows much simpler logic when erasing elements during a reverse traversal. - Remove ilist_traits::createNode, by deleting the remaining API that creates nodes. Intrusive lists shouldn't be creating nodes themselves. - Remove ilist_traits::deleteNode, by (1) asserting that lists are empty on destruction and (2) changing API that calls it to take a Deleter functor (intrusive lists shouldn't be in the memory management business). - Reconfigure the remaining callback traits (addNodeToList, etc.) to be higher-level, pulling out a simple_ilist<T> that is much easier to read and understand. - Allow tags (e.g., ilist_node<T,tag1> and ilist_node<T,tag2>) so that T can be a member of multiple intrusive lists. llvm-svn: 278974 |
||
|---|---|---|
| .. | ||
| APFloatTest.cpp | ||
| APIntTest.cpp | ||
| APSIntTest.cpp | ||
| ArrayRefTest.cpp | ||
| BitmaskEnumTest.cpp | ||
| BitVectorTest.cpp | ||
| CMakeLists.txt | ||
| DAGDeltaAlgorithmTest.cpp | ||
| DeltaAlgorithmTest.cpp | ||
| DenseMapTest.cpp | ||
| DenseSetTest.cpp | ||
| FoldingSet.cpp | ||
| FunctionRefTest.cpp | ||
| HashingTest.cpp | ||
| ilistTest.cpp | ||
| ImmutableMapTest.cpp | ||
| ImmutableSetTest.cpp | ||
| IntEqClassesTest.cpp | ||
| IntervalMapTest.cpp | ||
| IntrusiveRefCntPtrTest.cpp | ||
| MakeUniqueTest.cpp | ||
| MapVectorTest.cpp | ||
| OptionalTest.cpp | ||
| PackedVectorTest.cpp | ||
| PointerEmbeddedIntTest.cpp | ||
| PointerIntPairTest.cpp | ||
| PointerSumTypeTest.cpp | ||
| PointerUnionTest.cpp | ||
| PostOrderIteratorTest.cpp | ||
| PriorityWorklistTest.cpp | ||
| RangeAdapterTest.cpp | ||
| SCCIteratorTest.cpp | ||
| ScopeExitTest.cpp | ||
| SequenceTest.cpp | ||
| SetVectorTest.cpp | ||
| SmallPtrSetTest.cpp | ||
| SmallStringTest.cpp | ||
| SmallVectorTest.cpp | ||
| SparseBitVectorTest.cpp | ||
| SparseMultiSetTest.cpp | ||
| SparseSetTest.cpp | ||
| StringMapTest.cpp | ||
| StringRefTest.cpp | ||
| TinyPtrVectorTest.cpp | ||
| TripleTest.cpp | ||
| TwineTest.cpp | ||
| VariadicFunctionTest.cpp | ||