[utils] Remove StringBuilder class

StringBuilder and its base class SimpleStringBuilder aren't very safe
and are a potential source of memory leaks or double-frees.

This removes the StringBuilder class and converts all of its usages to
use the standard library.

(As a drive-by, this converts std::ostream* to std::ostream& which is
more idiomatic C++).

Bug: v8:11917
Change-Id: I0eaf9d60cf49836e65bb28f0e114b33ef8103a61
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2978252
Auto-Submit: Dan Elphick <delphick@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75298}

src/codegen/assembler.cc

@@ -236,7 +236,7 @@ AssemblerBase::~AssemblerBase() = default;
 
 void AssemblerBase::Print(Isolate* isolate) {
   StdoutStream os;
-  v8::internal::Disassembler::Decode(isolate, &os, buffer_start_, pc_);
+  v8::internal::Disassembler::Decode(isolate, os, buffer_start_, pc_);
 }
 
 // -----------------------------------------------------------------------------

src/compiler/pipeline.cc

@@ -3217,7 +3217,7 @@ wasm::WasmCompilationResult Pipeline::GenerateCodeForWasmNativeStub(
 #ifdef ENABLE_DISASSEMBLER
     std::stringstream disassembler_stream;
     Disassembler::Decode(
-        nullptr, &disassembler_stream, result.code_desc.buffer,
+        nullptr, disassembler_stream, result.code_desc.buffer,
         result.code_desc.buffer + result.code_desc.safepoint_table_offset,
         CodeReference(&result.code_desc));
     for (auto const c : disassembler_stream.str()) {
@@ -3326,7 +3326,7 @@ void Pipeline::GenerateCodeForWasmFunction(
 #ifdef ENABLE_DISASSEMBLER
     std::stringstream disassembler_stream;
     Disassembler::Decode(
-        nullptr, &disassembler_stream, result->code_desc.buffer,
+        nullptr, disassembler_stream, result->code_desc.buffer,
         result->code_desc.buffer + result->code_desc.safepoint_table_offset,
         CodeReference(&result->code_desc));
     for (auto const c : disassembler_stream.str()) {

src/diagnostics/disassembler.cc

@@ -4,7 +4,10 @@
 
 #include "src/diagnostics/disassembler.h"
 
+#include <algorithm>
+#include <iomanip>
 #include <memory>
+#include <sstream>
 #include <unordered_map>
 #include <vector>
 
@@ -184,42 +187,42 @@ const char* V8NameConverter::RootRelativeName(int offset) const {
   }
 }
 
-static void DumpBuffer(std::ostream* os, StringBuilder* out) {
-  (*os) << out->Finalize() << std::endl;
-  out->Reset();
+// Output the contents of the string stream and empty it.
+static void DumpBuffer(std::ostream& os, std::ostringstream& out) {
+  os << out.str() << std::endl;
+  out.str("");
 }
 
-static const int kOutBufferSize = 2048 + String::kMaxShortPrintLength;
 static const int kRelocInfoPosition = 57;
 
-static void PrintRelocInfo(StringBuilder* out, Isolate* isolate,
+static void PrintRelocInfo(std::ostringstream& out, Isolate* isolate,
                            const ExternalReferenceEncoder* ref_encoder,
-                           std::ostream* os, CodeReference host,
+                           std::ostream& os, CodeReference host,
                            RelocInfo* relocinfo, bool first_reloc_info = true) {
   // Indent the printing of the reloc info.
+  int padding = kRelocInfoPosition;
   if (first_reloc_info) {
     // The first reloc info is printed after the disassembled instruction.
-    out->AddPadding(' ', kRelocInfoPosition - out->position());
+    padding -= std::min(padding, static_cast<int>(out.tellp()));
   } else {
     // Additional reloc infos are printed on separate lines.
     DumpBuffer(os, out);
-    out->AddPadding(' ', kRelocInfoPosition);
   }
+  std::fill_n(std::ostream_iterator<char>(out), padding, ' ');
 
   RelocInfo::Mode rmode = relocinfo->rmode();
   if (rmode == RelocInfo::DEOPT_SCRIPT_OFFSET) {
-    out->AddFormatted("    ;; debug: deopt position, script offset '%d'",
-                      static_cast<int>(relocinfo->data()));
+    out << "    ;; debug: deopt position, script offset '"
+        << static_cast<int>(relocinfo->data()) << "'";
   } else if (rmode == RelocInfo::DEOPT_INLINING_ID) {
-    out->AddFormatted("    ;; debug: deopt position, inlining id '%d'",
-                      static_cast<int>(relocinfo->data()));
+    out << "    ;; debug: deopt position, inlining id '"
+        << static_cast<int>(relocinfo->data()) << "'";
   } else if (rmode == RelocInfo::DEOPT_REASON) {
     DeoptimizeReason reason = static_cast<DeoptimizeReason>(relocinfo->data());
-    out->AddFormatted("    ;; debug: deopt reason '%s'",
-                      DeoptimizeReasonToString(reason));
+    out << "    ;; debug: deopt reason '" << DeoptimizeReasonToString(reason)
+        << "'";
   } else if (rmode == RelocInfo::DEOPT_ID) {
-    out->AddFormatted("    ;; debug: deopt index %d",
-                      static_cast<int>(relocinfo->data()));
+    out << "    ;; debug: deopt index " << static_cast<int>(relocinfo->data());
   } else if (RelocInfo::IsEmbeddedObjectMode(rmode)) {
     HeapStringAllocator allocator;
     StringStream accumulator(&allocator);
@@ -230,24 +233,24 @@ static void PrintRelocInfo(StringBuilder* out, Isolate* isolate,
     }
     std::unique_ptr<char[]> obj_name = accumulator.ToCString();
     const bool is_compressed = RelocInfo::IsCompressedEmbeddedObject(rmode);
-    out->AddFormatted("    ;; %sobject: %s",
-                      is_compressed ? "(compressed) " : "", obj_name.get());
+    out << "    ;; " << (is_compressed ? "(compressed) " : "")
+        << "object: " << obj_name.get();
   } else if (rmode == RelocInfo::EXTERNAL_REFERENCE) {
     Address address = relocinfo->target_external_reference();
     const char* reference_name =
         ref_encoder
             ? ref_encoder->NameOfAddress(isolate, address)
             : ExternalReferenceTable::NameOfIsolateIndependentAddress(address);
-    out->AddFormatted("    ;; external reference (%s)", reference_name);
+    out << "    ;; external reference (" << reference_name << ")";
   } else if (RelocInfo::IsCodeTargetMode(rmode)) {
-    out->AddFormatted("    ;; code:");
+    out << "    ;; code:";
     Code code = isolate->heap()->GcSafeFindCodeForInnerPointer(
         relocinfo->target_address());
     CodeKind kind = code.kind();
     if (code.is_builtin()) {
-      out->AddFormatted(" Builtin::%s", Builtins::name(code.builtin_id()));
+      out << " Builtin::" << Builtins::name(code.builtin_id());
     } else {
-      out->AddFormatted(" %s", CodeKindToString(kind));
+      out << " " << CodeKindToString(kind);
     }
 #if V8_ENABLE_WEBASSEMBLY
   } else if (RelocInfo::IsWasmStubCall(rmode) && host.is_wasm_code()) {
@@ -255,31 +258,30 @@ static void PrintRelocInfo(StringBuilder* out, Isolate* isolate,
     const char* runtime_stub_name = GetRuntimeStubName(
         host.as_wasm_code()->native_module()->GetRuntimeStubId(
             relocinfo->wasm_stub_call_address()));
-    out->AddFormatted("    ;; wasm stub: %s", runtime_stub_name);
+    out << "    ;; wasm stub: " << runtime_stub_name;
 #endif  // V8_ENABLE_WEBASSEMBLY
   } else if (RelocInfo::IsRuntimeEntry(rmode) && isolate != nullptr) {
     // A runtime entry relocinfo might be a deoptimization bailout.
     Address addr = relocinfo->target_address();
     DeoptimizeKind type;
     if (Deoptimizer::IsDeoptimizationEntry(isolate, addr, &type)) {
-      out->AddFormatted("    ;; %s deoptimization bailout",
-                        Deoptimizer::MessageFor(type, false));
+      out << "    ;; " << Deoptimizer::MessageFor(type, false)
+          << " deoptimization bailout";
     } else {
-      out->AddFormatted("    ;; %s", RelocInfo::RelocModeName(rmode));
+      out << "    ;; " << RelocInfo::RelocModeName(rmode);
     }
   } else {
-    out->AddFormatted("    ;; %s", RelocInfo::RelocModeName(rmode));
+    out << "    ;; " << RelocInfo::RelocModeName(rmode);
   }
 }
 
 static int DecodeIt(Isolate* isolate, ExternalReferenceEncoder* ref_encoder,
-                    std::ostream* os, CodeReference code,
+                    std::ostream& os, CodeReference code,
                     const V8NameConverter& converter, byte* begin, byte* end,
                     Address current_pc) {
   CHECK(!code.is_null());
   v8::base::EmbeddedVector<char, 128> decode_buffer;
-  v8::base::EmbeddedVector<char, kOutBufferSize> out_buffer;
-  StringBuilder out(out_buffer.begin(), out_buffer.length());
+  std::ostringstream out;
   byte* pc = begin;
   disasm::Disassembler d(converter,
                          disasm::Disassembler::kContinueOnUnimplementedOpcode);
@@ -352,20 +354,20 @@ static int DecodeIt(Isolate* isolate, ExternalReferenceEncoder* ref_encoder,
 
     // Comments.
     for (size_t i = 0; i < comments.size(); i++) {
-      out.AddFormatted("                  %s", comments[i]);
-      DumpBuffer(os, &out);
+      out << "                  " << comments[i];
+      DumpBuffer(os, out);
     }
 
     // Instruction address and instruction offset.
     if (FLAG_log_colour && reinterpret_cast<Address>(prev_pc) == current_pc) {
       // If this is the given "current" pc, make it yellow and bold.
-      out.AddFormatted("\033[33;1m");
+      out << "\033[33;1m";
     }
-    out.AddFormatted("%p  %4" V8PRIxPTRDIFF "  ", static_cast<void*>(prev_pc),
-                     prev_pc - begin);
+    out << static_cast<void*>(prev_pc) << "  " << std::setw(4)
+        << prev_pc - begin;
 
     // Instruction.
-    out.AddFormatted("%s", decode_buffer.begin());
+    out << decode_buffer.begin();
 
     // Print all the reloc info for this instruction which are not comments.
     for (size_t i = 0; i < pcs.size(); i++) {
@@ -382,7 +384,7 @@ static int DecodeIt(Isolate* isolate, ExternalReferenceEncoder* ref_encoder,
                           constant_pool);
 
       bool first_reloc_info = (i == 0);
-      PrintRelocInfo(&out, isolate, ref_encoder, os, code, &relocinfo,
+      PrintRelocInfo(out, isolate, ref_encoder, os, code, &relocinfo,
                      first_reloc_info);
     }
 
@@ -404,7 +406,7 @@ static int DecodeIt(Isolate* isolate, ExternalReferenceEncoder* ref_encoder,
           if (reloc_it.rinfo()->IsInConstantPool() &&
               (reloc_it.rinfo()->constant_pool_entry_address() ==
                constant_pool_entry_address)) {
-            PrintRelocInfo(&out, isolate, ref_encoder, os, code,
+            PrintRelocInfo(out, isolate, ref_encoder, os, code,
                            reloc_it.rinfo());
             break;
           }
@@ -414,17 +416,17 @@ static int DecodeIt(Isolate* isolate, ExternalReferenceEncoder* ref_encoder,
     }
 
     if (FLAG_log_colour && reinterpret_cast<Address>(prev_pc) == current_pc) {
-      out.AddFormatted("\033[m");
+      out << "\033[m";
     }
 
-    DumpBuffer(os, &out);
+    DumpBuffer(os, out);
   }
 
   // Emit comments following the last instruction (if any).
   while (cit.HasCurrent() &&
          cit.GetPCOffset() < static_cast<Address>(pc - begin)) {
-    out.AddFormatted("                  %s", cit.GetComment());
-    DumpBuffer(os, &out);
+    out << "                  " << cit.GetComment();
+    DumpBuffer(os, out);
     cit.Next();
   }
 
@@ -432,7 +434,7 @@ static int DecodeIt(Isolate* isolate, ExternalReferenceEncoder* ref_encoder,
   return static_cast<int>(pc - begin);
 }
 
-int Disassembler::Decode(Isolate* isolate, std::ostream* os, byte* begin,
+int Disassembler::Decode(Isolate* isolate, std::ostream& os, byte* begin,
                          byte* end, CodeReference code, Address current_pc) {
   DCHECK_WITH_MSG(FLAG_text_is_readable,
                   "Builtins disassembly requires a readable .text section");
@@ -455,7 +457,7 @@ int Disassembler::Decode(Isolate* isolate, std::ostream* os, byte* begin,
 
 #else  // ENABLE_DISASSEMBLER
 
-int Disassembler::Decode(Isolate* isolate, std::ostream* os, byte* begin,
+int Disassembler::Decode(Isolate* isolate, std::ostream& os, byte* begin,
                          byte* end, CodeReference code, Address current_pc) {
   return 0;
 }

src/diagnostics/disassembler.h

@@ -20,7 +20,7 @@ class Disassembler : public AllStatic {
   // Instruction'.
   // the code object is used for name resolution and may be null.
   // TODO(titzer): accept a {WasmCodeManager*} if {isolate} is null
-  V8_EXPORT_PRIVATE static int Decode(Isolate* isolate, std::ostream* os,
+  V8_EXPORT_PRIVATE static int Decode(Isolate* isolate, std::ostream& os,
                                       byte* begin, byte* end,
                                       CodeReference code = {},
                                       Address current_pc = kNullAddress);

src/diagnostics/gdb-jit.cc

@@ -1093,14 +1093,10 @@ class DebugInfoSection : public DebugSection {
       int internal_slots = Context::MIN_CONTEXT_SLOTS;
       int current_abbreviation = 4;
 
-      base::EmbeddedVector<char, 256> buffer;
-      StringBuilder builder(buffer.begin(), buffer.length());
-
       for (int param = 0; param < params; ++param) {
         w->WriteULEB128(current_abbreviation++);
-        builder.Reset();
-        builder.AddFormatted("param%d", param);
-        w->WriteString(builder.Finalize());
+        w->WriteString("param");
+        w->Write(std::to_string(param).c_str());
         w->Write<uint32_t>(ty_offset);
         Writer::Slot<uint32_t> block_size = w->CreateSlotHere<uint32_t>();
         uintptr_t block_start = w->position();
@@ -1124,9 +1120,8 @@ class DebugInfoSection : public DebugSection {
 
       for (int context_slot = 0; context_slot < context_slots; ++context_slot) {
         w->WriteULEB128(current_abbreviation++);
-        builder.Reset();
-        builder.AddFormatted("context_slot%d", context_slot + internal_slots);
-        w->WriteString(builder.Finalize());
+        w->WriteString("context_slot");
+        w->Write(std::to_string(context_slot + internal_slots).c_str());
       }
 
       {
@@ -2024,14 +2019,12 @@ void EventHandler(const v8::JitCodeEvent* event) {
       Isolate* isolate = reinterpret_cast<Isolate*>(event->isolate);
       Code code = isolate->heap()->GcSafeFindCodeForInnerPointer(addr);
       LineInfo* lineinfo = GetLineInfo(addr);
-      base::EmbeddedVector<char, 256> buffer;
-      StringBuilder builder(buffer.begin(), buffer.length());
-      builder.AddSubstring(event->name.str, static_cast<int>(event->name.len));
+      std::string event_name(event->name.str, event->name.len);
       // It's called UnboundScript in the API but it's a SharedFunctionInfo.
       SharedFunctionInfo shared = event->script.IsEmpty()
                                       ? SharedFunctionInfo()
                                       : *Utils::OpenHandle(*event->script);
-      AddCode(builder.Finalize(), code, shared, lineinfo);
+      AddCode(event_name.c_str(), code, shared, lineinfo);
       break;
     }
     case v8::JitCodeEvent::CODE_MOVED:

src/objects/code.cc

@@ -478,7 +478,7 @@ inline void DisassembleCodeRange(Isolate* isolate, std::ostream& os, Code code,
   AllowHandleAllocation allow_handles;
   DisallowGarbageCollection no_gc;
   HandleScope handle_scope(isolate);
-  Disassembler::Decode(isolate, &os, reinterpret_cast<byte*>(begin),
+  Disassembler::Decode(isolate, os, reinterpret_cast<byte*>(begin),
                        reinterpret_cast<byte*>(end),
                        CodeReference(handle(code, isolate)), current_pc);
 }

src/utils/utils.cc

@@ -273,23 +273,6 @@ int WriteBytes(const char* filename, const byte* bytes, int size,
   return WriteChars(filename, str, size, verbose);
 }
 
-void StringBuilder::AddFormatted(const char* format, ...) {
-  va_list arguments;
-  va_start(arguments, format);
-  AddFormattedList(format, arguments);
-  va_end(arguments);
-}
-
-void StringBuilder::AddFormattedList(const char* format, va_list list) {
-  DCHECK(!is_finalized() && position_ <= buffer_.length());
-  int n = VSNPrintF(buffer_ + position_, format, list);
-  if (n < 0 || n >= (buffer_.length() - position_)) {
-    position_ = buffer_.length();
-  } else {
-    position_ += n;
-  }
-}
-
 // Returns false iff d is NaN, +0, or -0.
 bool DoubleToBoolean(double d) {
   IeeeDoubleArchType u;

src/utils/utils.h

@@ -656,21 +656,6 @@ V8_EXPORT_PRIVATE std::string ReadFile(const char* filename, bool* exists,
 V8_EXPORT_PRIVATE std::string ReadFile(FILE* file, bool* exists,
                                        bool verbose = true);
 
-class StringBuilder : public SimpleStringBuilder {
- public:
-  explicit StringBuilder(int size) : SimpleStringBuilder(size) {}
-  StringBuilder(char* buffer, int size) : SimpleStringBuilder(buffer, size) {}
-
-  // Add formatted contents to the builder just like printf().
-  void PRINTF_FORMAT(2, 3) AddFormatted(const char* format, ...);
-
-  // Add formatted contents like printf based on a va_list.
-  void PRINTF_FORMAT(2, 0) AddFormattedList(const char* format, va_list list);
-
- private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(StringBuilder);
-};
-
 bool DoubleToBoolean(double d);
 
 template <typename Char>

src/wasm/wasm-code-manager.cc

@@ -381,7 +381,7 @@ void WasmCode::Disassemble(const char* name, std::ostream& os,
   }
   DCHECK_LT(0, instruction_size);
   os << "Instructions (size = " << instruction_size << ")\n";
-  Disassembler::Decode(nullptr, &os, instructions().begin(),
+  Disassembler::Decode(nullptr, os, instructions().begin(),
                        instructions().begin() + instruction_size,
                        CodeReference(this), current_pc);
   os << "\n";

test/cctest/disasm-regex-helper.cc

@@ -23,7 +23,7 @@ std::string DisassembleFunction(const char* function) {
   Address end = f->code().raw_instruction_end();
   Isolate* isolate = CcTest::i_isolate();
   std::ostringstream os;
-  Disassembler::Decode(isolate, &os, reinterpret_cast<byte*>(begin),
+  Disassembler::Decode(isolate, os, reinterpret_cast<byte*>(begin),
                        reinterpret_cast<byte*>(end),
                        CodeReference(handle(f->code(), isolate)));
   return os.str();