[compiler-rt] [scudo] Support linking with index in IntrusiveList (PR #101262)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 9 10:46:40 PDT 2024
https://github.com/ChiaHungDuan updated https://github.com/llvm/llvm-project/pull/101262
>From 0b0e5383f0577251eced3763ea0959e18a32900d Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Tue, 30 Jul 2024 22:17:38 +0000
Subject: [PATCH 1/5] [scudo] Support linking with index in IntrusiveList
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.
---
compiler-rt/lib/scudo/standalone/list.h | 185 ++++++++++++++----
.../lib/scudo/standalone/tests/list_test.cpp | 91 ++++++---
2 files changed, 206 insertions(+), 70 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/list.h b/compiler-rt/lib/scudo/standalone/list.h
index 0137667d1dcf3e..c4817b172d17e9 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 140ca027ae9283..adcf0107455888 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>();
+}
>From 9fb47466a66ae0fd2c0c42acb62eadf25bfbfef6 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Mon, 5 Aug 2024 18:57:03 +0000
Subject: [PATCH 2/5] Reorder the code sections
---
compiler-rt/lib/scudo/standalone/list.h | 32 ++++++++++++-------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/list.h b/compiler-rt/lib/scudo/standalone/list.h
index c4817b172d17e9..1900037bcb7d86 100644
--- a/compiler-rt/lib/scudo/standalone/list.h
+++ b/compiler-rt/lib/scudo/standalone/list.h
@@ -40,6 +40,22 @@ namespace scudo {
template <class T, bool LinkWithPtr = isPointer<decltype(T::Next)>::value>
class LinkOp {
+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 LinkOp<T, /*LinkWithPtr=*/false> {
public:
using LinkTy = decltype(T::Next);
@@ -85,22 +101,6 @@ class LinkOp {
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)
>From 136f5439cfa8180aa495c4f29ca34bee0d90e553 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Thu, 8 Aug 2024 23:30:54 +0000
Subject: [PATCH 3/5] Add Size in the LinkOp
---
compiler-rt/lib/scudo/standalone/list.h | 22 +++++++++++++------
.../lib/scudo/standalone/tests/list_test.cpp | 14 ++++++------
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/list.h b/compiler-rt/lib/scudo/standalone/list.h
index 1900037bcb7d86..6148093ca146f8 100644
--- a/compiler-rt/lib/scudo/standalone/list.h
+++ b/compiler-rt/lib/scudo/standalone/list.h
@@ -31,10 +31,10 @@ namespace scudo {
// 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.
+// array, instead of using a pointer type, linking with an index type can save
+// some space.
//
-// There are two things to be noticed while using index type,
+// There are two things to be noticed while using an index type,
// 1. Call init() to set up the base address of the array.
// 2. Define `EndOfListVal` as the nil of the list.
@@ -43,7 +43,7 @@ class LinkOp {
public:
LinkOp() = default;
LinkOp(UNUSED T *BaseT) {}
- void init(UNUSED T *LinkBase) {}
+ void init(UNUSED T *LinkBase, UNUSED uptr Size) {}
T *getBase() const { return nullptr; }
T *getNext(T *X) const { return X->Next; }
@@ -61,13 +61,18 @@ template <class T> class LinkOp<T, /*LinkWithPtr=*/false> {
LinkOp() = default;
LinkOp(T *BaseT) : Base(BaseT) {}
- void init(T *LinkBase) { Base = LinkBase; }
+ void init(T *LinkBase, uptr BaseSize) {
+ Base = LinkBase;
+ // TODO: Check if the `BaseSize` can fit in `Size`.
+ Size = static_cast<LinkTy>(BaseSize);
+ }
T *getBase() const { return Base; }
T *getNext(T *X) const {
DCHECK_NE(getBase(), nullptr);
if (X->Next == getEndOfListVal())
return nullptr;
+ DCHECK_LT(X->Next, Size);
return &Base[X->Next];
}
// Set `X->Next` to `Next`.
@@ -83,11 +88,13 @@ template <class T> class LinkOp<T, /*LinkWithPtr=*/false> {
DCHECK_NE(getBase(), nullptr);
if (X->Prev == getEndOfListVal())
return nullptr;
+ DCHECK_LT(X->Prev, Size);
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`.
+ DCHECK_LT(reinterpret_cast<uptr>(Prev),
+ reinterpret_cast<uptr>(Base + Size));
if (Prev == nullptr)
X->Prev = getEndOfListVal();
else
@@ -99,6 +106,7 @@ template <class T> class LinkOp<T, /*LinkWithPtr=*/false> {
protected:
T *Base = nullptr;
+ LinkTy Size = 0;
};
template <class T> class IteratorBase : public LinkOp<T> {
@@ -120,7 +128,7 @@ template <class T> class IteratorBase : public LinkOp<T> {
template <class T> struct IntrusiveList : public LinkOp<T> {
IntrusiveList() = default;
IntrusiveList(T *Base) { init(Base); }
- void init(T *Base) { LinkOp<T>::init(Base); }
+ void init(T *Base, uptr BaseSize) { LinkOp<T>::init(Base, BaseSize); }
bool empty() const { return Size == 0; }
uptr size() const { return Size; }
diff --git a/compiler-rt/lib/scudo/standalone/tests/list_test.cpp b/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
index adcf0107455888..45815a51356690 100644
--- a/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
@@ -66,14 +66,14 @@ static void checkList(ListT *L, ListItemTy *I1, ListItemTy *I2 = nullptr,
template <template <typename> class ListTy, typename ListItemTy>
static void testListCommon(void) {
- ListItemTy Items[6];
+ ListItemTy Items[3];
ListItemTy *X = &Items[0];
ListItemTy *Y = &Items[1];
ListItemTy *Z = &Items[2];
ListTy<ListItemTy> L;
L.clear();
- L.init(Items);
+ L.init(Items, sizeof(Items));
EXPECT_EQ(L.size(), 0U);
L.push_back(X);
@@ -146,7 +146,7 @@ static void testSinglyLinkedList() {
ListTy<ListItemTy> L;
L.clear();
- L.init(Items);
+ L.init(Items, sizeof(Items));
L.push_back(X);
L.push_back(Y);
@@ -167,8 +167,8 @@ static void testSinglyLinkedList() {
ListTy<ListItemTy> L1, L2;
L1.clear();
L2.clear();
- L1.init(Items);
- L2.init(Items);
+ L1.init(Items, sizeof(Items));
+ L2.init(Items, sizeof(Items));
L1.append_back(&L2);
EXPECT_TRUE(L1.empty());
@@ -203,14 +203,14 @@ TEST(ScudoListTest, SinglyLinkedList) {
template <template <typename> class ListTy, typename ListItemTy>
static void testDoublyLinkedList() {
- ListItemTy Items[6];
+ ListItemTy Items[3];
ListItemTy *X = &Items[0];
ListItemTy *Y = &Items[1];
ListItemTy *Z = &Items[2];
ListTy<ListItemTy> L;
L.clear();
- L.init(Items);
+ L.init(Items, sizeof(Items));
L.push_back(X);
L.push_back(Y);
>From b99f63fae1988acb56ab203ac97d5f78eba2d217 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Fri, 9 Aug 2024 17:12:04 +0000
Subject: [PATCH 4/5] Fix missing argument Size in LinkOp ctor
---
compiler-rt/lib/scudo/standalone/list.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/list.h b/compiler-rt/lib/scudo/standalone/list.h
index 6148093ca146f8..b7e6377a5e7980 100644
--- a/compiler-rt/lib/scudo/standalone/list.h
+++ b/compiler-rt/lib/scudo/standalone/list.h
@@ -42,9 +42,10 @@ template <class T, bool LinkWithPtr = isPointer<decltype(T::Next)>::value>
class LinkOp {
public:
LinkOp() = default;
- LinkOp(UNUSED T *BaseT) {}
+ LinkOp(UNUSED T *BaseT, UNUSED uptr BaseSize) {}
void init(UNUSED T *LinkBase, UNUSED uptr Size) {}
T *getBase() const { return nullptr; }
+ uptr getSize() const { return 0; }
T *getNext(T *X) const { return X->Next; }
void setNext(T *X, T *Next) const { X->Next = Next; }
@@ -60,13 +61,14 @@ template <class T> class LinkOp<T, /*LinkWithPtr=*/false> {
using LinkTy = decltype(T::Next);
LinkOp() = default;
- LinkOp(T *BaseT) : Base(BaseT) {}
+ LinkOp(T *BaseT, uptr BaseSize) : Base(BaseT), Size(BaseSize) {}
void init(T *LinkBase, uptr BaseSize) {
Base = LinkBase;
// TODO: Check if the `BaseSize` can fit in `Size`.
Size = static_cast<LinkTy>(BaseSize);
}
T *getBase() const { return Base; }
+ LinkTy getSize() const { return Size; }
T *getNext(T *X) const {
DCHECK_NE(getBase(), nullptr);
@@ -127,7 +129,6 @@ template <class T> class IteratorBase : public LinkOp<T> {
template <class T> struct IntrusiveList : public LinkOp<T> {
IntrusiveList() = default;
- IntrusiveList(T *Base) { init(Base); }
void init(T *Base, uptr BaseSize) { LinkOp<T>::init(Base, BaseSize); }
bool empty() const { return Size == 0; }
@@ -150,10 +151,10 @@ template <class T> struct IntrusiveList : public LinkOp<T> {
Iterator end() { return Iterator(*this, this->getEndOfListVal()); }
ConstIterator begin() const {
- return ConstIterator(LinkOp<const T>(this->getBase()), First);
+ return ConstIterator(LinkOp<const T>(this->getBase(), this->getSize()), First);
}
ConstIterator end() const {
- return ConstIterator(LinkOp<const T>(this->getBase()),
+ return ConstIterator(LinkOp<const T>(this->getBase(), this->getSize()),
this->getEndOfListVal());
}
>From 89d637536521fedc7ac32e14fa0a0deacfebf817 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Fri, 9 Aug 2024 17:45:55 +0000
Subject: [PATCH 5/5] Fix iterator init and add a test for iterator
---
compiler-rt/lib/scudo/standalone/list.h | 13 +++++++++----
.../lib/scudo/standalone/tests/list_test.cpp | 14 ++++++++++++++
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/list.h b/compiler-rt/lib/scudo/standalone/list.h
index b7e6377a5e7980..6b952a610e3055 100644
--- a/compiler-rt/lib/scudo/standalone/list.h
+++ b/compiler-rt/lib/scudo/standalone/list.h
@@ -147,15 +147,20 @@ template <class T> struct IntrusiveList : public LinkOp<T> {
typedef IteratorBase<T> Iterator;
typedef IteratorBase<const T> ConstIterator;
- Iterator begin() { return Iterator(*this, First); }
- Iterator end() { return Iterator(*this, this->getEndOfListVal()); }
+ Iterator begin() {
+ return Iterator(LinkOp<T>(this->getBase(), this->getSize()), First);
+ }
+ Iterator end() {
+ return Iterator(LinkOp<T>(this->getBase(), this->getSize()), nullptr);
+ }
ConstIterator begin() const {
- return ConstIterator(LinkOp<const T>(this->getBase(), this->getSize()), First);
+ return ConstIterator(LinkOp<const T>(this->getBase(), this->getSize()),
+ First);
}
ConstIterator end() const {
return ConstIterator(LinkOp<const T>(this->getBase(), this->getSize()),
- this->getEndOfListVal());
+ nullptr);
}
void checkConsistency() const;
diff --git a/compiler-rt/lib/scudo/standalone/tests/list_test.cpp b/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
index 45815a51356690..688cbbef6a032f 100644
--- a/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/list_test.cpp
@@ -10,6 +10,8 @@
#include "list.h"
+#include <array>
+
struct ListItemLinkedWithPtr {
ListItemLinkedWithPtr *Next;
ListItemLinkedWithPtr *Prev;
@@ -125,6 +127,18 @@ static void testListCommon(void) {
L.pop_front();
EXPECT_TRUE(L.empty());
L.checkConsistency();
+
+ L.push_back(X);
+ L.push_back(Y);
+ L.push_back(Z);
+
+ // Verify the iterator
+ std::array<ListItemTy *, 3> visitOrder{X, Y, Z};
+ auto Iter = visitOrder.begin();
+ for (const auto &Item : L) {
+ EXPECT_EQ(&Item, *Iter);
+ ++Iter;
+ }
}
TEST(ScudoListTest, LinkedListCommon) {
More information about the llvm-commits
mailing list