lee/renderdoc
1
0
Fork 0
mirror of https://github.com/baldurk/renderdoc.git synced 2026-07-17 21:17:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix bug in Intervals<T>::update()

Browse Source 
When the interval being updated went all the way to the end of the last
interval (UINT64_MAX), `update` would call `mergeLeft` on the `end()` interval.
This would cause `mergeLeft` to dereference an invalid iterator.

Also added a quick return for updating empty intervals, and unit tests to verify
correct handling of empty intervals, which were run both with and without this
optimization.

Also removed some unnecessary asserts that likely called many times. These
asserts could only fail if there was a bug in the `Intervals<T>` implementation,
which should be caught by the unit tests.
This commit is contained in:
Benson Joeris
2019-02-19 10:35:47 -05:00
committed by Baldur Karlsson
parent a7b21ae2d2
commit cb26815570
2 changed files with 34 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
renderdoc/core/intervals.h
+5 -10
View File
@@ -80,7 +80,6 @@ public:
// If `x == start`, then `split(x)` is a no-op.
inline void split(uint64_t x)
{
RDCASSERT(this->start() <= x && x < this->finish());
if(this->start() < x)
this->iter = this->owner->insert(std::pair<uint64_t, T>(x, this->value())).first;
}
@@ -194,7 +193,6 @@ public:
{
// Find the first interval starting after `x`; return the preceding interval.
auto it = StartPoints.upper_bound(x);
RDCASSERT(it != StartPoints.begin());
it--;
return Wrap(it);
}
@@ -204,7 +202,6 @@ public:
{
// Find the first interval starting after `x`; return the preceding interval.
auto it = StartPoints.upper_bound(x);
RDCASSERT(it != StartPoints.begin());
it--;
return Wrap(it);
}
@@ -216,6 +213,9 @@ public:
template <typename Compose>
void update(uint64_t start, uint64_t finish, T val, Compose comp)
{
if(finish <= start)
return;
auto i = find(start);
// Split the interval so that `i.start == start`
@@ -240,7 +240,8 @@ public:
// `i` now points to the interval following the last interval whose value was
// modified; merge `i` with that last modified interval, if the values match.
i->mergeLeft();
if(i != end())
i->mergeLeft();
}
// Update `this` by composing the value of each interval with the value of the
@@ -262,8 +263,6 @@ public:
// * i.start() < j.end()
while(true)
{
RDCASSERT(i->start() >= j->start());
RDCASSERT(i->start() < j->finish());
if(i->finish() > j->finish())
{
i->split(j->finish());
@@ -282,11 +281,7 @@ public:
// in `other`, if necessary to maintain the invariant `i.start < j.end`.
i++;
if(i == end())
{
j++;
RDCASSERT(j == other.end());
return;
}
if(i->start() >= j->finish())
j++;
}
renderdoc/core/intervals_tests.cpp
+29
View File
@@ -265,6 +265,35 @@ TEST_CASE("Test Intervals type", "[intervals]")
test.update(0, UINT64_MAX, 1, [](uint64_t x, uint64_t y) -> uint64_t { return x + y; });
check_intervals(test, {{0, 1, 5}, {5, 2, 10}, {10, 1, UINT64_MAX}});
};
SECTION("update an empty interval in the interior of an interval")
{
Intervals<uint64_t> test = make_intervals({{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
test.update(2, 2, 1, [](uint64_t x, uint64_t y) -> uint64_t { return x + y; });
check_intervals(test, {{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
};
SECTION("update an empty interval on a boundary")
{
Intervals<uint64_t> test = make_intervals({{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
test.update(5, 5, 1, [](uint64_t x, uint64_t y) -> uint64_t { return x + y; });
check_intervals(test, {{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
};
SECTION("update an empty interval at 0")
{
Intervals<uint64_t> test = make_intervals({{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
test.update(0, 0, 1, [](uint64_t x, uint64_t y) -> uint64_t { return x + y; });
check_intervals(test, {{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
};
SECTION("update an empty interval at UINT64_MAX")
{
Intervals<uint64_t> test = make_intervals({{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
test.update(UINT64_MAX, UINT64_MAX, 1,
[](uint64_t x, uint64_t y) -> uint64_t { return x + y; });
check_intervals(test, {{0, 0, 5}, {5, 1, 10}, {10, 0, UINT64_MAX}});
};
};
SECTION("mergeIntervals tests")