Skip to content

Commit

Permalink
Fix potential infinite loop when trimming data before stream.
Browse files Browse the repository at this point in the history
Previously we would trigger an infinite loop if one tried to trim before
the head chunk of a stream. In praxis this seem to have been no issue
due to #1549 and us emitting way less calls to trim than possible.

This patch adds an explicit check whether we need to trim anything, and
exits the low-level function early for such cases.

Closes #1554.

(cherry picked from commit bc888f5)
  • Loading branch information
bbannier committed Oct 2, 2023
1 parent 0764193 commit b1d2d91
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
27 changes: 27 additions & 0 deletions hilti/runtime/src/tests/stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,33 @@ TEST_CASE("Trim to beyond end") {
CHECK_EQ(x.view().end().offset(), 10);
}

TEST_CASE("Trim noop") {
auto x = Stream("1"_b);
auto i = x.begin(); // Into first chunk.

x.append("2"_b);
REQUIRE_EQ(x.numberOfChunks(), 2);

auto j = x.begin() + x.size() - 1; // Into second chunk.

x.trim(j); // Drops the first chunk.
CHECK_EQ(x.numberOfChunks(), 1);

// Trimming away data before the range of the stream should be a noop.
x.trim(i);
CHECK_EQ(x.numberOfChunks(), 1);
}

TEST_CASE("Trim empty") {
auto x = Stream();
REQUIRE_EQ(x.numberOfChunks(), 0);

auto i = x.begin();

x.trim(i);
CHECK_EQ(x.numberOfChunks(), 0);
}

TEST_CASE("Block iteration") {
auto content = [](auto b, auto s) -> bool { return memcmp(b->start, s, strlen(s)) == 0; };

Expand Down
13 changes: 13 additions & 0 deletions hilti/runtime/src/types/stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <hilti/rt/extension-points.h>
#include <hilti/rt/types/bytes.h>
#include <hilti/rt/types/stream.h>
#include <hilti/rt/util.h>

using namespace hilti::rt;
using namespace hilti::rt::stream;
Expand Down Expand Up @@ -96,11 +97,19 @@ void Chain::append(Chain&& other) {
void Chain::trim(const Offset& offset) {
_ensureValid();

if ( ! _head || offset < _head->offset() )
// Noop: chain is empty, or offset is before the head of chain; we do
// not need to trim anything.
return;

// We search the first chunk that's containing the desired position,
// deleting all the ones we pass on the way. We trim the one that
// contains the position.
while ( _head ) {
if ( offset >= _head->endOffset() ) {
// Chain should be in order and we progress forward in offset.
assert(! _head->next() || _head->offset() < _head->next()->offset());

// Delete chunk.
_head = std::move(_head->_next);
if ( ! _head || _head->isLast() )
Expand All @@ -112,6 +121,10 @@ void Chain::trim(const Offset& offset) {
assert(_head->offset() == offset);
break;
}

else
// Other offsets are already rejected before entering loop.
cannot_be_reached();
}

_head_offset = offset;
Expand Down

0 comments on commit b1d2d91

Please sign in to comment.