Skip to content

Commit

Permalink
Don't allow pwrite to resize a stream.
Browse files Browse the repository at this point in the history
The current implementations could exhibit some behavior differences:

raw_fd_ostream: Whatever the underlying fd does with seek+write. In a normal
file, the write position would be back to the old offset.

raw_svector_ostream: The write position is always the end of the stream, so
after pwrite the write position would be the new end. This matches what OS_X
(all BSD?) do with a pwrite in a O_APPEND fd.

Given that we don't need that feature and don't use O_APPEND a lot in LLVM,
just disallow it.

I am open to suggestions on renaming pwrite to something else, but this fixes
the issue for now.

Thanks to Yaron Keren for reporting it.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@235303 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
espindola committed Apr 20, 2015
1 parent dbef017 commit f30ac8f
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 15 deletions.
16 changes: 11 additions & 5 deletions include/llvm/Support/raw_ostream.h
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,15 @@ class raw_ostream {
/// pwrite operation. This is usefull for code that can mostly stream out data,
/// but needs to patch in a header that needs to know the output size.
class raw_pwrite_stream : public raw_ostream {
virtual void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) = 0;

public:
explicit raw_pwrite_stream(bool Unbuffered = false)
: raw_ostream(Unbuffered) {}
virtual void pwrite(const char *Ptr, size_t Size, uint64_t Offset) = 0;
void pwrite(const char *Ptr, size_t Size, uint64_t Offset) {
assert(Size + Offset <= tell() && "We don't support extending the stream");
pwrite_impl(Ptr, Size, Offset);
}
};

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -348,6 +353,8 @@ class raw_fd_ostream : public raw_pwrite_stream {
/// See raw_ostream::write_impl.
void write_impl(const char *Ptr, size_t Size) override;

void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) override;

/// Return the current position within the stream, not counting the bytes
/// currently in the buffer.
uint64_t current_pos() const override { return pos; }
Expand Down Expand Up @@ -388,8 +395,6 @@ class raw_fd_ostream : public raw_pwrite_stream {
/// to the offset specified from the beginning of the file.
uint64_t seek(uint64_t off);

void pwrite(const char *Ptr, size_t Size, uint64_t Offset) override;

/// Set the stream to attempt to use atomic writes for individual output
/// routines where possible.
///
Expand Down Expand Up @@ -478,6 +483,8 @@ class raw_svector_ostream : public raw_pwrite_stream {
/// See raw_ostream::write_impl.
void write_impl(const char *Ptr, size_t Size) override;

void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) override;

/// Return the current position within the stream, not counting the bytes
/// currently in the buffer.
uint64_t current_pos() const override;
Expand All @@ -495,7 +502,6 @@ class raw_svector_ostream : public raw_pwrite_stream {
explicit raw_svector_ostream(SmallVectorImpl<char> &O);
~raw_svector_ostream() override;

void pwrite(const char *Ptr, size_t Size, uint64_t Offset) override;

/// This is called when the SmallVector we're appending to is changed outside
/// of the raw_svector_ostream's control. It is only safe to do this if the
Expand All @@ -511,6 +517,7 @@ class raw_svector_ostream : public raw_pwrite_stream {
class raw_null_ostream : public raw_pwrite_stream {
/// See raw_ostream::write_impl.
void write_impl(const char *Ptr, size_t size) override;
void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) override;

/// Return the current position within the stream, not counting the bytes
/// currently in the buffer.
Expand All @@ -519,7 +526,6 @@ class raw_null_ostream : public raw_pwrite_stream {
public:
explicit raw_null_ostream() {}
~raw_null_ostream() override;
void pwrite(const char *Ptr, size_t Size, uint64_t Offset) override;
};

class buffer_ostream : public raw_svector_ostream {
Expand Down
15 changes: 6 additions & 9 deletions lib/Support/raw_ostream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,8 @@ uint64_t raw_fd_ostream::seek(uint64_t off) {
return pos;
}

void raw_fd_ostream::pwrite(const char *Ptr, size_t Size, uint64_t Offset) {
void raw_fd_ostream::pwrite_impl(const char *Ptr, size_t Size,
uint64_t Offset) {
uint64_t Pos = tell();
seek(Offset);
write(Ptr, Size);
Expand Down Expand Up @@ -781,14 +782,9 @@ raw_svector_ostream::~raw_svector_ostream() {
flush();
}

void raw_svector_ostream::pwrite(const char *Ptr, size_t Size,
uint64_t Offset) {
void raw_svector_ostream::pwrite_impl(const char *Ptr, size_t Size,
uint64_t Offset) {
flush();

uint64_t End = Offset + Size;
if (End > OS.size())
OS.resize(End);

memcpy(OS.begin() + Offset, Ptr, Size);
}

Expand Down Expand Up @@ -847,4 +843,5 @@ uint64_t raw_null_ostream::current_pos() const {
return 0;
}

void raw_null_ostream::pwrite(const char *Ptr, size_t Size, uint64_t Offset) {}
void raw_null_ostream::pwrite_impl(const char *Ptr, size_t Size,
uint64_t Offset) {}
10 changes: 9 additions & 1 deletion unittests/Support/raw_pwrite_stream_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,18 @@ using namespace llvm;
namespace {

TEST(raw_pwrite_ostreamTest, TestSVector) {
SmallString<64> Buffer;
SmallVector<char, 0> Buffer;
raw_svector_ostream OS(Buffer);
OS << "abcd";
StringRef Test = "test";
OS.pwrite(Test.data(), Test.size(), 0);
EXPECT_EQ(Test, OS.str());

#ifdef GTEST_HAS_DEATH_TEST
#ifndef NDEBUG
EXPECT_DEATH(OS.pwrite("12345", 5, 0),
"We don't support extending the stream");
#endif
#endif
}
}

0 comments on commit f30ac8f

Please sign in to comment.