tsan: add intrusive doubly-linked list

[dvyukov]

Summary

Add intrusive doubly-linked list container template, IList.
It will be used in the new tsan runtime.

[melver]

compiler-rt/lib/tsan/rtl/tsan_ilist.h

20

I thought about if some of it could be made nicer/safer, but don't think it can.

In theory, this could be nicer if you only ever needed to add an element to 1 list, which would allow doing template <typename Node> class INode and requiring the containing class Foo to inherit from INode<Foo>.

But looking at the rest of tsanv3 changes, it seems you need the ability to add an INode more than once since an element might be added to more than 1 list.

While not the prettiest, I think it requires INode* to be part of template arguments of IList, and no other way to do better.

It would be nice to document this requirement, because it makes it clearer why you came up with this design.

176

reinterpret_cast<Base*> instead of C-style (Base*) ?

[dvyukov]

added comment re template arguments
replaced C casts with C++ casts

[dvyukov]

compiler-rt/lib/tsan/rtl/tsan_ilist.h

20

Added an explanatory comment.
In practice it's not that bad, because the verbose template arguments are specified only once on the list type declarations.

[NeHuang]

Hi @dvyukov, this patch was causing a failure on clang-ppc64le-rhel bot https://lab.llvm.org/buildbot/#/builders/57/builds/8932 with a unmatching type issue - line 30 EXPECT_EQ(list.Size(), 0); in compiler-rt/lib/tsan/tests/unit/tsan_ilist_test.cpp.
To unblock our bot, I have committed a fix ac2ffdef9cc8d152801ca87d81736fb1dec9cb88. The latest build passed at https://lab.llvm.org/buildbot/#/builders/57/builds/8971 If you want a different fix, feel free to commit a follow-up fix patch. Thanks.

[dvyukov]

In D107050#2915501, @NeHuang wrote:

Hi @dvyukov, this patch was causing a failure on clang-ppc64le-rhel bot https://lab.llvm.org/buildbot/#/builders/57/builds/8932 with a unmatching type issue - line 30 EXPECT_EQ(list.Size(), 0); in compiler-rt/lib/tsan/tests/unit/tsan_ilist_test.cpp.
To unblock our bot, I have committed a fix ac2ffdef9cc8d152801ca87d81736fb1dec9cb88. The latest build passed at https://lab.llvm.org/buildbot/#/builders/57/builds/8971 If you want a different fix, feel free to commit a follow-up fix patch. Thanks.

Thanks, Victor! That's a fine fix.

