# HG changeset patch # User stuefe # Date 1561649001 -7200 # Thu Jun 27 17:23:21 2019 +0200 # Node ID 0aa31af4e42f5bc9516e2178f3b52fc3d408c38b # Parent 91b38bfb9079f6a77383748152dc7747454c3e00 8224213: various stringStream issues diff -r 91b38bfb9079 -r 0aa31af4e42f src/hotspot/share/utilities/ostream.cpp --- a/src/hotspot/share/utilities/ostream.cpp Thu Jun 27 18:00:54 2019 +0800 +++ b/src/hotspot/share/utilities/ostream.cpp Thu Jun 27 17:23:21 2019 +0200 @@ -307,46 +307,130 @@ } } -stringStream::stringStream(size_t initial_size) : outputStream() { - buffer_length = initial_size; - buffer = NEW_C_HEAP_ARRAY(char, buffer_length, mtInternal); - buffer_pos = 0; - buffer_fixed = false; +// Allocate backing buffer. In fixed mode this is a noop since we work on a caller +// provided buffer. In dynamic mode, we attempt to use the _init_buffer for small +// allocations but will use C-heap otherwise; we will not grow beyond _max_dynamic_size. +void stringStream::ensure_backing_buffer_size(size_t requested_size) { + + if (_buffer_length >= requested_size) { + return; // Nothing to do. + } + + if (_fixed) { + return; // Nothing we can do. + } + + if (_buffer == NULL) { + + // On first allocation, use _init_buffer if possible, C-heap otherwise, + // but cap out at max dynamic size. + + assert(_buffer_length == 0 && _buffer_pos == 0, "Sanity"); + + if (requested_size <= sizeof(_init_buffer)) { + _buffer = _init_buffer; + _buffer_length = sizeof(_init_buffer); // Note: may be larger than requested but this is okay. + } else { + size_t allowed_size = MIN2(requested_size, _max_dynamic_size); + _buffer = NEW_C_HEAP_ARRAY(char, allowed_size, mtInternal); + _buffer_length = allowed_size; + } + + } else { + + // Subsequent resizes: use C-heap exclusively and cap out at max dynamic size. + + assert(_buffer_length > 0 && + _buffer_length <= _max_dynamic_size && + _buffer_pos <= _buffer_length, "Sanity"); + + // Always grow by at least by factor 2... + size_t grow_size = MAX2(requested_size, _buffer_length * 2); + + // ..but cap out at max dynamic size. + size_t allowed_size = MIN2(grow_size, _max_dynamic_size); + + if (_buffer == _init_buffer) { + // init-buffer -> C heap + assert(_buffer_length == sizeof(_init_buffer), "Sanity"); + _buffer = NEW_C_HEAP_ARRAY(char, allowed_size, mtInternal); + _buffer_length = allowed_size; + memcpy(_buffer, _init_buffer, _buffer_pos); + } else { + // C heap -> C heap + _buffer = REALLOC_C_HEAP_ARRAY(char, _buffer, allowed_size, mtInternal); + _buffer_length = allowed_size; + } + } +} + +void stringStream::free_backing_buffer() { + + if (_fixed == false && + _buffer != _init_buffer && + _buffer != NULL) { + FREE_C_HEAP_ARRAY(char, _buffer); + } +} + + +stringStream::stringStream(size_t initial_size, size_t max_dynamic_size) + : outputStream(), + _buffer(NULL), + _buffer_pos(0), + _buffer_length(0), + _fixed(false), + _max_dynamic_size(max_dynamic_size), + _truncated(false) +{ + // For simplicity reasons, we do not allow max_dynamic_size + // to be smaller than the initial buffer size nor the initial + // allocation size. + assert(max_dynamic_size >= initial_size && + max_dynamic_size >= sizeof(_init_buffer), + "max_dynamic_size " SIZE_FORMAT ", initial_size " SIZE_FORMAT + ", init buffer size " SIZE_FORMAT, + max_dynamic_size, initial_size, sizeof(_init_buffer)); + ensure_backing_buffer_size(initial_size); zero_terminate(); } // useful for output to fixed chunks of memory, such as performance counters -stringStream::stringStream(char* fixed_buffer, size_t fixed_buffer_size) : outputStream() { - buffer_length = fixed_buffer_size; - buffer = fixed_buffer; - buffer_pos = 0; - buffer_fixed = true; +stringStream::stringStream(char* fixed_buffer, size_t fixed_buffer_size) + : outputStream(), + _buffer(fixed_buffer), + _buffer_pos(0), + _buffer_length(fixed_buffer_size), + _fixed(true), + _max_dynamic_size(0), + _truncated(false) +{ zero_terminate(); } void stringStream::write(const char* s, size_t len) { - size_t write_len = len; // number of non-null bytes to write - size_t end = buffer_pos + len + 1; // position after write and final '\0' - if (end > buffer_length) { - if (buffer_fixed) { - // if buffer cannot resize, silently truncate - end = buffer_length; - write_len = end - buffer_pos - 1; // leave room for the final '\0' - } else { - // For small overruns, double the buffer. For larger ones, - // increase to the requested size. - if (end < buffer_length * 2) { - end = buffer_length * 2; - } - buffer = REALLOC_C_HEAP_ARRAY(char, buffer, end, mtInternal); - buffer_length = end; - } + + if (_truncated) { + return; } + + size_t needed = _buffer_pos + len + 1; + + ensure_backing_buffer_size(needed); + + // If we were not able to expand the buffer to contain the whole string, + // write whatever fits and mark stream as truncated. + size_t write_len = len; + if (needed > _buffer_length) { + _truncated = true; + write_len = _buffer_length - _buffer_pos - 1; + } + // invariant: buffer is always null-terminated - guarantee(buffer_pos + write_len + 1 <= buffer_length, "stringStream oob"); + guarantee(_buffer_pos + write_len + 1 <= _buffer_length, "stringStream oob"); if (write_len > 0) { - memcpy(buffer + buffer_pos, s, write_len); - buffer_pos += write_len; + memcpy(_buffer + _buffer_pos, s, write_len); + _buffer_pos += write_len; zero_terminate(); } @@ -354,30 +438,29 @@ // This means that position and count get updated // even when overflow occurs. update_position(s, len); + } void stringStream::zero_terminate() { - assert(buffer != NULL && - buffer_pos < buffer_length, "sanity"); - buffer[buffer_pos] = '\0'; + assert(_buffer != NULL && _buffer_pos < _buffer_length, "sanity"); + _buffer[_buffer_pos] = '\0'; } void stringStream::reset() { - buffer_pos = 0; _precount = 0; _position = 0; + _buffer_pos = 0; _precount = 0; _position = 0; + _truncated = false; 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 + char* copy = NEW_RESOURCE_ARRAY(char, _buffer_pos + 1); + strncpy(copy, _buffer, _buffer_pos); + copy[_buffer_pos] = 0; // terminating null return copy; } stringStream::~stringStream() { - if (buffer_fixed == false && buffer != NULL) { - FREE_C_HEAP_ARRAY(char, buffer); - } + free_backing_buffer(); } xmlStream* xtty; diff -r 91b38bfb9079 -r 0aa31af4e42f src/hotspot/share/utilities/ostream.hpp --- a/src/hotspot/share/utilities/ostream.hpp Thu Jun 27 18:00:54 2019 +0800 +++ b/src/hotspot/share/utilities/ostream.hpp Thu Jun 27 17:23:21 2019 +0200 @@ -190,33 +190,67 @@ } }; -// for writing to strings; buffer will expand automatically. -// Buffer will always be zero-terminated. +// stringStream: stream class for writing to char buffers. +// +// stringStream has two modes: +// - "static" (fixed) where it works on a caller provided buffer. When that buffer is full +// any subsequent output is silently ignored. +// - "dynamic" where it works on an internal buffer which is allocated on C heap. The buffer +// will be expanded automatically until a maximum is reached (by default very large, 100M). +// Any subsequent output will be silently ignored. +// +// zero termination is always guaranteed, also upon truncation. class stringStream : public outputStream { - protected: - char* buffer; - size_t buffer_pos; - size_t buffer_length; - bool buffer_fixed; + + char* _buffer; + size_t _buffer_pos; + size_t _buffer_length; + + const bool _fixed; + + static const size_t _initial_dynamic_size_default = 64; + static const size_t _max_dynamic_size_default = 100 * M; + + // In dynamic mode, we use a small internal buffer to avoid C heap allocations for short texts. + char _init_buffer[_initial_dynamic_size_default]; + + // In dynamic mode, max size of backing buffer. + const size_t _max_dynamic_size; + + // true if stream truncated. + bool _truncated; // zero terminate at buffer_pos. void zero_terminate(); + // Allocate backing buffer. In fixed mode this is a no-op since we work on a caller + // provided buffer. In dynamic mode, we attempt to use the _init_buffer for small + // allocations but will use C-heap otherwise; we will not grow beyond _max_dynamic_size. + void ensure_backing_buffer_size(size_t requested_size); + void free_backing_buffer(); + public: + // Create a stringStream using an internal buffer of initially initial_bufsize size; - // will be enlarged on demand. There is no maximum cap. - stringStream(size_t initial_bufsize = 256); - // Creates a stringStream using a caller-provided buffer. Will truncate silently if - // it overflows. + // will be enlarged on demand. Will silently truncate beyond max_dynamic_size. + stringStream(size_t initial_bufsize = _initial_dynamic_size_default, + size_t max_dynamic_size = _max_dynamic_size_default); + + // Creates a stringStream using a caller-provided buffer ("static" mode). Will truncate + // silently if it overflows. stringStream(char* fixed_buffer, size_t fixed_buffer_size); + ~stringStream(); 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() const { return buffer_pos; } - const char* base() const { return buffer; } + size_t size() const { return _buffer_pos; } + const char* base() const { return _buffer; } void reset(); char* as_string() const; + + bool truncated() const { return _truncated; } + }; class fileStream : public outputStream { diff -r 91b38bfb9079 -r 0aa31af4e42f test/hotspot/gtest/utilities/test_ostream.cpp --- a/test/hotspot/gtest/utilities/test_ostream.cpp Thu Jun 27 18:00:54 2019 +0800 +++ b/test/hotspot/gtest/utilities/test_ostream.cpp Thu Jun 27 17:23:21 2019 +0200 @@ -26,71 +26,187 @@ #include "precompiled.hpp" #include "memory/resourceArea.hpp" #include "runtime/os.hpp" +#include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/ostream.hpp" #include "unittest.hpp" + +static const char* const lorem = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, " + "sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Lacinia at quis " + "risus sed vulputate odio ut enim blandit. Amet risus nullam eget felis eget. Viverra " + "orci sagittis eu volutpat odio facilisis mauris sit. Erat velit scelerisque in dictum non."; + +// Prints a text to a stream and remembers what it did print; +// can later check result string against expectations. +class LoremPrinter { + outputStream* const _st; + size_t _pos; +public: + LoremPrinter(outputStream* st) : _st(st), _pos(0) {} + + // Print len characters + void print(size_t len) { + const size_t lorem_len = ::strlen(lorem); + assert(_pos < lorem_len, "oob"); + size_t to_write = len; + while (to_write > 0) { + size_t l = MIN2(lorem_len - _pos, to_write); + { + // Create a ResourceMark as a regression test for 8224193. + // If the stream uses RA and happens to resize, we will assert. + ResourceMark rm; + _st->write(lorem + _pos, l); + } + to_write -= l; + _pos += l; + if (_pos == lorem_len) { + _pos = 0; + } + } + } + + // check a string against expectations. + bool check(const char* s, size_t len) const { + size_t to_check = len; + const char* p = s; + while (to_check > 0) { + size_t l = MIN2(strlen(lorem), to_check); + if (memcmp(lorem, p, l) != 0) { + printf("Error checking at " PTR_FORMAT ".", p2i(p)); + return false; + } + to_check -= l; + } + return true; + } + +}; + static size_t print_lorem(outputStream* st, bool short_len) { - // Create a ResourceMark just to make sure the stream does not use ResourceArea - ResourceMark rm; - static const char* const lorem = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, " - "sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Lacinia at quis " - "risus sed vulputate odio ut enim blandit. Amet risus nullam eget felis eget. Viverra " - "orci sagittis eu volutpat odio facilisis mauris sit. Erat velit scelerisque in dictum non."; - static const size_t len_lorem = strlen(lorem); - size_t len; - if (short_len) { - len = os::random() % 10; - } else { - len = MAX2(1, (int)(os::random() % len_lorem)); - } - st->write(lorem, len); + LoremPrinter lp(st); + size_t len = os::random() % (short_len ? 10 : 128) + 1; + lp.print(len); return len; } +// returns s +- random(max_wobble/2) +static size_t random_wobble(size_t s, size_t max_wobble) { + assert(s > max_wobble, "too much wobble"); + return s + (os::random() % max_wobble) - (max_wobble / 2); +} + +// returns s +- random(s/2) +static size_t random_wobble(size_t s) { + return random_wobble(s, s/2); +} + static void test_stringStream_is_zero_terminated(const stringStream* ss) { ASSERT_EQ(ss->base()[ss->size()], '\0'); } +static void do_test_stringStream(stringStream* ss, size_t max_size, size_t avg_write_len) { -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); - if (expected_cap > 0 && written >= expected_cap) { - ASSERT_EQ(ss->size(), expected_cap - 1); + // Print enough to saturate stream beyond truncation + int num_iterations = 1 + (max_size / avg_write_len) * 3; + + LoremPrinter lp(ss); + + for (int i = 0; i < num_iterations; i ++) { + + size_t write_len = random_wobble(avg_write_len); + lp.print(write_len); + written += write_len; + + if (written >= max_size) { + ASSERT_EQ(ss->size(), max_size - 1); + ASSERT_TRUE(ss->truncated()); } else { ASSERT_EQ(ss->size(), written); + ASSERT_FALSE(ss->truncated()); } + + ASSERT_TRUE(lp.check(ss->base(), ss->size())); + // Internal buffer should always be zero-terminated. 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); + +} + +static void do_test_dynamic_stringStream(size_t initial_size, size_t max_size, size_t avg_write_len) { + + stringStream ss(initial_size, max_size); + do_test_stringStream(&ss, max_size, avg_write_len); + +} + +static void do_test_static_stringStream(size_t buffer_size, size_t avg_write_len) { + const char canary1 = 'X'; + const char canary2 = 'x'; + char* p = NEW_C_HEAP_ARRAY(char, buffer_size + 2, mtInternal); + p[0] = canary1; + p[buffer_size + 1] = canary2; + stringStream ss(p + 1, buffer_size); + do_test_stringStream(&ss, buffer_size, avg_write_len); + ASSERT_EQ(p[0], canary1); + ASSERT_EQ(p[buffer_size + 1], canary2); + FREE_C_HEAP_ARRAY(char, p); } TEST_VM(ostream, stringStream_dynamic_realloc_1) { - stringStream ss(2); // dynamic buffer with very small starting size - do_test_stringStream(&ss, false, 0); + // small starting size, smallish cap, small steps + do_test_dynamic_stringStream(random_wobble(10), random_wobble(1024), 10); } TEST_VM(ostream, stringStream_dynamic_realloc_2) { - stringStream ss(2); // dynamic buffer with very small starting size - do_test_stringStream(&ss, true, 0); + // small starting size, smallish cap, larger steps + do_test_dynamic_stringStream(random_wobble(10), random_wobble(1024), 128); } -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, stringStream_dynamic_realloc_3) { + // larger starting size (beyond size of init buffer), largish cap, large steps + do_test_dynamic_stringStream(random_wobble(4096), random_wobble(1 * M), 16 * K); +} + +TEST_VM(ostream, stringStream_dynamic_realloc_4) { + // small starting size, largish cap, very large steps + do_test_dynamic_stringStream(random_wobble(10), random_wobble(1 * M), 512 * K); +} + +TEST_VM(ostream, stringStream_dynamic_realloc_5) { + // small starting size, immediate truncation + do_test_dynamic_stringStream(random_wobble(10), 128 * K, 512 * K); +} + +#if 0 +// This tests with the default cap, which is very large (100M) so only enable this for manual tests. +TEST_VM(ostream, stringStream_dynamic_realloc_default_cap) { + stringStream ss; + do_test_stringStream(&ss, 100 * M, 1 * M); +} +#endif + +TEST_VM(ostream, stringStream_static_1) { + do_test_static_stringStream(random_wobble(30), 2); +} + +TEST_VM(ostream, stringStream_static_2) { + do_test_static_stringStream(random_wobble(1024), 128); +} + +TEST_VM(ostream, stringStream_static_3) { + do_test_static_stringStream(random_wobble(1 * M), 64 * K); } TEST_VM(ostream, bufferedStream_static) { @@ -121,13 +237,13 @@ } } -/* Activate to manually test bufferedStream dynamic cap. - +#if 0 +// This tests with the default cap, which is very large (100M) so only enable this for manual tests. +// Note that this will assert with "Exceeded max buffer size for this string" in debug builds +// which is the expected behavior. TEST_VM(ostream, bufferedStream_dynamic_large) { bufferedStream bs(1); // small to excercise realloc. size_t written = 0; - // The max cap imposed is 100M. Writing this much should safely hit it. - // Note that this will assert in debug builds which is the expected behavior. size_t expected_cap_at = 100 * M; for (int i = 0; i < 10000000; i ++) { written += print_lorem(&bs, false); @@ -139,7 +255,7 @@ } } -*/ +#endif