Skip to content

Commit 6260825

Browse files
Fix memory corruption in vector
1 parent b48ef47 commit 6260825

File tree

1 file changed

+102
-27
lines changed

1 file changed

+102
-27
lines changed

source/numem/collections/vector.d

Lines changed: 102 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ private:
8383

8484
pragma(inline, true)
8585
void copy(T* dst, T* src, size_t length) {
86-
this.freeRange(dst, length);
86+
this.freeRange(dst-memory, length);
8787

8888
static if (__traits(hasCopyConstructor, T)) {
8989

@@ -110,14 +110,36 @@ private:
110110
}
111111
}
112112

113+
// Gets whether src overlaps with the memory in the vector.
114+
bool doSourceOverlap(T* src, size_t length = 0) {
115+
return src+length >= memory && src < memory+size_;
116+
}
117+
113118
pragma(inline, true)
114119
void move(T* dst, T* src, size_t length) {
115-
this.freeRange(dst, length);
116120
memmove(dst, src, T.sizeof*length);
121+
if (this.doSourceOverlap(src, length)) {
122+
if (src < dst) {
123+
this.fillRangeInit(src, dst-src);
124+
} else if (src > dst) {
125+
auto ptr = dst+length;
126+
auto len = (src-length)-ptr;
127+
this.fillRangeInit(ptr, len);
128+
}
129+
}
117130
}
118131

119132
pragma(inline, true)
120-
void freeRange(T* dst, size_t length) {
133+
void fillRangeInit(T* dst, size_t length) {
134+
foreach(i; 0..length) {
135+
136+
T tmp = T.init;
137+
memcpy(dst + i, &tmp, T.sizeof);
138+
}
139+
}
140+
141+
pragma(inline, true)
142+
void freeRange(size_t offset, size_t length) {
121143
static if (ownsMemory) {
122144

123145
// Heap allocated items require more smarts.
@@ -127,28 +149,33 @@ private:
127149
// To prevent double-frees this is neccesary.
128150
weak_set!(void*) freed;
129151

130-
foreach_reverse(i; 0..length) {
131-
if (cast(void*)dst[i] in freed)
152+
foreach_reverse(i; offset..offset+length) {
153+
if (cast(void*)memory[i] in freed)
132154
continue;
133155

134-
freed.insert(cast(void*)dst[i]);
156+
freed.insert(cast(void*)memory[i]);
135157

136158
// Basic types don't need destruction,
137159
// so we can skip this step.
138160
static if (!isBasicType!T)
139-
nogc_delete(dst[i]);
140-
dst[i] = null;
161+
nogc_delete(memory[i]);
162+
memory[i] = null;
141163
}
142164

143165
nogc_delete(freed);
144166
} else static if (!isBasicType!T) {
145167

146-
foreach_reverse(i; 0..length)
147-
nogc_delete(dst[i]);
168+
foreach_reverse(i; offset..offset+length)
169+
nogc_delete(memory[i]);
148170
}
171+
149172
}
150173
}
151174

175+
void freeAll() {
176+
this.freeRange(0, size_);
177+
}
178+
152179
public:
153180

154181
/// Gets the type of character stored in the string.
@@ -159,7 +186,7 @@ public:
159186
~this() {
160187
if (this.memory) {
161188
static if (ownsMemory) {
162-
this.freeRange(this.memory, size_);
189+
this.freeRange(0, size_);
163190
}
164191

165192
// Free the pointer
@@ -341,12 +368,7 @@ public:
341368
void clear() {
342369

343370
// Delete elements in the array.
344-
static if (ownsMemory && !isBasicType!T) {
345-
foreach_reverse(item; 0..size_) {
346-
nogc_delete(memory[item]);
347-
}
348-
}
349-
371+
this.freeAll();
350372
this.size_ = 0;
351373
}
352374

@@ -495,6 +517,17 @@ public:
495517
return this;
496518
}
497519

520+
/**
521+
Appends 2 vectors together, creating a new vector.
522+
*/
523+
@trusted
524+
vector!T opBinary(string op: "~", U)(ref auto U items) if (isCompatibleRange!(U, T)) {
525+
vector!T output;
526+
output ~= this;
527+
output ~= items;
528+
return output;
529+
}
530+
498531
/**
499532
Add value to vector
500533
*/
@@ -601,6 +634,11 @@ public:
601634
NuRangeException.sliceOutOfRange(offset, offset+slice.length)
602635
);
603636

637+
enforce(
638+
!this.doSourceOverlap(cast(T*)values.ptr, values.length),
639+
"Overlapping copies are not allowed!"
640+
);
641+
604642
this.copy(cast(T*)slice.ptr, cast(T*)values.ptr, slice.length);
605643
}
606644

@@ -615,6 +653,9 @@ public:
615653
if (offset+values.length > size_)
616654
return false;
617655

656+
if (this.doSourceOverlap(cast(T*)values.ptr, values.length))
657+
return false;
658+
618659
this.copy(cast(T*)memory+offset, cast(T*)values.ptr, values.length);
619660
return true;
620661
}
@@ -696,19 +737,53 @@ unittest {
696737

697738
@("vector: slice overlap")
698739
unittest {
699-
static
700-
struct Test {
701-
@nogc:
702-
static uint counter;
740+
try {
741+
static
742+
struct Test { }
743+
744+
vector!(Test*) tests;
745+
tests ~= nogc_new!Test;
746+
tests ~= nogc_new!Test;
747+
tests ~= nogc_new!Test;
703748

704-
~this() { counter++; }
749+
tests[0..1] = tests[1..2];
750+
} catch(NuException ex) {
751+
ex.free();
752+
return;
705753
}
706754

755+
assert(0, "An exception should've been thrown!");
756+
}
757+
758+
@("vector: insert pointers")
759+
unittest {
760+
static struct Test { int a; }
761+
707762
vector!(Test*) tests;
708-
tests ~= nogc_new!Test;
709-
tests ~= nogc_new!Test;
710-
tests ~= nogc_new!Test;
763+
tests ~= nogc_new!Test(0);
764+
tests ~= nogc_new!Test(1);
765+
tests ~= nogc_new!Test(2);
766+
767+
tests.insert(1, [nogc_new!Test(24), nogc_new!Test(25), nogc_new!Test(26)]);
768+
foreach(i; 0..tests.length) {
769+
foreach(j; 0..tests.length) {
770+
if (i == j) continue;
771+
assert(tests[i] !is tests[j]);
772+
}
773+
}
774+
775+
assert(tests[0].a == 0);
776+
assert(tests[1].a == 24);
777+
assert(tests[2].a == 25);
778+
assert(tests[3].a == 26);
779+
assert(tests[4].a == 1);
780+
assert(tests[5].a == 2);
781+
}
782+
783+
@("vector: append vectors")
784+
unittest {
785+
vector!uint veca = [0, 1, 2, 3, 4];
786+
vector!uint vecb = [0, 1, 2, 3, 4];
711787

712-
tests[0..1] = tests[1..2];
713-
assert(Test.counter == 1);
788+
assert((veca~vecb)[] == [0, 1, 2, 3, 4, 0, 1, 2, 3, 4]);
714789
}

0 commit comments

Comments
 (0)