Skip to content

Commit 7a97c14

Browse files
PagghiuPagghiu
committed
Foundation: Refactor Segment to reduce and isolate UB
1 parent ac70d9c commit 7a97c14

File tree

12 files changed

+333
-375
lines changed

12 files changed

+333
-375
lines changed

‎Libraries/Containers/Array.h‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,29 @@ namespace SC
77
{
88
namespace detail
99
{
10-
template <typename T>
10+
template <typename T, int N>
1111
struct ArrayVTable : public ObjectVTable<T>
1212
{
1313
static constexpr bool IsArray = true;
14+
15+
T* data() { return items; }
16+
const T* data() const { return items; }
17+
void setData(T*) {}
18+
T* getInlineData() { return items; }
19+
uint32_t getInlineCapacity() { return header.capacityBytes; }
20+
21+
static constexpr bool isInline() { return true; }
22+
23+
ArrayVTable(uint32_t capacity = 0) : header(capacity) {}
24+
~ArrayVTable() {}
25+
26+
SegmentHeader header;
27+
union
28+
{
29+
T items[N];
30+
};
1431
};
32+
1533
} // namespace detail
1634
//! @addtogroup group_containers
1735
//! @{
@@ -27,11 +45,10 @@ struct ArrayVTable : public ObjectVTable<T>
2745
///
2846
/// \snippet Libraries/Containers/Tests/ArrayTest.cpp ArraySnippet
2947
template <typename T, int N>
30-
struct Array : public Segment<detail::ArrayVTable<T>>
48+
struct Array : public Segment<detail::ArrayVTable<T, N>>
3149
{
32-
using Parent = Segment<detail::ArrayVTable<T>>;
50+
using Parent = Segment<detail::ArrayVTable<T, N>>;
3351
Array() : Parent(sizeof(T) * N){};
34-
~Array() {}
3552
// clang-format off
3653
Array(std::initializer_list<T> list) : Array() { SC_ASSERT_RELEASE(Parent::assign({list.begin(), list.size()})); }
3754
Array(const Array& other) : Array() { SC_ASSERT_RELEASE(Parent::append(other.toSpanConst())); }
@@ -41,11 +58,11 @@ struct Array : public Segment<detail::ArrayVTable<T>>
4158
template <int M>
4259
Array(const Array<T, M>& other) : Array() { SC_ASSERT_RELEASE(Parent::assign(other.toSpanConst()));}
4360
template <int M>
44-
Array(Array<T, M>&& other) : Array() { SC_ASSERT_RELEASE(Parent::assign(move(other))); }
61+
Array(Array<T, M>&& other) : Array() { SC_ASSERT_RELEASE(Parent::assignMove(move(other))); }
4562
template <int M>
4663
Array& operator=(const Array<T, M>& other) { SC_ASSERT_RELEASE(Parent::assign(other.toSpanConst())); return *this; }
4764
template <int M>
48-
Array& operator=(Array<T, M>&& other) { SC_ASSERT_RELEASE(Parent::assign(move(other))); return *this; }
65+
Array& operator=(Array<T, M>&& other) { SC_ASSERT_RELEASE(Parent::assignMove(move(other))); return *this; }
4966
Array(Span<const T> span) : Array() { SC_ASSERT_RELEASE(Parent::assign(span)); }
5067
template <typename U>
5168
Array(Span<const U> span) : Array() { SC_ASSERT_RELEASE(Parent::assign(span)); }
@@ -101,12 +118,6 @@ struct Array : public Segment<detail::ArrayVTable<T>>
101118
{
102119
return removeAll([&](auto& item) { return item == value; });
103120
}
104-
105-
private:
106-
union
107-
{
108-
T items[N];
109-
};
110121
};
111122

112123
//! @}

‎Libraries/Containers/SmallVector.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ struct SC::SmallVector : public Vector<T>
2828
{
2929
// clang-format off
3030
SmallVector() : Vector<T>( N * sizeof(T)) {}
31+
~SmallVector() {}
3132
SmallVector(const Vector<T>& other) : SmallVector() { Vector<T>::operator=(other); }
3233
SmallVector(Vector<T>&& other) : SmallVector() { Vector<T>::operator=(move(other)); }
3334
Vector<T>& operator=(const Vector<T>& other) { return Vector<T>::operator=(other); }
@@ -43,6 +44,9 @@ struct SC::SmallVector : public Vector<T>
4344

4445
private:
4546
uint64_t inlineCapacity = N * sizeof(T);
46-
char inlineBuffer[N * sizeof(T)];
47+
union
48+
{
49+
T inlineData[N];
50+
};
4751
};
4852
//! @}

‎Libraries/Containers/Tests/SmallVectorTest.cpp‎

Lines changed: 77 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -20,37 +20,31 @@ struct SC::SmallVectorTest : public SC::TestCase
2020
SC_TEST_EXPECT(vec.shrink_to_fit());
2121
SC_TEST_EXPECT(vec.capacity() == 3);
2222
SC_TEST_EXPECT(vec.size() == 2);
23-
SC_TEST_EXPECT(vec.isInlineBuffer());
23+
SC_TEST_EXPECT(vec.isInline());
2424
}
2525
if (test_section("resize stack heap"))
2626
{
2727
SmallVector<int, 3> vec;
2828
SC_TEST_EXPECT(vec.resize(3));
29-
auto* header = vec.getHeader();
30-
SC_TEST_EXPECT(vec.isInlineBuffer());
31-
SC_TEST_EXPECT(not header->restoreInlineBuffer);
29+
SC_TEST_EXPECT(vec.isInline());
3230
SC_TEST_EXPECT(vec.resize(4));
33-
header = vec.getHeader();
34-
SC_TEST_EXPECT(not vec.isInlineBuffer());
35-
SC_TEST_EXPECT(header->restoreInlineBuffer);
31+
SC_TEST_EXPECT(not vec.isInline());
3632
SC_TEST_EXPECT(vec.resize(3));
3733
SC_TEST_EXPECT(vec.shrink_to_fit());
38-
header = vec.getHeader();
39-
SC_TEST_EXPECT(vec.isInlineBuffer());
40-
SC_TEST_EXPECT(not header->restoreInlineBuffer);
34+
SC_TEST_EXPECT(vec.isInline());
4135
}
4236
if (test_section("construction copy stack"))
4337
{
4438
SmallVector<int, 3> vec2;
4539
{
4640
SmallVector<int, 3> vec;
4741
addItems(vec, 3);
48-
SC_TEST_EXPECT(vec.isInlineBuffer() and vec.size() == 3);
42+
SC_TEST_EXPECT(vec.isInline() and vec.size() == 3);
4943
SC_TEST_EXPECT(vec.push_back(3));
50-
SC_TEST_EXPECT(not vec.isInlineBuffer());
44+
SC_TEST_EXPECT(not vec.isInline());
5145
SC_TEST_EXPECT(vec.pop_back());
5246
SC_TEST_EXPECT(vec.shrink_to_fit());
53-
SC_TEST_EXPECT(vec.isInlineBuffer() and vec.size() == 3);
47+
SC_TEST_EXPECT(vec.isInline() and vec.size() == 3);
5448
vec2 = vec;
5549
}
5650
SC_TEST_EXPECT(vec2.size() == 3);
@@ -65,11 +59,9 @@ struct SC::SmallVectorTest : public SC::TestCase
6559
SC_TEST_EXPECT(vec.size() == 4);
6660
vec2 = vec;
6761
}
68-
auto* vec2Header = vec2.getHeader();
69-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
70-
SC_TEST_EXPECT(vec2Header->restoreInlineBuffer);
62+
SC_TEST_EXPECT(not vec2.isInline());
7163
SC_TEST_EXPECT(vec2.size() == 4);
72-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
64+
SC_TEST_EXPECT(not vec2.isInline());
7365
checkItems(vec2, 4);
7466
}
7567
if (test_section("construction move SmallVector(stack)->Vector"))
@@ -81,148 +73,125 @@ struct SC::SmallVectorTest : public SC::TestCase
8173
SC_TEST_EXPECT(vec.size() == 3);
8274
vec2 = move(vec);
8375
}
84-
auto* vec2Header = vec2.getHeader();
85-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
86-
SC_TEST_EXPECT(not vec2Header->restoreInlineBuffer);
76+
SC_TEST_EXPECT(not vec2.isInline());
8777
checkItems(vec2, 3);
8878
}
8979
if (test_section("construction move SmallVector(heap)->Vector"))
9080
{
91-
Vector<int> vec2;
81+
Vector<int> vec4;
9282
{
93-
SmallVector<int, 3> vec;
94-
auto* vec1Header = vec.getHeader();
95-
addItems(vec, 4);
96-
SC_TEST_EXPECT(vec.size() == 4);
83+
SmallVector<int, 3> smallVec3;
84+
addItems(smallVec3, 4);
85+
SC_TEST_EXPECT(smallVec3.size() == 4);
9786

98-
vec2 = move(vec);
99-
SC_TEST_EXPECT(vec.data() != nullptr);
100-
auto* vec1Header2 = vec.getHeader();
101-
SC_TEST_EXPECT(vec1Header2 == vec1Header);
102-
SC_TEST_EXPECT(vec.isInlineBuffer());
103-
SC_TEST_EXPECT(vec1Header2->capacityBytes == 3 * sizeof(int));
87+
vec4 = move(smallVec3);
88+
SC_TEST_EXPECT(smallVec3.data() != nullptr);
89+
SC_TEST_EXPECT(smallVec3.isInline());
90+
SC_TEST_EXPECT(smallVec3.capacity() == 3); // restored initial capacity
10491
}
105-
auto* vec2Header = vec2.getHeader();
106-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
107-
SC_TEST_EXPECT(not vec2Header->restoreInlineBuffer);
108-
checkItems(vec2, 4);
92+
SC_TEST_EXPECT(not vec4.isInline());
93+
checkItems(vec4, 4);
10994
}
11095
if (test_section("construction move Vector->SmallVector(heap)"))
11196
{
112-
SmallVector<int, 3> vec2;
97+
SmallVector<int, 3> smallVec3;
11398
{
114-
Vector<int> vec;
115-
addItems(vec, 4);
116-
SC_TEST_EXPECT(vec.size() == 4);
117-
vec2 = move(vec);
118-
SC_TEST_EXPECT(vec.data() == nullptr);
99+
Vector<int> vec4;
100+
addItems(vec4, 4);
101+
SC_TEST_EXPECT(vec4.size() == 4);
102+
smallVec3 = move(vec4);
103+
SC_TEST_EXPECT(vec4.data() == nullptr);
119104
}
120-
auto* vec2Header = vec2.getHeader();
121-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
122-
SC_TEST_EXPECT(vec2Header->restoreInlineBuffer);
123-
checkItems(vec2, 4);
105+
SC_TEST_EXPECT(not smallVec3.isInline());
106+
checkItems(smallVec3, 4);
124107
}
125108
if (test_section("construction move Vector->SmallVector(stack)"))
126109
{
127-
SmallVector<int, 3> vec2;
110+
SmallVector<int, 3> smallVec3;
128111
{
129-
Vector<int> vec;
130-
addItems(vec, 3);
131-
SC_TEST_EXPECT(vec.size() == 3);
132-
vec2 = move(vec);
133-
SC_TEST_EXPECT(vec.data() == nullptr);
112+
Vector<int> vec3;
113+
addItems(vec3, 3);
114+
SC_TEST_EXPECT(vec3.size() == 3);
115+
smallVec3 = move(vec3);
116+
SC_TEST_EXPECT(vec3.data() == nullptr);
134117
}
135-
auto* vec2Header = vec2.getHeader();
136-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
137-
SC_TEST_EXPECT(vec2Header->restoreInlineBuffer);
138-
SC_TEST_EXPECT(vec2.size() == 3);
139-
checkItems(vec2, 3);
118+
SC_TEST_EXPECT(not smallVec3.isInline());
119+
SC_TEST_EXPECT(smallVec3.size() == 3);
120+
checkItems(smallVec3, 3);
140121
}
141122
if (test_section("construction move SmallVector(stack)->SmallVector(stack)"))
142123
{
143-
SmallVector<int, 3> vec2;
124+
SmallVector<int, 3> smallVec3A;
144125
{
145-
SmallVector<int, 3> vec;
146-
addItems(vec, 3);
147-
SC_TEST_EXPECT(vec.size() == 3);
148-
vec2 = move(vec);
126+
SmallVector<int, 3> smallVec3B;
127+
addItems(smallVec3B, 3);
128+
SC_TEST_EXPECT(smallVec3B.size() == 3);
129+
smallVec3A = move(smallVec3B);
149130
#ifndef __clang_analyzer__
150-
SC_TEST_EXPECT(vec.size() == 0);
131+
SC_TEST_EXPECT(smallVec3B.size() == 0);
151132
#endif // not __clang_analyzer__
152-
SC_TEST_EXPECT(vec2.size() == 3);
153-
auto* vec1Header = vec.getHeader();
154-
SC_TEST_EXPECT(vec1Header != nullptr);
133+
SC_TEST_EXPECT(smallVec3A.size() == 3);
155134
#ifndef __clang_analyzer__
156-
SC_TEST_EXPECT(vec.isInlineBuffer());
135+
SC_TEST_EXPECT(smallVec3B.isInline());
157136
#endif // not __clang_analyzer__
158137
}
159-
SC_TEST_EXPECT(vec2.isInlineBuffer());
160-
checkItems(vec2, 3);
138+
SC_TEST_EXPECT(smallVec3A.isInline());
139+
checkItems(smallVec3A, 3);
161140
}
162141
if (test_section("construction move SmallVector(heap)->SmallVector(stack)"))
163142
{
164-
SmallVector<int, 3> vec2;
143+
SmallVector<int, 3> smallVec3A;
165144
{
166-
SmallVector<int, 3> vec;
167-
addItems(vec, 4);
168-
SC_TEST_EXPECT(vec.size() == 4);
169-
vec2 = move(vec);
145+
SmallVector<int, 3> smallVec3B;
146+
addItems(smallVec3B, 4);
147+
SC_TEST_EXPECT(smallVec3B.size() == 4);
148+
smallVec3A = move(smallVec3B);
170149
#ifndef __clang_analyzer__
171-
SC_TEST_EXPECT(vec.size() == 0);
150+
SC_TEST_EXPECT(smallVec3B.size() == 0);
172151
#endif // not __clang_analyzer__
173-
SC_TEST_EXPECT(vec2.size() == 4);
174-
auto* vec1Header = vec.getHeader();
175-
SC_TEST_EXPECT(vec1Header != nullptr);
152+
SC_TEST_EXPECT(smallVec3A.size() == 4);
176153
#ifndef __clang_analyzer__
177-
SC_TEST_EXPECT(vec.isInlineBuffer());
154+
SC_TEST_EXPECT(smallVec3B.isInline());
178155
#endif // not __clang_analyzer__
179156
}
180-
auto* vec2Header = vec2.getHeader();
181-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
182-
SC_TEST_EXPECT(vec2Header->restoreInlineBuffer);
183-
checkItems(vec2, 4);
157+
SC_TEST_EXPECT(not smallVec3A.isInline());
158+
checkItems(smallVec3A, 4);
184159
}
185160
if (test_section("construction move SmallVector(stack)->SmallVector(stack)"))
186161
{
187-
SmallVector<int, 3> vec2;
162+
SmallVector<int, 3> smallVec3A;
188163
{
189-
SmallVector<int, 3> vec;
190-
addItems(vec, 3);
191-
SC_TEST_EXPECT(vec.size() == 3);
192-
vec2 = move(vec);
164+
SmallVector<int, 3> smallVec3B;
165+
addItems(smallVec3B, 3);
166+
SC_TEST_EXPECT(smallVec3B.size() == 3);
167+
smallVec3A = move(smallVec3B);
193168
#ifndef __clang_analyzer__
194-
SC_TEST_EXPECT(vec.size() == 0);
169+
SC_TEST_EXPECT(smallVec3B.size() == 0);
195170
#endif // not __clang_analyzer__
196-
SC_TEST_EXPECT(vec2.size() == 3);
197-
auto* vec1Header = vec.getHeader();
198-
SC_TEST_EXPECT(vec1Header != nullptr);
171+
SC_TEST_EXPECT(smallVec3A.size() == 3);
199172
#ifndef __clang_analyzer__
200-
SC_TEST_EXPECT(vec.isInlineBuffer());
173+
SC_TEST_EXPECT(smallVec3B.isInline());
201174
#endif // not __clang_analyzer__
202175
}
203-
SC_TEST_EXPECT(vec2.isInlineBuffer());
204-
checkItems(vec2, 3);
176+
SC_TEST_EXPECT(smallVec3A.isInline());
177+
checkItems(smallVec3A, 3);
205178
}
206179
if (test_section("construction move SmallVector(heap)->SmallVector(stack)"))
207180
{
208-
SmallVector<int, 4> vec2;
181+
SmallVector<int, 4> smallVec4;
209182
{
210-
SmallVector<int, 3> vec;
211-
addItems(vec, 4);
212-
SC_TEST_EXPECT(vec.size() == 4);
213-
vec2 = move(vec);
214-
SC_TEST_EXPECT(vec.size() == 0);
215-
SC_TEST_EXPECT(vec2.size() == 4);
216-
auto* vec1Header = vec.getHeader();
217-
SC_TEST_EXPECT(vec1Header != nullptr);
183+
SmallVector<int, 3> smallVec3;
184+
addItems(smallVec3, 4);
185+
SC_TEST_EXPECT(smallVec3.size() == 4);
186+
smallVec4 = move(smallVec3);
187+
SC_TEST_EXPECT(smallVec3.size() == 0);
188+
SC_TEST_EXPECT(smallVec4.size() == 4);
218189
#ifndef __clang_analyzer__
219-
SC_TEST_EXPECT(vec.isInlineBuffer());
190+
SC_TEST_EXPECT(smallVec3.isInline());
220191
#endif // not __clang_analyzer__
221192
}
222-
auto* vec2Header = vec2.getHeader();
223-
SC_TEST_EXPECT(not vec2.isInlineBuffer());
224-
SC_TEST_EXPECT(vec2Header->restoreInlineBuffer);
225-
checkItems(vec2, 4);
193+
SC_TEST_EXPECT(not smallVec4.isInline());
194+
checkItems(smallVec4, 4);
226195
}
227196
if (test_section("move operations"))
228197
{

‎Libraries/Containers/Vector.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#pragma once
44
#include "../Algorithms/AlgorithmRemove.h" // removeIf
55
#include "../Foundation/Internal/Segment.inl"
6+
#include "../Foundation/Internal/SegmentTrivial.inl"
67
#include "../Foundation/TypeTraits.h" // IsTriviallyCopyable
78

89
namespace SC
@@ -11,7 +12,7 @@ namespace detail
1112
{
1213

1314
template <typename T, bool isTrivial = TypeTraits::IsTriviallyCopyable<T>::value>
14-
struct SegmentVTable : public SegmentTrivial
15+
struct SegmentVTable : public SegmentTrivial<T>
1516
{
1617
};
1718

@@ -153,7 +154,7 @@ struct ObjectVTable
153154
};
154155

155156
template <typename T>
156-
struct VectorVTable : public ObjectVTable<T>
157+
struct VectorVTable : public ObjectVTable<T>, public SegmentSelfRelativePointer<T>
157158
{
158159
static constexpr bool IsArray = false;
159160
};

0 commit comments

Comments
 (0)