forked from OSchip/llvm-project
64093a35ff
This reverts commit r279053, reapplying r278974 after fixing PR29035 with r279104. Note that r279312 has been committed in the meantime, and this has been rebased on top of that. Otherwise it's identical to r278974. Note for maintainers of out-of-tree code (that I missed in the original message): if the new isKnownSentinel() assertion is firing from ilist_iterator<>::operator*(), this patch has identified a bug in your code. There are a few common patterns: - Some IR-related APIs htake an IRUnit* that might be nullptr, and pass in an incremented iterator as an insertion point. Some old code was using "&*++I", which in the case of end() only worked by fluke. If the IRUnit in question inherits from ilist_node_with_parent<>, you can use "I->getNextNode()". Otherwise, use "List.getNextNode(*I)". - In most other cases, crashes on &*I just need to check for I==end() before dereferencing. - There's also occasional code that sends iterators into a function, and then starts calling I->getOperand() (or other API). Either check for end() before the entering the function, or early exit. Note for if the static_assert with HasObsoleteCustomization is firing for you: - r278513 has examples of how to stop using custom sentinel traits. - r278532 removed ilist_nextprev_traits since no one was using it. See lld's r278469 for the only migration I needed to do. Original commit message follows. ---- 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: 279314 |
||
|---|---|---|
| .. | ||
| APFloatTest.cpp | ||
| APIntTest.cpp | ||
| APSIntTest.cpp | ||
| ArrayRefTest.cpp | ||
| BitVectorTest.cpp | ||
| BitmaskEnumTest.cpp | ||
| CMakeLists.txt | ||
| DAGDeltaAlgorithmTest.cpp | ||
| DeltaAlgorithmTest.cpp | ||
| DenseMapTest.cpp | ||
| DenseSetTest.cpp | ||
| FoldingSet.cpp | ||
| FunctionRefTest.cpp | ||
| HashingTest.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 | ||
| STLExtrasTest.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 | ||
| ilistTest.cpp | ||