Skip to content

Commit

Permalink
Add byte counting mechanism to LLDB's Stream class.
Browse files Browse the repository at this point in the history
Summary:
This patch allows LLDB's Stream class to count the bytes it has written to so far.

There are two major motivations for this patch:

The first one is that this will allow us to get rid of all the handwritten byte counting code
we have in LLDB so far. Examples for this are pretty much all functions in LLDB that
take a Stream to write to and return a size_t, which usually represents the bytes written.

By moving to this centralized byte counting mechanism, we hopefully can avoid some
tricky errors that happen when some code forgets to count the written bytes while
writing something to a stream.

The second motivation is that this is needed for the migration away from LLDB's `Stream`
and towards LLVM's `raw_ostream`. My current plan is to start offering a fake raw_ostream
class that just forwards to a LLDB Stream.

However, for this raw_ostream wrapper we need to fulfill the raw_ostream interface with
LLDB's Stream, which currently lacks the ability to count the bytes written so far (which
raw_ostream exposes by it's `tell()` method). By adding this functionality it is trivial to start
rolling out our raw_ostream wrapper (and then eventually completely move to raw_ostream).

Also, once this fake raw_ostream is available, we can start replacing our own code writing
to LLDB's Stream by LLVM code writing to raw_ostream. The best example for this is the
LEB128 encoding we currently ship, which can be replaced with by LLVM's version which
accepts an raw_ostream.

From the point of view of the pure source changes this test does, we essentially just renamed
the Write implementation in Stream to `WriteImpl` while the `Write` method everyone is using
to write its raw bytes is now just forwarding and counting the written bytes.

Reviewers: labath, davide

Reviewed By: labath

Subscribers: JDevlieghere, lldb-commits

Differential Revision: https://reviews.llvm.org/D50159

llvm-svn: 338733
  • Loading branch information
Teemperor committed Aug 2, 2018
1 parent 98768e4 commit 92b1673
Show file tree
Hide file tree
Showing 11 changed files with 168 additions and 39 deletions.
3 changes: 2 additions & 1 deletion lldb/include/lldb/Core/StreamAsynchronousIO.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class StreamAsynchronousIO : public Stream {

void Flush() override;

size_t Write(const void *src, size_t src_len) override;
protected:
size_t WriteImpl(const void *src, size_t src_len) override;

private:
Debugger &m_debugger;
Expand Down
12 changes: 6 additions & 6 deletions lldb/include/lldb/Core/StreamBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ template <unsigned N> class StreamBuffer : public Stream {
// Nothing to do when flushing a buffer based stream...
}

virtual size_t Write(const void *s, size_t length) {
if (s && length)
m_packet.append((const char *)s, ((const char *)s) + length);
return length;
}

void Clear() { m_packet.clear(); }

// Beware, this might not be NULL terminated as you can expect from
Expand All @@ -48,6 +42,12 @@ template <unsigned N> class StreamBuffer : public Stream {

protected:
llvm::SmallVector<char, N> m_packet;

virtual size_t WriteImpl(const void *s, size_t length) {
if (s && length)
m_packet.append((const char *)s, ((const char *)s) + length);
return length;
}
};

} // namespace lldb_private
Expand Down
2 changes: 1 addition & 1 deletion lldb/include/lldb/Core/StreamFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ class StreamFile : public Stream {

void Flush() override;

size_t Write(const void *s, size_t length) override;

protected:
//------------------------------------------------------------------
// Classes that inherit from StreamFile can see and modify these
//------------------------------------------------------------------
File m_file;
size_t WriteImpl(const void *s, size_t length) override;

private:
DISALLOW_COPY_AND_ASSIGN(StreamFile);
Expand Down
25 changes: 24 additions & 1 deletion lldb/include/lldb/Utility/Stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,13 @@ class Stream {
/// @return
/// The number of bytes that were appended to the stream.
//------------------------------------------------------------------
virtual size_t Write(const void *src, size_t src_len) = 0;
size_t Write(const void *src, size_t src_len) {
size_t appended_byte_count = WriteImpl(src, src_len);
m_bytes_written += appended_byte_count;
return appended_byte_count;
}

size_t GetWrittenBytes() const { return m_bytes_written; }

//------------------------------------------------------------------
// Member functions
Expand Down Expand Up @@ -523,8 +529,25 @@ class Stream {
lldb::ByteOrder
m_byte_order; ///< Byte order to use when encoding scalar types.
int m_indent_level; ///< Indention level.
std::size_t m_bytes_written = 0; ///< Number of bytes written so far.

size_t _PutHex8(uint8_t uvalue, bool add_prefix);

//------------------------------------------------------------------
/// Output character bytes to the stream.
///
/// Appends \a src_len characters from the buffer \a src to the stream.
///
/// @param[in] src
/// A buffer containing at least \a src_len bytes of data.
///
/// @param[in] src_len
/// A number of bytes to append to the stream.
///
/// @return
/// The number of bytes that were appended to the stream.
//------------------------------------------------------------------
virtual size_t WriteImpl(const void *src, size_t src_len) = 0;
};

} // namespace lldb_private
Expand Down
3 changes: 1 addition & 2 deletions lldb/include/lldb/Utility/StreamString.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class StreamString : public Stream {

void Flush() override;

size_t Write(const void *s, size_t length) override;

void Clear();

bool Empty() const;
Expand All @@ -49,6 +47,7 @@ class StreamString : public Stream {

protected:
std::string m_packet;
size_t WriteImpl(const void *s, size_t length) override;
};

} // namespace lldb_private
Expand Down
46 changes: 23 additions & 23 deletions lldb/include/lldb/Utility/StreamTee.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,29 +70,6 @@ class StreamTee : public Stream {
}
}

