# HG changeset patch
# User stuefe
# Date 1559756083 -7200
# Node ID c87e52dbdca07d4697fb2d3a20a429d9f6191617
# Parent  cad7e13ca5876df109422aa6bf0f0fc25f85cd0a
8225225: stringStream internal buffer should always be zero terminated
Reviewed-by: coleenp, dholmes

diff -r cad7e13ca587 -r c87e52dbdca0 src/hotspot/share/utilities/ostream.cpp
--- a/src/hotspot/share/utilities/ostream.cpp	Wed Jun 05 08:43:41 2019 -0700
+++ b/src/hotspot/share/utilities/ostream.cpp	Wed Jun 05 19:34:43 2019 +0200
@@ -312,6 +312,7 @@
   buffer        = NEW_C_HEAP_ARRAY(char, buffer_length, mtInternal);
   buffer_pos    = 0;
   buffer_fixed  = false;
+  zero_terminate();
 }
 
 // useful for output to fixed chunks of memory, such as performance counters
@@ -320,6 +321,7 @@
   buffer        = fixed_buffer;
   buffer_pos    = 0;
   buffer_fixed  = true;
+  zero_terminate();
 }
 
 void stringStream::write(const char* s, size_t len) {
@@ -343,9 +345,9 @@
   // invariant: buffer is always null-terminated
   guarantee(buffer_pos + write_len + 1 <= buffer_length, "stringStream oob");
   if (write_len > 0) {
-    buffer[buffer_pos + write_len] = 0;
     memcpy(buffer + buffer_pos, s, write_len);
     buffer_pos += write_len;
+    zero_terminate();
   }
 
   // Note that the following does not depend on write_len.
@@ -354,7 +356,18 @@
   update_position(s, len);
 }
 
-char* stringStream::as_string() {
+void stringStream::zero_terminate() {
+  assert(buffer != NULL &&
+         buffer_pos < buffer_length, "sanity");
+  buffer[buffer_pos] = '\0';
+}
+
+void stringStream::reset() {
+  buffer_pos = 0; _precount = 0; _position = 0;
+  zero_terminate();
+}
+
+char* stringStream::as_string() const {
   char* copy = NEW_RESOURCE_ARRAY(char, buffer_pos + 1);
   strncpy(copy, buffer, buffer_pos);
   copy[buffer_pos] = 0;  // terminating null
diff -r cad7e13ca587 -r c87e52dbdca0 src/hotspot/share/utilities/ostream.hpp
--- a/src/hotspot/share/utilities/ostream.hpp	Wed Jun 05 08:43:41 2019 -0700
+++ b/src/hotspot/share/utilities/ostream.hpp	Wed Jun 05 19:34:43 2019 +0200
@@ -198,6 +198,10 @@
   size_t buffer_pos;
   size_t buffer_length;
   bool   buffer_fixed;
+
+  // zero terminate at buffer_pos.
+  void zero_terminate();
+
  public:
   // Create a stringStream using an internal buffer of initially initial_bufsize size;
   // will be enlarged on demand. There is no maximum cap.
@@ -209,10 +213,10 @@
   virtual void write(const char* c, size_t len);
   // Return number of characters written into buffer, excluding terminating zero and
   // subject to truncation in static buffer mode.
-  size_t      size() { return buffer_pos; }
-  const char* base() { return buffer; }
-  void  reset() { buffer_pos = 0; _precount = 0; _position = 0; }
-  char* as_string();
+  size_t      size() const { return buffer_pos; }
+  const char* base() const { return buffer; }
+  void  reset();
+  char* as_string() const;
 };
 
 class fileStream : public outputStream {
diff -r cad7e13ca587 -r c87e52dbdca0 test/hotspot/gtest/utilities/test_ostream.cpp
--- a/test/hotspot/gtest/utilities/test_ostream.cpp	Wed Jun 05 08:43:41 2019 -0700
+++ b/test/hotspot/gtest/utilities/test_ostream.cpp	Wed Jun 05 19:34:43 2019 +0200
@@ -49,37 +49,66 @@
   return len;
 }
 
-static void do_test_stringStream_dynamic_realloc(bool short_len) {
-  stringStream ss(2); // small buffer to force lots of reallocations.
+static void test_stringStream_is_zero_terminated(const stringStream* ss) {
+  ASSERT_EQ(ss->base()[ss->size()], '\0');
+}
+
+
+static void do_test_stringStream(stringStream* ss, bool short_len, size_t expected_cap) {
+  test_stringStream_is_zero_terminated(ss);
   size_t written = 0;
   for (int i = 0; i < 1000; i ++) {
-    written += print_lorem(&ss, short_len);
-    ASSERT_EQ(ss.size(), written);
+    written += print_lorem(ss, short_len);
+    if (expected_cap > 0 && written >= expected_cap) {
+      ASSERT_EQ(ss->size(), expected_cap - 1);
+    } else {
+      ASSERT_EQ(ss->size(), written);
+    }
     // Internal buffer should always be zero-terminated.
-    ASSERT_EQ(ss.base()[ss.size()], '\0');
+    test_stringStream_is_zero_terminated(ss);
   }
+  // Reset should zero terminate too
+  ss->reset();
+  ASSERT_EQ(ss->size(), (size_t)0);
+  test_stringStream_is_zero_terminated(ss);
 }
 
 TEST_VM(ostream, stringStream_dynamic_realloc_1) {
-  do_test_stringStream_dynamic_realloc(false);
+  stringStream ss(2); // dynamic buffer with very small starting size
+  do_test_stringStream(&ss, false, 0);
 }
 
 TEST_VM(ostream, stringStream_dynamic_realloc_2) {
-  do_test_stringStream_dynamic_realloc(true);
+  stringStream ss(2); // dynamic buffer with very small starting size
+  do_test_stringStream(&ss, true, 0);
+}
+
+TEST_VM(ostream, stringStream_static) {
+  char buffer[128 + 1];
+  char* canary_at = buffer + sizeof(buffer) - 1;
+  *canary_at = 'X';
+  size_t stream_buf_size = sizeof(buffer) - 1;
+  stringStream ss(buffer, stream_buf_size);
+  do_test_stringStream(&ss, false, stream_buf_size);
+  ASSERT_EQ(*canary_at, 'X'); // canary
 }
 
 TEST_VM(ostream, bufferedStream_static) {
-  char buf[100];
-  bufferedStream bs(buf, sizeof(buf));
+  char buf[100 + 1];
+  char* canary_at = buf + sizeof(buf) - 1;
+  *canary_at = 'X';
+  size_t stream_buf_size = sizeof(buf) - 1;
+  bufferedStream bs(buf, stream_buf_size);
   size_t written = 0;
   for (int i = 0; i < 100; i ++) {
     written += print_lorem(&bs, true);
-    if (written < sizeof(buf)) {
+    if (written < stream_buf_size) {
       ASSERT_EQ(bs.size(), written);
     } else {
-      ASSERT_EQ(bs.size(), sizeof(buf) - 1);
+      ASSERT_EQ(bs.size(), stream_buf_size - 1);
     }
   }
+  ASSERT_EQ(*canary_at, 'X'); // canary
 }
 
 TEST_VM(ostream, bufferedStream_dynamic_small) {