8225225: stringStream internal buffer should always be zero terminated
Reviewed-by: coleenp, dholmes
--- 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
--- 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 {
--- 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) {