size_t Write(const void *s, size_t length) override {
std::lock_guard<std::recursive_mutex> guard(m_streams_mutex);
if (m_streams.empty())
return 0;

size_t min_bytes_written = SIZE_MAX;
collection::iterator pos, end;
for (pos = m_streams.begin(), end = m_streams.end(); pos != end; ++pos) {
// Allow for our collection to contain NULL streams. This allows the
// StreamTee to be used with hard coded indexes for clients that might
// want N total streams with only a few that are set to valid values.
Stream *strm = pos->get();
if (strm) {
const size_t bytes_written = strm->Write(s, length);
if (min_bytes_written > bytes_written)
min_bytes_written = bytes_written;
}
}
if (min_bytes_written == SIZE_MAX)
return 0;
return min_bytes_written;
}

size_t AppendStream(const lldb::StreamSP &stream_sp) {
size_t new_idx = m_streams.size();
std::lock_guard<std::recursive_mutex> guard(m_streams_mutex);
Expand Down Expand Up @@ -131,6 +108,29 @@ class StreamTee : public Stream {
typedef std::vector<lldb::StreamSP> collection;
mutable std::recursive_mutex m_streams_mutex;
collection m_streams;

size_t WriteImpl(const void *s, size_t length) override {
std::lock_guard<std::recursive_mutex> guard(m_streams_mutex);
if (m_streams.empty())
return 0;

size_t min_bytes_written = SIZE_MAX;
collection::iterator pos, end;
for (pos = m_streams.begin(), end = m_streams.end(); pos != end; ++pos) {
// Allow for our collection to contain NULL streams. This allows the
// StreamTee to be used with hard coded indexes for clients that might
// want N total streams with only a few that are set to valid values.
Stream *strm = pos->get();
if (strm) {
const size_t bytes_written = strm->Write(s, length);
if (min_bytes_written > bytes_written)
min_bytes_written = bytes_written;
}
}
if (min_bytes_written == SIZE_MAX)
return 0;
return min_bytes_written;
}
};

} // namespace lldb_private
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/StreamAsynchronousIO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void StreamAsynchronousIO::Flush() {
}
}

size_t StreamAsynchronousIO::Write(const void *s, size_t length) {
size_t StreamAsynchronousIO::WriteImpl(const void *s, size_t length) {
m_data.append((const char *)s, length);
return length;
}
2 changes: 1 addition & 1 deletion lldb/source/Core/StreamFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ StreamFile::~StreamFile() {}

void StreamFile::Flush() { m_file.Flush(); }

size_t StreamFile::Write(const void *s, size_t length) {
size_t StreamFile::WriteImpl(const void *s, size_t length) {
m_file.Write(s, length);
return length;
}
7 changes: 5 additions & 2 deletions lldb/source/Utility/StreamString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ void StreamString::Flush() {
// Nothing to do when flushing a buffer based stream...
}

size_t StreamString::Write(const void *s, size_t length) {
size_t StreamString::WriteImpl(const void *s, size_t length) {
m_packet.append(reinterpret_cast<const char *>(s), length);
return length;
}

void StreamString::Clear() { m_packet.clear(); }
void StreamString::Clear() {
m_packet.clear();
m_bytes_written = 0;
}

bool StreamString::Empty() const { return GetSize() == 0; }

Expand Down
4 changes: 3 additions & 1 deletion lldb/unittests/Utility/StreamTeeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ namespace {
void Flush() override {
++m_flush_count;
}
size_t Write(const void *src, size_t src_len) override { return src_len; }
size_t WriteImpl(const void *src, size_t src_len) override {
return src_len;
}
};
}

Expand Down
Loading

0 comments on commit 92b1673

Please sign in to comment.