diff --git a/src/wasm/string-builder.h b/src/wasm/string-builder.h index 33d4781ded..903414076b 100644 --- a/src/wasm/string-builder.h +++ b/src/wasm/string-builder.h @@ -30,13 +30,16 @@ class StringBuilder { StringBuilder& operator=(const StringBuilder&) = delete; ~StringBuilder() { for (char* chunk : chunks_) delete[] chunk; + if (on_growth_ == kReplacePreviousChunk && start_ != stack_buffer_) { + delete[] start_; + } } // Reserves space for {n} characters and returns a pointer to its beginning. // Clients *must* write all {n} characters after calling this! // Don't call this directly, use operator<< overloads instead. char* allocate(size_t n) { - if (remaining_bytes_ < n) Grow(); + if (remaining_bytes_ < n) Grow(n); char* result = cursor_; cursor_ += n; remaining_bytes_ -= n; @@ -68,14 +71,25 @@ class StringBuilder { void start_here() { start_ = cursor_; } private: - void Grow() { + void Grow(size_t requested) { size_t used = length(); - // Safety net for super-long strings/lines. - size_t chunk_size = used < kChunkSize ? kChunkSize : used * 2; + size_t required = used + requested; + size_t chunk_size; + if (on_growth_ == kKeepOldChunks) { + // Usually grow by kChunkSize, unless super-long lines need even more. + chunk_size = required < kChunkSize ? kChunkSize : required * 2; + } else { + // When we only have one chunk, always (at least) double its size + // when it grows, to minimize both wasted memory and growth effort. + chunk_size = required * 2; + } + char* new_chunk = new char[chunk_size]; memcpy(new_chunk, start_, used); if (on_growth_ == kKeepOldChunks) { chunks_.push_back(new_chunk); + } else if (start_ != stack_buffer_) { + delete[] start_; } start_ = new_chunk; cursor_ = new_chunk + used; diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn index dbb47e686d..59115c6d10 100644 --- a/test/unittests/BUILD.gn +++ b/test/unittests/BUILD.gn @@ -517,6 +517,7 @@ v8_source_set("unittests_sources") { "wasm/module-decoder-unittest.cc", "wasm/simd-shuffle-unittest.cc", "wasm/streaming-decoder-unittest.cc", + "wasm/string-builder-unittest.cc", "wasm/subtyping-unittest.cc", "wasm/wasm-code-manager-unittest.cc", "wasm/wasm-compiler-unittest.cc", diff --git a/test/unittests/wasm/string-builder-unittest.cc b/test/unittests/wasm/string-builder-unittest.cc new file mode 100644 index 0000000000..2021e17feb --- /dev/null +++ b/test/unittests/wasm/string-builder-unittest.cc @@ -0,0 +1,43 @@ +// Copyright 2022 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "src/wasm/string-builder.h" + +#include "testing/gtest/include/gtest/gtest.h" + +namespace v8::internal::wasm { +namespace string_builder_unittest { + +TEST(StringBuilder, Simple) { + StringBuilder sb; + sb << "foo" + << "bar" << -42 << "\n"; + EXPECT_STREQ(std::string(sb.start(), sb.length()).c_str(), "foobar-42\n"); +} + +TEST(StringBuilder, DontLeak) { + // Should be bigger than StringBuilder::kStackSize = 256. + constexpr size_t kMoreThanStackBufferSize = 300; + StringBuilder sb; + const char* on_stack = sb.start(); + sb.allocate(kMoreThanStackBufferSize); + const char* on_heap = sb.start(); + // If this fails, then kMoreThanStackBufferSize was too small. + ASSERT_NE(on_stack, on_heap); + // Still don't leak on further growth. + sb.allocate(kMoreThanStackBufferSize * 4); +} + +TEST(StringBuilder, SuperLongStrings) { + // Should be bigger than StringBuilder::kChunkSize = 1024 * 1024. + constexpr size_t kMoreThanChunkSize = 2 * 1024 * 1024; + StringBuilder sb; + char* s = sb.allocate(kMoreThanChunkSize); + for (size_t i = 0; i < kMoreThanChunkSize; i++) { + s[i] = 'a'; + } +} + +} // namespace string_builder_unittest +} // namespace v8::internal::wasm