JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, labath.
Herald added a project: All.
JDevlieghere requested review of this revision.
In D122025 <https://reviews.llvm.org/D122025> Jim suggested that we should 
sanitize the input to the command return object to retain the guarantee that we 
don't have null bytes in the output. This patch implements that through a 
Stream wrapper that escapes null bytes.


https://reviews.llvm.org/D122283

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Stream.h
  lldb/unittests/Utility/StreamTest.cpp

Index: lldb/unittests/Utility/StreamTest.cpp
===================================================================
--- lldb/unittests/Utility/StreamTest.cpp
+++ lldb/unittests/Utility/StreamTest.cpp
@@ -25,16 +25,23 @@
     return result;
   }
 };
-}
+
+struct NullEscapedTest : ::testing::Test {
+  NullEscaped<StreamString> s;
+  std::string TakeValue() {
+    std::string result = s.GetString().str();
+    s.Clear();
+    return result;
+  }
+};
+} // namespace
 
 namespace {
 // A StreamTest where we expect the Stream output to be binary.
 struct BinaryStreamTest : StreamTest {
-  void SetUp() override {
-    s.GetFlags().Set(Stream::eBinary);
-  }
+  void SetUp() override { s.GetFlags().Set(Stream::eBinary); }
 };
-}
+} // namespace
 
 TEST_F(StreamTest, AddressPrefix) {
   DumpAddress(s.AsRawOstream(), 0x1, 1, "foo");
@@ -718,3 +725,21 @@
   EXPECT_EQ("0x6533", TakeValue());
   EXPECT_EQ(6U, bytes);
 }
+
+TEST_F(NullEscapedTest, NonNull) {
+  s.PutChar(' ');
+  EXPECT_EQ(1U, s.GetWrittenBytes());
+  EXPECT_EQ(" ", TakeValue());
+
+  s.PutCString(llvm::StringRef("\0", 1));
+  EXPECT_EQ(2U, s.GetWrittenBytes());
+  EXPECT_EQ("\\0", TakeValue());
+
+  s.PutCString(llvm::StringRef("foo\0bar\0baz", 11));
+  EXPECT_EQ(13U, s.GetWrittenBytes());
+  EXPECT_EQ("foo\\0bar\\0baz", TakeValue());
+
+  s.PutCString(llvm::StringRef("foo\0bar\0baz\0", 12));
+  EXPECT_EQ(15U, s.GetWrittenBytes());
+  EXPECT_EQ("foo\\0bar\\0baz\\0", TakeValue());
+}
Index: lldb/include/lldb/Utility/Stream.h
===================================================================
--- lldb/include/lldb/Utility/Stream.h
+++ lldb/include/lldb/Utility/Stream.h
@@ -412,6 +412,27 @@
   RawOstreamForward m_forwarder;
 };
 
+template <typename T> class NullEscaped : public T {
+public:
+  using T::T;
+  ~NullEscaped() = default;
+
+  static_assert(std::is_base_of<Stream, T>::value, "T must be a Stream");
+
+protected:
+  size_t WriteImpl(const void *s, size_t length) override {
+    llvm::StringRef input(static_cast<const char *>(s), length);
+    llvm::SmallVector<llvm::StringRef, 1> parts;
+    input.split(parts, '\0');
+    size_t written = T::WriteImpl(parts[0].data(), parts[0].size());
+    for (llvm::StringRef part : llvm::drop_begin(parts)) {
+      written += T::WriteImpl("\\0", 2);
+      written += T::WriteImpl(part.data(), part.size());
+    }
+    return written;
+  }
+};
+
 /// Output an address value to this stream.
 ///
 /// Put an address \a addr out to the stream with optional \a prefix and \a
Index: lldb/include/lldb/Interpreter/CommandReturnObject.h
===================================================================
--- lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -158,8 +158,8 @@
 private:
   enum { eStreamStringIndex = 0, eImmediateStreamIndex = 1 };
 
-  StreamTee m_out_stream;
-  StreamTee m_err_stream;
+  NullEscaped<StreamTee> m_out_stream;
+  NullEscaped<StreamTee> m_err_stream;
 
   lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Jonas Devlieghere via Phabricator via lldb-commits

Reply via email to