From: Rafael Espindola Date: Mon, 20 Apr 2015 13:04:30 +0000 (+0000) Subject: Don't allow pwrite to resize a stream. X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=f30ac8f5ece0c90d804cd6ddec142ac05e1d32f1;p=oota-llvm.git Don't allow pwrite to resize a stream. 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 --- diff --git a/include/llvm/Support/raw_ostream.h b/include/llvm/Support/raw_ostream.h index 338f0f4295c..ed9c8b09ba4 100644 --- a/include/llvm/Support/raw_ostream.h +++ b/include/llvm/Support/raw_ostream.h @@ -317,10 +317,15 @@ private: /// 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); + } }; //===----------------------------------------------------------------------===// @@ -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; } @@ -388,8 +395,6 @@ public: /// 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. /// @@ -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; @@ -495,7 +502,6 @@ public: explicit raw_svector_ostream(SmallVectorImpl &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 @@ -511,6 +517,7 @@ public: 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. @@ -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 { diff --git a/lib/Support/raw_ostream.cpp b/lib/Support/raw_ostream.cpp index 6f9f910cad9..4c0b6c7b563 100644 --- a/lib/Support/raw_ostream.cpp +++ b/lib/Support/raw_ostream.cpp @@ -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); @@ -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); } @@ -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) {} diff --git a/unittests/Support/raw_pwrite_stream_test.cpp b/unittests/Support/raw_pwrite_stream_test.cpp index bcbb29b86bd..2792f44b605 100644 --- a/unittests/Support/raw_pwrite_stream_test.cpp +++ b/unittests/Support/raw_pwrite_stream_test.cpp @@ -16,10 +16,18 @@ using namespace llvm; namespace { TEST(raw_pwrite_ostreamTest, TestSVector) { - SmallString<64> Buffer; + SmallVector 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 } }