[compiler-rt] [scudo] Support linking with index in IntrusiveList (PR #101262)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 30 16:38:41 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: None (ChiaHungDuan)
<details>
<summary>Changes</summary>
The nodes of list may be managed in an array. Instead of managing them with pointers, using the array index will save some memory in some cases.
When using the list linked with index, remember to call init() to set up the base address of the array and a `EndOfListVal` which is nil of the list.
---
Full diff: https://github.com/llvm/llvm-project/pull/101262.diff
2 Files Affected:
- (modified) compiler-rt/lib/scudo/standalone/list.h (+144-41)
- (modified) compiler-rt/lib/scudo/standalone/tests/list_test.cpp (+62-29)
``````````diff
diff --git a/compiler-rt/lib/scudo/standalone/list.h b/compiler-rt/lib/scudo/standalone/list.h
index 0137667d1dcf3..c4817b172d17e 100644
--- a/compiler-rt/lib/scudo/standalone/list.h
+++ b/compiler-rt/lib/scudo/standalone/list.h
@@ -11,17 +11,103 @@
#include "internal_defs.h"
+// TODO: Move the helpers to a header.
+namespace {
+template <typename T> struct isPointer {
+ static constexpr bool value = false;
+};
+
+template <typename T> struct isPointer<T *> {
+ static constexpr bool value = true;
+};
+} // namespace
+
namespace scudo {
// Intrusive POD singly and doubly linked list.
// An object with all zero fields should represent a valid empty list. clear()
// should be called on all non-zero-initialized objects before using.
+//
+// The intrusive list requires the member `Next` (and `Prev` if doubly linked
+// list)` defined in the node type. The type of `Next`/`Prev` can be a pointer
+// or an index to an array. For example, if the storage of the nodes is an
+// array, instead of using pointer type, linking with index type can save some
+// space.
+//
+// There are two things to be noticed while using index type,
+// 1. Call init() to set up the base address of the array.
+// 2. Define `EndOfListVal` as the nil of the list.
-template <class T> class IteratorBase {
+template <class T, bool LinkWithPtr = isPointer<decltype(T::Next)>::value>
+class LinkOp {
public:
- explicit IteratorBase(T *CurrentT) : Current(CurrentT) {}
+ using LinkTy = decltype(T::Next);
+
+ LinkOp() = default;
+ LinkOp(T *BaseT) : Base(BaseT) {}
+ void init(T *LinkBase) { Base = LinkBase; }
+ T *getBase() const { return Base; }
+
+ T *getNext(T *X) const {
+ DCHECK_NE(getBase(), nullptr);
+ if (X->Next == getEndOfListVal())
+ return nullptr;
+ return &Base[X->Next];
+ }
+ // Set `X->Next` to `Next`.
+ void setNext(T *X, T *Next) const {
+ // TODO: Check if the offset fits in the size of `LinkTy`.
+ if (Next == nullptr)
+ X->Next = getEndOfListVal();
+ else
+ X->Next = static_cast<LinkTy>(Next - Base);
+ }
+
+ T *getPrev(T *X) const {
+ DCHECK_NE(getBase(), nullptr);
+ if (X->Prev == getEndOfListVal())
+ return nullptr;
+ return &Base[X->Prev];
+ }
+ // Set `X->Prev` to `Prev`.
+ void setPrev(T *X, T *Prev) const {
+ // TODO: Check if the offset fits in the size of `LinkTy`.
+ if (Prev == nullptr)
+ X->Prev = getEndOfListVal();
+ else
+ X->Prev = static_cast<LinkTy>(Prev - Base);
+ }
+
+ // TODO: `LinkTy` should be the same as decltype(T::EndOfListVal).
+ LinkTy getEndOfListVal() const { return T::EndOfListVal; }
+
+protected:
+ T *Base = nullptr;
+};
+
+template <class T> class LinkOp<T, /*LinkWithPtr=*/true> {
+public:
+ LinkOp() = default;
+ LinkOp(UNUSED T *BaseT) {}
+ void init(UNUSED T *LinkBase) {}
+ T *getBase() const { return nullptr; }
+
+ T *getNext(T *X) const { return X->Next; }
+ void setNext(T *X, T *Next) const { X->Next = Next; }
+
+ T *getPrev(T *X) const { return X->Prev; }
+ void setPrev(T *X, T *Prev) const { X->Prev = Prev; }
+
+ T *getEndOfListVal() const { return nullptr; }
+};
+
+template <class T> class IteratorBase : public LinkOp<T> {
+public:
+ IteratorBase(const LinkOp<T> &Link, T *CurrentT)
+ : LinkOp<T>(Link), Current(CurrentT) {}
+
IteratorBase &operator++() {
- Current = Current->Next;
+ Current = this->getNext(Current);
return *this;
}
bool operator!=(IteratorBase Other) const { return Current != Other.Current; }
@@ -31,7 +117,11 @@ template <class T> class IteratorBase {
T *Current;
};
-template <class T> struct IntrusiveList {
+template <class T> struct IntrusiveList : public LinkOp<T> {
+ IntrusiveList() = default;
+ IntrusiveList(T *Base) { init(Base); }
+ void init(T *Base) { LinkOp<T>::init(Base); }
+
bool empty() const { return Size == 0; }
uptr size() const { return Size; }
@@ -48,11 +138,16 @@ template <class T> struct IntrusiveList {
typedef IteratorBase<T> Iterator;
typedef IteratorBase<const T> ConstIterator;
- Iterator begin() { return Iterator(First); }
- Iterator end() { return Iterator(nullptr); }
+ Iterator begin() { return Iterator(*this, First); }
+ Iterator end() { return Iterator(*this, this->getEndOfListVal()); }
- ConstIterator begin() const { return ConstIterator(First); }
- ConstIterator end() const { return ConstIterator(nullptr); }
+ ConstIterator begin() const {
+ return ConstIterator(LinkOp<const T>(this->getBase()), First);
+ }
+ ConstIterator end() const {
+ return ConstIterator(LinkOp<const T>(this->getBase()),
+ this->getEndOfListVal());
+ }
void checkConsistency() const;
@@ -68,13 +163,13 @@ template <class T> void IntrusiveList<T>::checkConsistency() const {
CHECK_EQ(Last, nullptr);
} else {
uptr Count = 0;
- for (T *I = First;; I = I->Next) {
+ for (T *I = First;; I = this->getNext(I)) {
Count++;
if (I == Last)
break;
}
CHECK_EQ(this->size(), Count);
- CHECK_EQ(Last->Next, nullptr);
+ CHECK_EQ(this->getNext(Last), nullptr);
}
}
@@ -83,13 +178,16 @@ template <class T> struct SinglyLinkedList : public IntrusiveList<T> {
using IntrusiveList<T>::Last;
using IntrusiveList<T>::Size;
using IntrusiveList<T>::empty;
+ using IntrusiveList<T>::setNext;
+ using IntrusiveList<T>::getNext;
+ using IntrusiveList<T>::getEndOfListVal;
void push_back(T *X) {
- X->Next = nullptr;
+ setNext(X, nullptr);
if (empty())
First = X;
else
- Last->Next = X;
+ setNext(Last, X);
Last = X;
Size++;
}
@@ -97,14 +195,14 @@ template <class T> struct SinglyLinkedList : public IntrusiveList<T> {
void push_front(T *X) {
if (empty())
Last = X;
- X->Next = First;
+ setNext(X, First);
First = X;
Size++;
}
void pop_front() {
DCHECK(!empty());
- First = First->Next;
+ First = getNext(First);
if (!First)
Last = nullptr;
Size--;
@@ -115,8 +213,8 @@ template <class T> struct SinglyLinkedList : public IntrusiveList<T> {
DCHECK(!empty());
DCHECK_NE(Prev, nullptr);
DCHECK_NE(X, nullptr);
- X->Next = Prev->Next;
- Prev->Next = X;
+ setNext(X, getNext(Prev));
+ setNext(Prev, X);
if (Last == Prev)
Last = X;
++Size;
@@ -126,8 +224,8 @@ template <class T> struct SinglyLinkedList : public IntrusiveList<T> {
DCHECK(!empty());
DCHECK_NE(Prev, nullptr);
DCHECK_NE(X, nullptr);
- DCHECK_EQ(Prev->Next, X);
- Prev->Next = X->Next;
+ DCHECK_EQ(getNext(Prev), X);
+ setNext(Prev, getNext(X));
if (Last == X)
Last = Prev;
Size--;
@@ -140,7 +238,7 @@ template <class T> struct SinglyLinkedList : public IntrusiveList<T> {
if (empty()) {
*this = *L;
} else {
- Last->Next = L->First;
+ setNext(Last, L->First);
Last = L->Last;
Size += L->size();
}
@@ -153,16 +251,21 @@ template <class T> struct DoublyLinkedList : IntrusiveList<T> {
using IntrusiveList<T>::Last;
using IntrusiveList<T>::Size;
using IntrusiveList<T>::empty;
+ using IntrusiveList<T>::setNext;
+ using IntrusiveList<T>::getNext;
+ using IntrusiveList<T>::setPrev;
+ using IntrusiveList<T>::getPrev;
+ using IntrusiveList<T>::getEndOfListVal;
void push_front(T *X) {
- X->Prev = nullptr;
+ setPrev(X, nullptr);
if (empty()) {
Last = X;
} else {
- DCHECK_EQ(First->Prev, nullptr);
- First->Prev = X;
+ DCHECK_EQ(getPrev(First), nullptr);
+ setPrev(First, X);
}
- X->Next = First;
+ setNext(X, First);
First = X;
Size++;
}
@@ -171,37 +274,37 @@ template <class T> struct DoublyLinkedList : IntrusiveList<T> {
void insert(T *X, T *Y) {
if (Y == First)
return push_front(X);
- T *Prev = Y->Prev;
+ T *Prev = getPrev(Y);
// This is a hard CHECK to ensure consistency in the event of an intentional
// corruption of Y->Prev, to prevent a potential write-{4,8}.
- CHECK_EQ(Prev->Next, Y);
- Prev->Next = X;
- X->Prev = Prev;
- X->Next = Y;
- Y->Prev = X;
+ CHECK_EQ(getNext(Prev), Y);
+ setNext(Prev, X);
+ setPrev(X, Prev);
+ setNext(X, Y);
+ setPrev(Y, X);
Size++;
}
void push_back(T *X) {
- X->Next = nullptr;
+ setNext(X, nullptr);
if (empty()) {
First = X;
} else {
- DCHECK_EQ(Last->Next, nullptr);
- Last->Next = X;
+ DCHECK_EQ(getNext(Last), nullptr);
+ setNext(Last, X);
}
- X->Prev = Last;
+ setPrev(X, Last);
Last = X;
Size++;
}
void pop_front() {
DCHECK(!empty());
- First = First->Next;
+ First = getNext(First);
if (!First)
Last = nullptr;
else
- First->Prev = nullptr;
+ setPrev(First, nullptr);
Size--;
}
@@ -209,15 +312,15 @@ template <class T> struct DoublyLinkedList : IntrusiveList<T> {
// catch potential corruption attempts, that could yield a mirrored
// write-{4,8} primitive. nullptr checks are deemed less vital.
void remove(T *X) {
- T *Prev = X->Prev;
- T *Next = X->Next;
+ T *Prev = getPrev(X);
+ T *Next = getNext(X);
if (Prev) {
- CHECK_EQ(Prev->Next, X);
- Prev->Next = Next;
+ CHECK_EQ(getNext(Prev), X);
+ setNext(Prev, Next);
}
if (Next) {
- CHECK_EQ(Next->Prev, X);
- Next->Prev = Prev;
+ CHECK_EQ(getPrev(Next), X);
+ setPrev(Next, Prev);
}
if (First == X) {
DCHECK_EQ(Prev, nullptr);
diff --git a/compiler-rt/lib/scudo/standalone/tests/list_test.cpp b/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
index 140ca027ae928..adcf010745588 100644
--- a/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
@@ -10,25 +10,20 @@
#include "list.h"
-struct ListItem {
- ListItem *Next;
- ListItem *Prev;
+struct ListItemLinkedWithPtr {
+ ListItemLinkedWithPtr *Next;
+ ListItemLinkedWithPtr *Prev;
};
-static ListItem Items[6];
-static ListItem *X = &Items[0];
-static ListItem *Y = &Items[1];
-static ListItem *Z = &Items[2];
-static ListItem *A = &Items[3];
-static ListItem *B = &Items[4];
-static ListItem *C = &Items[5];
-
-typedef scudo::SinglyLinkedList<ListItem> SLList;
-typedef scudo::DoublyLinkedList<ListItem> DLList;
+struct ListItemLinkedWithIndex {
+ scudo::uptr Next;
+ scudo::uptr Prev;
+ static constexpr scudo::uptr EndOfListVal = 1ULL << 30;
+};
-template <typename ListT>
-static void setList(ListT *L, ListItem *I1 = nullptr, ListItem *I2 = nullptr,
- ListItem *I3 = nullptr) {
+template <typename ListT, typename ListItemTy>
+static void setList(ListT *L, ListItemTy *I1 = nullptr,
+ ListItemTy *I2 = nullptr, ListItemTy *I3 = nullptr) {
L->clear();
if (I1)
L->push_back(I1);
@@ -38,10 +33,10 @@ static void setList(ListT *L, ListItem *I1 = nullptr, ListItem *I2 = nullptr,
L->push_back(I3);
}
-template <typename ListT>
-static void checkList(ListT *L, ListItem *I1, ListItem *I2 = nullptr,
- ListItem *I3 = nullptr, ListItem *I4 = nullptr,
- ListItem *I5 = nullptr, ListItem *I6 = nullptr) {
+template <typename ListT, typename ListItemTy>
+static void checkList(ListT *L, ListItemTy *I1, ListItemTy *I2 = nullptr,
+ ListItemTy *I3 = nullptr, ListItemTy *I4 = nullptr,
+ ListItemTy *I5 = nullptr, ListItemTy *I6 = nullptr) {
if (I1) {
EXPECT_EQ(L->front(), I1);
L->pop_front();
@@ -69,9 +64,16 @@ static void checkList(ListT *L, ListItem *I1, ListItem *I2 = nullptr,
EXPECT_TRUE(L->empty());
}
-template <typename ListT> static void testListCommon(void) {
- ListT L;
+template <template <typename> class ListTy, typename ListItemTy>
+static void testListCommon(void) {
+ ListItemTy Items[6];
+ ListItemTy *X = &Items[0];
+ ListItemTy *Y = &Items[1];
+ ListItemTy *Z = &Items[2];
+
+ ListTy<ListItemTy> L;
L.clear();
+ L.init(Items);
EXPECT_EQ(L.size(), 0U);
L.push_back(X);
@@ -126,13 +128,25 @@ template <typename ListT> static void testListCommon(void) {
}
TEST(ScudoListTest, LinkedListCommon) {
- testListCommon<SLList>();
- testListCommon<DLList>();
+ testListCommon<scudo::SinglyLinkedList, ListItemLinkedWithPtr>();
+ testListCommon<scudo::SinglyLinkedList, ListItemLinkedWithIndex>();
+ testListCommon<scudo::DoublyLinkedList, ListItemLinkedWithPtr>();
+ testListCommon<scudo::DoublyLinkedList, ListItemLinkedWithIndex>();
}
-TEST(ScudoListTest, SinglyLinkedList) {
- SLList L;
+template <template <typename> class ListTy, typename ListItemTy>
+static void testSinglyLinkedList() {
+ ListItemTy Items[6];
+ ListItemTy *X = &Items[0];
+ ListItemTy *Y = &Items[1];
+ ListItemTy *Z = &Items[2];
+ ListItemTy *A = &Items[3];
+ ListItemTy *B = &Items[4];
+ ListItemTy *C = &Items[5];
+
+ ListTy<ListItemTy> L;
L.clear();
+ L.init(Items);
L.push_back(X);
L.push_back(Y);
@@ -150,9 +164,11 @@ TEST(ScudoListTest, SinglyLinkedList) {
L.pop_front();
EXPECT_TRUE(L.empty());
- SLList L1, L2;
+ ListTy<ListItemTy> L1, L2;
L1.clear();
L2.clear();
+ L1.init(Items);
+ L2.init(Items);
L1.append_back(&L2);
EXPECT_TRUE(L1.empty());
@@ -180,9 +196,21 @@ TEST(ScudoListTest, SinglyLinkedList) {
EXPECT_EQ(L1.size(), 1U);
}
-TEST(ScudoListTest, DoublyLinkedList) {
- DLList L;
+TEST(ScudoListTest, SinglyLinkedList) {
+ testSinglyLinkedList<scudo::SinglyLinkedList, ListItemLinkedWithPtr>();
+ testSinglyLinkedList<scudo::SinglyLinkedList, ListItemLinkedWithIndex>();
+}
+
+template <template <typename> class ListTy, typename ListItemTy>
+static void testDoublyLinkedList() {
+ ListItemTy Items[6];
+ ListItemTy *X = &Items[0];
+ ListItemTy *Y = &Items[1];
+ ListItemTy *Z = &Items[2];
+
+ ListTy<ListItemTy> L;
L.clear();
+ L.init(Items);
L.push_back(X);
L.push_back(Y);
@@ -214,3 +242,8 @@ TEST(ScudoListTest, DoublyLinkedList) {
L.pop_front();
EXPECT_TRUE(L.empty());
}
+
+TEST(ScudoListTest, DoublyLinkedList) {
+ testDoublyLinkedList<scudo::DoublyLinkedList, ListItemLinkedWithPtr>();
+ testDoublyLinkedList<scudo::DoublyLinkedList, ListItemLinkedWithIndex>();
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/101262
More information about the llvm-commits
mailing list