compiler-rt/lib/tsan/CMakeLists.txt

 set(TSAN_HEADERS
   rtl/tsan_clock.h
   rtl/tsan_defs.h
   rtl/tsan_dense_alloc.h
   rtl/tsan_fd.h
   rtl/tsan_flags.h
   rtl/tsan_flags.inc
   rtl/tsan_ignoreset.h
+  rtl/tsan_ilist.h
   rtl/tsan_interceptors.h
   rtl/tsan_interface.h
   rtl/tsan_interface_ann.h
   rtl/tsan_interface_inl.h
   rtl/tsan_interface_java.h
   rtl/tsan_mman.h
   rtl/tsan_mutexset.h
   rtl/tsan_platform.h

compiler-rt/lib/tsan/rtl/tsan_ilist.h

+//===-- tsan_ilist.h --------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of ThreadSanitizer (TSan), a race detector.
+//
+//===----------------------------------------------------------------------===//
+#ifndef TSAN_ILIST_H
+#define TSAN_ILIST_H
+
+#include "sanitizer_common/sanitizer_internal_defs.h"
+
+namespace __tsan {
+
+class INode {
+ public:
+  INode() = default;
+
+ private:
+  INode* next_ = nullptr;
+  INode* prev_ = nullptr;
+
+  template <typename Base, INode Base::*Node, typename Elem>
+  friend class IList;
+  INode(const INode&) = delete;
+  void operator=(const INode&) = delete;
+};
+
+// Intrusive doubly-linked list.
+//
+// The node class (MyNode) needs to include "INode foo" field,
+// then the list can be declared as IList<MyNode, &MyNode::foo>.
+// This design allows to link MyNode into multiple lists using
+// different INode fields.
+// The optional Elem template argument allows to specify node MDT
+// (most derived type) if it's different from MyNode.
+template <typename Base, INode Base::*Node, typename Elem = Base>
+class IList {
+ public:
+  IList();
+
+  void PushFront(Elem* e);
+  void PushBack(Elem* e);
+  void Remove(Elem* e);
+
+  Elem* PopFront();
+  Elem* PopBack();
+  Elem* Front();
+  Elem* Back();
+
+  // Prev links point towards front of the queue.
+  Elem* Prev(Elem* e);
+  // Next links point towards back of the queue.
+  Elem* Next(Elem* e);
+
+  uptr Size() const;
+  bool Empty() const;
+  bool Queued(Elem* e) const;
+
+ private:
+  INode node_;
+  uptr size_ = 0;
+
+  void Push(Elem* e, INode* after);
+  static INode* ToNode(Elem* e);
+  static Elem* ToElem(INode* n);
+
+  IList(const IList&) = delete;
+  void operator=(const IList&) = delete;
+};
+
+template <typename Base, INode Base::*Node, typename Elem>
+IList<Base, Node, Elem>::IList() {
+  node_.next_ = node_.prev_ = &node_;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+void IList<Base, Node, Elem>::PushFront(Elem* e) {
+  Push(e, &node_);
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+void IList<Base, Node, Elem>::PushBack(Elem* e) {
+  Push(e, node_.prev_);
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+void IList<Base, Node, Elem>::Push(Elem* e, INode* after) {
+  INode* n = ToNode(e);
+  DCHECK_EQ(n->next_, nullptr);
+  DCHECK_EQ(n->prev_, nullptr);
+  INode* next = after->next_;
+  n->next_ = next;
+  n->prev_ = after;
+  next->prev_ = n;
+  after->next_ = n;
+  size_++;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+void IList<Base, Node, Elem>::Remove(Elem* e) {
+  INode* n = ToNode(e);
+  INode* next = n->next_;
+  INode* prev = n->prev_;
+  DCHECK(next);
+  DCHECK(prev);
+  DCHECK(size_);
+  next->prev_ = prev;
+  prev->next_ = next;
+  n->prev_ = n->next_ = nullptr;
+  size_--;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+Elem* IList<Base, Node, Elem>::PopFront() {
+  Elem* e = Front();
+  if (e)
+    Remove(e);
+  return e;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+Elem* IList<Base, Node, Elem>::PopBack() {
+  Elem* e = Back();
+  if (e)
+    Remove(e);
+  return e;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+Elem* IList<Base, Node, Elem>::Front() {
+  return size_ ? ToElem(node_.next_) : nullptr;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+Elem* IList<Base, Node, Elem>::Back() {
+  return size_ ? ToElem(node_.prev_) : nullptr;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+Elem* IList<Base, Node, Elem>::Prev(Elem* e) {
+  INode* n = ToNode(e);
+  DCHECK(n->prev_);
+  return n->prev_ != &node_ ? ToElem(n->prev_) : nullptr;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+Elem* IList<Base, Node, Elem>::Next(Elem* e) {
+  INode* n = ToNode(e);
+  DCHECK(n->next_);
+  return n->next_ != &node_ ? ToElem(n->next_) : nullptr;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+uptr IList<Base, Node, Elem>::Size() const {
+  return size_;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+bool IList<Base, Node, Elem>::Empty() const {
+  return size_ == 0;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+bool IList<Base, Node, Elem>::Queued(Elem* e) const {
+  INode* n = ToNode(e);
+  DCHECK_EQ(!n->next_, !n->prev_);
+  return n->next_;
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+INode* IList<Base, Node, Elem>::ToNode(Elem* e) {
+  return &(e->*Node);
+}
+
+template <typename Base, INode Base::*Node, typename Elem>
+Elem* IList<Base, Node, Elem>::ToElem(INode* n) {
+  return static_cast<Elem*>(reinterpret_cast<Base*>(
+      reinterpret_cast<uptr>(n) -
+      reinterpret_cast<uptr>(&(reinterpret_cast<Elem*>(0)->*Node))));
+}
+
+}  // namespace __tsan
+
+#endif

compiler-rt/lib/tsan/tests/unit/CMakeLists.txt

 set(TSAN_UNIT_TEST_SOURCES
   tsan_clock_test.cpp
   tsan_dense_alloc_test.cpp
   tsan_flags_test.cpp
+  tsan_ilist_test.cpp
   tsan_mman_test.cpp
   tsan_shadow_test.cpp
   tsan_stack_test.cpp
   tsan_sync_test.cpp
   tsan_unit_test_main.cpp
   )
 
 add_tsan_unittest(TsanUnitTest
   SOURCES ${TSAN_UNIT_TEST_SOURCES})

compiler-rt/lib/tsan/tests/unit/tsan_ilist_test.cpp

+//===-- tsan_ilist_test.cpp -----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of ThreadSanitizer (TSan), a race detector.
+//
+//===----------------------------------------------------------------------===//
+#include "tsan_ilist.h"
+
+#include "gtest/gtest.h"
+
+namespace __tsan {
+
+struct Node {
+  INode node1;
+  INode node2;
+};
+
+struct Parent : Node {};
+
+TEST(IList, Empty) {
+  IList<Node, &Node::node1> list;
+  Node node;
+
+  EXPECT_TRUE(list.Empty());
+  EXPECT_EQ(list.Size(), 0);
+  EXPECT_EQ(list.Back(), nullptr);
+  EXPECT_EQ(list.Front(), nullptr);
+  EXPECT_EQ(list.PopBack(), nullptr);
+  EXPECT_EQ(list.PopFront(), nullptr);
+  EXPECT_FALSE(list.Queued(&node));
+}
+
+TEST(IList, OneNode) {
+  IList<Node, &Node::node1> list;
+  Node node;
+
+  list.PushBack(&node);
+  EXPECT_FALSE(list.Empty());
+  EXPECT_EQ(list.Size(), 1);
+  EXPECT_EQ(list.Back(), &node);
+  EXPECT_EQ(list.Front(), &node);
+  EXPECT_TRUE(list.Queued(&node));
+  EXPECT_EQ(list.Prev(&node), nullptr);
+  EXPECT_EQ(list.Next(&node), nullptr);
+
+  EXPECT_EQ(list.PopFront(), &node);
+  EXPECT_TRUE(list.Empty());
+  EXPECT_EQ(list.Size(), 0);
+  EXPECT_FALSE(list.Queued(&node));
+}
+
+TEST(IList, MultipleNodes) {
+  IList<Node, &Node::node1> list;
+  Node nodes[3];
+
+  list.PushBack(&nodes[1]);
+  list.PushBack(&nodes[0]);
+  list.PushFront(&nodes[2]);
+
+  EXPECT_EQ(list.Size(), 3);
+  EXPECT_EQ(list.Back(), &nodes[0]);
+  EXPECT_EQ(list.Front(), &nodes[2]);
+
+  EXPECT_EQ(list.Next(&nodes[0]), nullptr);
+  EXPECT_EQ(list.Prev(&nodes[0]), &nodes[1]);
+
+  EXPECT_EQ(list.Next(&nodes[1]), &nodes[0]);
+  EXPECT_EQ(list.Prev(&nodes[1]), &nodes[2]);
+
+  EXPECT_EQ(list.Next(&nodes[2]), &nodes[1]);
+  EXPECT_EQ(list.Prev(&nodes[2]), nullptr);
+
+  EXPECT_EQ(list.PopBack(), &nodes[0]);
+  EXPECT_EQ(list.PopFront(), &nodes[2]);
+  EXPECT_EQ(list.PopFront(), &nodes[1]);
+  EXPECT_TRUE(list.Empty());
+}
+
+TEST(IList, TwoLists) {
+  IList<Node, &Node::node1> list1;
+  IList<Node, &Node::node2, Parent> list2;
+  Parent nodes[3];
+
+  list1.PushBack(&nodes[2]);
+  list1.PushBack(&nodes[1]);
+  list1.PushBack(&nodes[0]);
+
+  list2.PushFront(&nodes[1]);
+
+  EXPECT_EQ(list1.Size(), 3);
+  EXPECT_TRUE(list1.Queued(&nodes[0]));
+  EXPECT_TRUE(list1.Queued(&nodes[1]));
+  EXPECT_TRUE(list1.Queued(&nodes[2]));
+
+  EXPECT_EQ(list2.Size(), 1);
+  EXPECT_FALSE(list2.Queued(&nodes[0]));
+  EXPECT_TRUE(list2.Queued(&nodes[1]));
+  EXPECT_FALSE(list2.Queued(&nodes[2]));
+
+  EXPECT_EQ(list1.Next(&nodes[1]), &nodes[0]);
+  EXPECT_EQ(list1.Prev(&nodes[1]), &nodes[2]);
+
+  EXPECT_EQ(list2.Next(&nodes[1]), nullptr);
+  EXPECT_EQ(list2.Prev(&nodes[1]), nullptr);
+
+  list1.Remove(&nodes[1]);
+  EXPECT_EQ(list1.Size(), 2);
+  EXPECT_FALSE(list1.Queued(&nodes[1]));
+  EXPECT_EQ(list2.Size(), 1);
+  EXPECT_TRUE(list2.Queued(&nodes[1]));
+
+  EXPECT_EQ(list1.PopBack(), &nodes[0]);
+  EXPECT_EQ(list1.PopBack(), &nodes[2]);
+  EXPECT_EQ(list1.Size(), 0);
+
+  EXPECT_EQ(list2.PopBack(), &nodes[1]);
+  EXPECT_EQ(list2.Size(), 0);
+}
+
+}  // namespace __tsan