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.

llvm-svn: 235303

diff --git a/llvm/include/llvm/Support/raw_ostream.h b/llvm/include/llvm/Support/raw_ostream.h
index 338f0f4..ed9c8b0 100644 (file)
--- a/llvm/include/llvm/Support/raw_ostream.h
+++ b/llvm/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<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
@@ -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/llvm/lib/Support/raw_ostream.cpp b/llvm/lib/Support/raw_ostream.cpp
index 6f9f910..4c0b6c7 100644 (file)
--- a/llvm/lib/Support/raw_ostream.cpp
+++ b/llvm/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/llvm/unittests/Support/raw_pwrite_stream_test.cpp b/llvm/unittests/Support/raw_pwrite_stream_test.cpp
index bcbb29b..2792f44 100644 (file)
--- a/llvm/unittests/Support/raw_pwrite_stream_test.cpp
+++ b/llvm/unittests/Support/raw_pwrite_stream_test.cpp
@@ -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
 }
 }