[PATCH 00/13] Remove a bunch of alloca uses

[PATCH 00/13] Remove a bunch of alloca uses

[Andrew Burgess]

Continuing effort to replace alloca calls with C++ data structures,
here's a bunch of places where gdb::byte_vector can be used.

This isn't all of them, there's still plenty to work through, but I
thought I'd see how this lot is received.

Thanks,
Andrew

---

Andrew Burgess (13):
  gdb: remove uses of alloca from arch-utils.c
  gdb: remove use of alloca from auxv.c
  gdb: remove use of alloca from c-lang.c
  gdb: remove use of alloca from corefile.c
  gdb: remove uses of alloca from dwarf2/expr.c
  gdb: remove a use of alloca from elfread.c
  gdb: remove use of alloca from findvar.c
  gdb: remove use of alloca from linux-nat-trad.c
  gdb: remove use of alloca from mem-break.c
  gdb: remove some uses of alloca from printcmd.c
  gdb: remove some uses of alloca from remote.c
  gdb: remove uses of alloca from valprint.c
  gdb: remove a use of alloca from symfile.c

 gdb/arch-utils.c     | 13 ++++++-------
 gdb/auxv.c           | 10 +++++-----
 gdb/c-lang.c         |  8 +++-----
 gdb/corefile.c       | 18 +++++++++---------
 gdb/dwarf2/expr.c    | 25 ++++++++++++++-----------
 gdb/elfread.c        |  7 ++++---
 gdb/findvar.c        |  6 +++---
 gdb/linux-nat-trad.c |  8 ++++----
 gdb/mem-break.c      |  7 +++----
 gdb/printcmd.c       | 44 +++++++++++++++++++++-----------------------
 gdb/remote.c         | 40 ++++++++++++++++++----------------------
 gdb/symfile.c        | 11 +++++------
 gdb/valprint.c       | 17 ++++++++---------
 13 files changed, 103 insertions(+), 111 deletions(-)


base-commit: 85c7cb3c4b70cc484ecf3d72a116503876a28f0a
-- 
2.25.4

[PATCH 01/13] gdb: remove uses of alloca from arch-utils.c

[Andrew Burgess]

Remove the two uses of alloca from arch-utils.c, replace them both
with gdb::byte_vector instead.

There should be no user visible changes after this commit.
---
 gdb/arch-utils.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index e3af9ce2dbc..be90d0ed89c 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -933,7 +933,7 @@ default_program_breakpoint_here_p (struct gdbarch *gdbarch,
   if (bpoint == nullptr)
     return false;
 
-  gdb_byte *target_mem = (gdb_byte *) alloca (len);
+  gdb::byte_vector target_mem (len);
 
   /* Enable the automatic memory restoration from breakpoints while
      we read the memory.  Otherwise we may find temporary breakpoints, ones
@@ -941,11 +941,11 @@ default_program_breakpoint_here_p (struct gdbarch *gdbarch,
   scoped_restore restore_memory
     = make_scoped_restore_show_memory_breakpoints (0);
 
-  if (target_read_memory (address, target_mem, len) == 0)
+  if (target_read_memory (address, target_mem.data (), len) == 0)
     {
       /* Check if this is a breakpoint instruction for this architecture,
 	 including ones used by GDB.  */
-      if (memcmp (target_mem, bpoint, len) == 0)
+      if (memcmp (target_mem.data (), bpoint, len) == 0)
 	return true;
     }
 
@@ -1012,7 +1012,6 @@ default_guess_tracepoint_registers (struct gdbarch *gdbarch,
 				    CORE_ADDR addr)
 {
   int pc_regno = gdbarch_pc_regnum (gdbarch);
-  gdb_byte *regs;
 
   /* This guessing code below only works if the PC register isn't
      a pseudo-register.  The value of a pseudo-register isn't stored
@@ -1023,10 +1022,10 @@ default_guess_tracepoint_registers (struct gdbarch *gdbarch,
   if (pc_regno < 0 || pc_regno >= gdbarch_num_regs (gdbarch))
     return;
 
-  regs = (gdb_byte *) alloca (register_size (gdbarch, pc_regno));
-  store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
+  gdb::byte_vector regs (register_size (gdbarch, pc_regno));
+  store_unsigned_integer (regs.data (), register_size (gdbarch, pc_regno),
 			  gdbarch_byte_order (gdbarch), addr);
-  regcache->raw_supply (pc_regno, regs);
+  regcache->raw_supply (pc_regno, regs.data ());
 }
 
 int
-- 
2.25.4

[PATCH 02/13] gdb: remove use of alloca from auxv.c

[Andrew Burgess]

Remove the last use of alloca from auxv.c and replace it with
gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/auxv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/auxv.c b/gdb/auxv.c
index 812b2807554..aea58807f4c 100644
--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -86,7 +86,6 @@ ld_so_xfer_auxv (gdb_byte *readbuf,
   struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
   size_t ptr_size = ptr_type->length ();
   size_t auxv_pair_size = 2 * ptr_size;
-  gdb_byte *ptr_buf = (gdb_byte *) alloca (ptr_size);
   LONGEST retval;
   size_t block;
 
@@ -123,10 +122,11 @@ ld_so_xfer_auxv (gdb_byte *readbuf,
      we might need a valgrind extension to make it work.  This is PR
      11440.  */
 
-  if (target_read_memory (pointer_address, ptr_buf, ptr_size) != 0)
+  gdb::byte_vector ptr_buf (ptr_size);
+  if (target_read_memory (pointer_address, ptr_buf.data (), ptr_size) != 0)
     return TARGET_XFER_E_IO;
 
-  data_address = extract_typed_address (ptr_buf, ptr_type);
+  data_address = extract_typed_address (ptr_buf.data (), ptr_type);
 
   /* Possibly still not initialized such as during an inferior
      startup.  */
@@ -151,11 +151,11 @@ ld_so_xfer_auxv (gdb_byte *readbuf,
 
   if (offset >= auxv_pair_size)
     {
-      if (target_read_memory (data_address - auxv_pair_size, ptr_buf,
+      if (target_read_memory (data_address - auxv_pair_size, ptr_buf.data (),
 			      ptr_size) != 0)
 	return TARGET_XFER_E_IO;
 
-      if (extract_typed_address (ptr_buf, ptr_type) == AT_NULL)
+      if (extract_typed_address (ptr_buf.data (), ptr_type) == AT_NULL)
 	return TARGET_XFER_EOF;
     }
 
-- 
2.25.4

[PATCH 03/13] gdb: remove use of alloca from c-lang.c

[Andrew Burgess]

Remove the use of alloca from c-lang.c and replace it with
gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/c-lang.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 6535ab93498..faac5d47179 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -430,11 +430,9 @@ static void
 emit_numeric_character (struct type *type, unsigned long value,
 			struct obstack *output)
 {
-  gdb_byte *buffer;
-
-  buffer = (gdb_byte *) alloca (type->length ());
-  pack_long (buffer, type, value);
-  obstack_grow (output, buffer, type->length ());
+  gdb::byte_vector buffer (type->length ());
+  pack_long (buffer.data (), type, value);
+  obstack_grow (output, buffer.data (), type->length ());
 }
 
 /* Convert an octal escape sequence.  TYPE is the target character
-- 
2.25.4

[PATCH 04/13] gdb: remove use of alloca from corefile.c

[Andrew Burgess]

Remove uses of alloca from corefile.c and replace with
gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/corefile.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/corefile.c b/gdb/corefile.c
index 5cb559000b8..c76b812f717 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -334,10 +334,10 @@ read_code_unsigned_integer (CORE_ADDR memaddr, int len,
 CORE_ADDR
 read_memory_typed_address (CORE_ADDR addr, struct type *type)
 {
-  gdb_byte *buf = (gdb_byte *) alloca (type->length ());
+  gdb::byte_vector buf (type->length ());
 
-  read_memory (addr, buf, type->length ());
-  return extract_typed_address (buf, type);
+  read_memory (addr, buf.data (), type->length ());
+  return extract_typed_address (buf.data (), type);
 }
 
 /* See gdbcore.h.  */
@@ -370,10 +370,10 @@ write_memory_unsigned_integer (CORE_ADDR addr, int len,
 			       enum bfd_endian byte_order,
 			       ULONGEST value)
 {
-  gdb_byte *buf = (gdb_byte *) alloca (len);
+  gdb::byte_vector buf (len);
 
-  store_unsigned_integer (buf, len, byte_order, value);
-  write_memory (addr, buf, len);
+  store_unsigned_integer (buf.data (), len, byte_order, value);
+  write_memory (addr, buf.data (), len);
 }
 
 /* Store VALUE at ADDR in the inferior as a LEN-byte signed
@@ -383,10 +383,10 @@ write_memory_signed_integer (CORE_ADDR addr, int len,
 			     enum bfd_endian byte_order,
 			     LONGEST value)
 {
-  gdb_byte *buf = (gdb_byte *) alloca (len);
+  gdb::byte_vector buf (len);
 
-  store_signed_integer (buf, len, byte_order, value);
-  write_memory (addr, buf, len);
+  store_signed_integer (buf.data (), len, byte_order, value);
+  write_memory (addr, buf.data (), len);
 }
 \f
 /* The current default bfd target.  Points to storage allocated for
-- 
2.25.4

[PATCH 05/13] gdb: remove uses of alloca from dwarf2/expr.c

[Andrew Burgess]

Remove the remaining uses of alloca from dwarf2/expr.c and replace
them with gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/dwarf2/expr.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 2b41372be8a..c01e745bf0a 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -1155,12 +1155,12 @@ dwarf_expr_context::fetch_address (int n)
      for those architectures which require it.  */
   if (gdbarch_integer_to_address_p (arch))
     {
-      gdb_byte *buf = (gdb_byte *) alloca (this->m_addr_size);
-      type *int_type = get_unsigned_type (arch,
-					  result_val->type ());
+      gdb::byte_vector buf (this->m_addr_size);
+      type *int_type = get_unsigned_type (arch, result_val->type ());
 
-      store_unsigned_integer (buf, this->m_addr_size, byte_order, result);
-      return gdbarch_integer_to_address (arch, int_type, buf);
+      store_unsigned_integer (buf.data (), this->m_addr_size, byte_order,
+			      result);
+      return gdbarch_integer_to_address (arch, int_type, buf.data ());
     }
 
   return (CORE_ADDR) result;
@@ -1883,7 +1883,6 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
 	case DW_OP_GNU_deref_type:
 	  {
 	    int addr_size = (op == DW_OP_deref ? this->m_addr_size : *op_ptr++);
-	    gdb_byte *buf = (gdb_byte *) alloca (addr_size);
 	    CORE_ADDR addr = fetch_address (0);
 	    struct type *type;
 
@@ -1898,21 +1897,25 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
 	    else
 	      type = address_type;
 
-	    this->read_mem (buf, addr, addr_size);
+	    gdb::byte_vector buf (std::max ((ULONGEST) addr_size,
+					    type->length ()));
+
+	    this->read_mem (buf.data (), addr, addr_size);
 
 	    /* If the size of the object read from memory is different
 	       from the type length, we need to zero-extend it.  */
 	    if (type->length () != addr_size)
 	      {
 		ULONGEST datum =
-		  extract_unsigned_integer (buf, addr_size, byte_order);
+		  extract_unsigned_integer (buf.data (), addr_size,
+					    byte_order);
 
-		buf = (gdb_byte *) alloca (type->length ());
-		store_unsigned_integer (buf, type->length (),
+		store_unsigned_integer (buf.data (), type->length (),
 					byte_order, datum);
 	      }
 
-	    result_val = value_from_contents_and_address (type, buf, addr);
+	    result_val = value_from_contents_and_address (type, buf.data (),
+							  addr);
 	    break;
 	  }
 
-- 
2.25.4

[PATCH 06/13] gdb: remove a use of alloca from elfread.c

[Andrew Burgess]

This removes one of the uses of alloca from elfread.c and replaces it
with gdb::byte_vector.  There are other uses of alloca in this file,
but they all deal with types other than gdb_byte and so will require
replacing with something other than gdb::byte_vector.

There should be no user visible changes in this commit.
---
 gdb/elfread.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index ca684aab57e..05a7f753843 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -839,7 +839,6 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
 	 size_t ptr_size = ptr_type->length ();
 	 CORE_ADDR pointer_address, addr;
 	 asection *plt;
-	 gdb_byte *buf = (gdb_byte *) alloca (ptr_size);
 	 bound_minimal_symbol msym;
 
 	 msym = lookup_minimal_symbol (name_got_plt, NULL, objfile);
@@ -855,9 +854,11 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
 
 	 if (msym.minsym->size () != ptr_size)
 	   return 0;
-	 if (target_read_memory (pointer_address, buf, ptr_size) != 0)
+
+	 gdb::byte_vector buf (ptr_size);
+	 if (target_read_memory (pointer_address, buf.data (), ptr_size) != 0)
 	   return 0;
-	 addr = extract_typed_address (buf, ptr_type);
+	 addr = extract_typed_address (buf.data (), ptr_type);
 	 addr = gdbarch_convert_from_func_ptr_addr
 	   (gdbarch, addr, current_inferior ()->top_target ());
 	 addr = gdbarch_addr_bits_remove (gdbarch, addr);
-- 
2.25.4

[PATCH 07/13] gdb: remove use of alloca from findvar.c

[Andrew Burgess]

Remove the use of alloca from findvar.c and replace it with
gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/findvar.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 60b5ca3faf3..2c5ee11a2c7 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -951,11 +951,11 @@ address_from_register (int regnum, frame_info_ptr frame)
      pointer types.  Avoid constructing a value object in those cases.  */
   if (gdbarch_convert_register_p (gdbarch, regnum, type))
     {
-      gdb_byte *buf = (gdb_byte *) alloca (type->length ());
+      gdb::byte_vector buf (type->length ());
       int optim, unavail, ok;
 
       ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
-				      buf, &optim, &unavail);
+				      buf.data (), &optim, &unavail);
       if (!ok)
 	{
 	  /* This function is used while computing a location expression.
@@ -965,7 +965,7 @@ address_from_register (int regnum, frame_info_ptr frame)
 	  error_value_optimized_out ();
 	}
 
-      return unpack_long (type, buf);
+      return unpack_long (type, buf.data ());
     }
 
   value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id);
-- 
2.25.4

[PATCH 08/13] gdb: remove use of alloca from linux-nat-trad.c

[Andrew Burgess]

Remove use of alloca from linux-nat-trad.c and replace with
gdb::byte_vector.

I've compiled for the mips-linux target (which uses this file) to
ensure that the file still builds, but I've not tested GDB for a
target that uses this file.  Still, I'm pretty confident that I've not
broken anything with this change.

There should be no user visible changes after this commit.
---
 gdb/linux-nat-trad.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/linux-nat-trad.c b/gdb/linux-nat-trad.c
index 8204f3883f5..408d9118766 100644
--- a/gdb/linux-nat-trad.c
+++ b/gdb/linux-nat-trad.c
@@ -32,7 +32,6 @@ linux_nat_trad_target::fetch_register (struct regcache *regcache, int regnum)
   struct gdbarch *gdbarch = regcache->arch ();
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR addr;
-  gdb_byte *buf;
   size_t size;
   pid_t pid;
   int i;
@@ -49,7 +48,8 @@ linux_nat_trad_target::fetch_register (struct regcache *regcache, int regnum)
   pid = get_ptrace_pid (regcache->ptid ());
 
   size = register_size (gdbarch, regnum);
-  buf = (gdb_byte *) alloca (size);
+  gdb::byte_vector buf_storage (size);
+  gdb_byte *buf = buf_storage.data ();
 
   /* Read the register contents from the inferior a chunk at a time.  */
   for (i = 0; i < size; i += sizeof (PTRACE_TYPE_RET))
@@ -95,7 +95,6 @@ linux_nat_trad_target::store_register (const struct regcache *regcache,
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR addr;
   size_t size;
-  gdb_byte *buf;
   pid_t pid;
   int i;
 
@@ -108,7 +107,8 @@ linux_nat_trad_target::store_register (const struct regcache *regcache,
   pid = get_ptrace_pid (regcache->ptid ());
 
   size = register_size (gdbarch, regnum);
-  buf = (gdb_byte *) alloca (size);
+  gdb::byte_vector buf_storage (size);
+  gdb_byte *buf = buf_storage.data ();
 
   /* Write the register contents into the inferior a chunk at a time.  */
   regcache->raw_collect (regnum, buf);
-- 
2.25.4

[PATCH 09/13] gdb: remove use of alloca from mem-break.c

[Andrew Burgess]

Replace the use of alloca with gdb::byte_vector in mem-break.c.

There should be no user visible changes after this commit.
---
 gdb/mem-break.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gdb/mem-break.c b/gdb/mem-break.c
index abff4b47081..81059b37335 100644
--- a/gdb/mem-break.c
+++ b/gdb/mem-break.c
@@ -41,7 +41,6 @@ default_memory_insert_breakpoint (struct gdbarch *gdbarch,
 {
   CORE_ADDR addr = bp_tgt->placed_address;
   const unsigned char *bp;
-  gdb_byte *readbuf;
   int bplen;
   int val;
 
@@ -50,8 +49,8 @@ default_memory_insert_breakpoint (struct gdbarch *gdbarch,
 
   /* Save the memory contents in the shadow_contents buffer and then
      write the breakpoint instruction.  */
-  readbuf = (gdb_byte *) alloca (bplen);
-  val = target_read_memory (addr, readbuf, bplen);
+  gdb::byte_vector readbuf (bplen);
+  val = target_read_memory (addr, readbuf.data (), bplen);
   if (val == 0)
     {
       /* These must be set together, either before or after the shadow
@@ -63,7 +62,7 @@ default_memory_insert_breakpoint (struct gdbarch *gdbarch,
 	 conditions/commands on the target side for some types of
 	 breakpoints, such as target remote.  */
       bp_tgt->shadow_len = bplen;
-      memcpy (bp_tgt->shadow_contents, readbuf, bplen);
+      memcpy (bp_tgt->shadow_contents, readbuf.data (), bplen);
 
       val = target_write_raw_memory (addr, bp, bplen);
     }
-- 
2.25.4

[PATCH 10/13] gdb: remove some uses of alloca from printcmd.c

[Andrew Burgess]

Remove a couple of uses of alloca from printcmd.c, and replace them
with gdb::byte_vector.

Unlike most times when alloca is replace with gdb::byte_vector, the
size of the vector is not specified at the byte_vector creation time.
In this case the vector can be sized differently depending on which
path of an `if` is taken, however, the lifetime of the data within the
vector must extend beyond the `if` itself.

There is a remaining use of alloca in this file, but that case will
not be replace with gdb::byte_vector, so I've left that for now.

There should be no user visible changes after this commit.
---
 gdb/printcmd.c | 44 +++++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 0d3bc292d4e..1efd3cabf54 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2443,7 +2443,7 @@ static void
 printf_c_string (struct ui_file *stream, const char *format,
 		 struct value *value)
 {
-  const gdb_byte *str;
+  gdb::byte_vector str;
 
   if (value->type ()->code () != TYPE_CODE_PTR
       && value->lval () == lval_internalvar
@@ -2455,11 +2455,10 @@ printf_c_string (struct ui_file *stream, const char *format,
 	 character.  This protects against corrupted C-style strings that lack
 	 the terminating null char.  It also allows Ada-style strings (not
 	 null terminated) to be printed without problems.  */
-      gdb_byte *tem_str = (gdb_byte *) alloca (len + 1);
+      str.resize (len + 1);
 
-      memcpy (tem_str, value->contents ().data (), len);
-      tem_str [len] = 0;
-      str = tem_str;
+      memcpy (str.data (), value->contents ().data (), len);
+      str [len] = 0;
     }
   else
     {
@@ -2474,31 +2473,30 @@ printf_c_string (struct ui_file *stream, const char *format,
 	  return;
 	}
 
-      /* This is a %s argument.  Find the length of the string.  */
-      size_t len;
-
-      for (len = 0;; len++)
+      /* This is a %s argument.  Build the string in STR which is
+	 currently empty.  */
+      gdb_assert (str.size () == 0);
+      for (size_t len = 0;; len++)
 	{
 	  gdb_byte c;
 
 	  QUIT;
 	  read_memory (tem + len, &c, 1);
+	  str.push_back (c);
 	  if (c == 0)
 	    break;
 	}
 
-      /* Copy the string contents into a string inside GDB.  */
-      gdb_byte *tem_str = (gdb_byte *) alloca (len + 1);
-
-      if (len != 0)
-	read_memory (tem, tem_str, len);
-      tem_str[len] = 0;
-      str = tem_str;
+      /* We will have passed through the above loop at least once, and will
+	 only exit the loop when we have pushed a zero byte onto the end of
+	 STR.  */
+      gdb_assert (str.size () > 0);
+      gdb_assert (str.back () == 0);
     }
 
   DIAGNOSTIC_PUSH
   DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
-    gdb_printf (stream, format, (char *) str);
+    gdb_printf (stream, format, (char *) str.data ());
   DIAGNOSTIC_POP
 }
 
@@ -2539,23 +2537,23 @@ printf_wide_c_string (struct ui_file *stream, const char *format,
 
       /* This is a %s argument.  Find the length of the string.  */
       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-      gdb_byte *buf = (gdb_byte *) alloca (wcwidth);
+      gdb::byte_vector buf (wcwidth);
 
       for (len = 0;; len += wcwidth)
 	{
 	  QUIT;
-	  read_memory (tem + len, buf, wcwidth);
-	  if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0)
+	  read_memory (tem + len, buf.data (), wcwidth);
+	  if (extract_unsigned_integer (buf.data (), wcwidth, byte_order) == 0)
 	    break;
 	}
 
       /* Copy the string contents into a string inside GDB.  */
-      gdb_byte *tem_str = (gdb_byte *) alloca (len + wcwidth);
+      gdb::byte_vector tem_str (len + wcwidth);
 
       if (len != 0)
-	read_memory (tem, tem_str, len);
+	read_memory (tem, tem_str.data (), len);
       memset (&tem_str[len], 0, wcwidth);
-      str = tem_str;
+      str = tem_str.data ();
     }
 
   auto_obstack output;
-- 
2.25.4

[PATCH 11/13] gdb: remove some uses of alloca from remote.c

[Andrew Burgess]

Remove some of the uses of alloca from remote.c and replace them with
either gdb::byte_vector or std::vector<gdb_byte>.

In one case I've used std::vector<gdb_byte> because this handles
automatically zeroing the contents, and this allows me to remove an
associated memset.

There are other uses of alloca in this file (remote.c), but these are
all for types other than gdb_byte so need replacing with something
other than gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/remote.c | 40 ++++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index ba7a7520cb4..3f0552e7c9e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8554,7 +8554,6 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
   struct gdbarch *gdbarch = regcache->arch ();
   struct remote_state *rs = get_remote_state ();
   char *buf, *p;
-  gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
   int i;
 
   if (m_features.packet_support (PACKET_p) == PACKET_DISABLE)
@@ -8593,6 +8592,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
     }
 
   /* Otherwise, parse and supply the value.  */
+  gdb::byte_vector regp (register_size (gdbarch, reg->regnum));
   p = buf;
   i = 0;
   while (p[0] != 0)
@@ -8603,7 +8603,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
       regp[i++] = fromhex (p[0]) * 16 + fromhex (p[1]);
       p += 2;
     }
-  regcache->raw_supply (reg->regnum, regp);
+  regcache->raw_supply (reg->regnum, regp.data ());
   return 1;
 }
 
@@ -8862,7 +8862,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
   struct remote_state *rs = get_remote_state ();
   /* Try storing a single register.  */
   char *buf = rs->buf.data ();
-  gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
+  gdb::byte_vector regp (register_size (gdbarch, reg->regnum));
   char *p;
 
   if (m_features.packet_support (PACKET_P) == PACKET_DISABLE)
@@ -8873,8 +8873,8 @@ remote_target::store_register_using_P (const struct regcache *regcache,
 
   xsnprintf (buf, get_remote_packet_size (), "P%s=", phex_nz (reg->pnum, 0));
   p = buf + strlen (buf);
-  regcache->raw_collect (reg->regnum, regp);
-  bin2hex (regp, p, register_size (gdbarch, reg->regnum));
+  regcache->raw_collect (reg->regnum, regp.data ());
+  bin2hex (regp.data (), p, register_size (gdbarch, reg->regnum));
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
 
@@ -8900,30 +8900,26 @@ remote_target::store_registers_using_G (const struct regcache *regcache)
 {
   struct remote_state *rs = get_remote_state ();
   remote_arch_state *rsa = rs->get_remote_arch_state (regcache->arch ());
-  gdb_byte *regs;
-  char *p;
 
-  /* Extract all the registers in the regcache copying them into a
-     local buffer.  */
-  {
-    int i;
+  /* Extract all the registers in the REGCACHE copying them into a local
+     buffer REGS.
 
-    regs = (gdb_byte *) alloca (rsa->sizeof_g_packet);
-    memset (regs, 0, rsa->sizeof_g_packet);
-    for (i = 0; i < gdbarch_num_regs (regcache->arch ()); i++)
-      {
-	struct packet_reg *r = &rsa->regs[i];
+     Use std::vector<gdb_byte> here (instead of gdb::byte_vector) because
+     we want the contents to be zero initialized.  */
+  std::vector<gdb_byte> regs (rsa->sizeof_g_packet);
+  for (int i = 0; i < gdbarch_num_regs (regcache->arch ()); i++)
+    {
+      struct packet_reg *r = &rsa->regs[i];
 
-	if (r->in_g_packet)
-	  regcache->raw_collect (r->regnum, regs + r->offset);
-      }
-  }
+      if (r->in_g_packet)
+	regcache->raw_collect (r->regnum, regs.data () + r->offset);
+    }
 
   /* Command describes registers byte by byte,
      each byte encoded as two hex characters.  */
-  p = rs->buf.data ();
+  char *p = rs->buf.data ();
   *p++ = 'G';
-  bin2hex (regs, p, rsa->sizeof_g_packet);
+  bin2hex (regs.data (), p, rsa->sizeof_g_packet);
   putpkt (rs->buf);
   getpkt (&rs->buf, 0);
   if (packet_check_result (rs->buf) == PACKET_ERROR)
-- 
2.25.4

[PATCH 12/13] gdb: remove uses of alloca from valprint.c

[Andrew Burgess]

Remove the uses of alloca from valprint.c and replace them with
gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/valprint.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/gdb/valprint.c b/gdb/valprint.c
index 357db3815b0..63a2a4de060 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -2203,13 +2203,13 @@ generic_emit_char (int c, struct type *type, struct ui_file *stream,
 {
   enum bfd_endian byte_order
     = type_byte_order (type);
-  gdb_byte *c_buf;
   bool need_escape = false;
 
-  c_buf = (gdb_byte *) alloca (type->length ());
-  pack_long (c_buf, type, c);
+  gdb::byte_vector c_buf (type->length ());
+  pack_long (c_buf.data (), type, c);
 
-  wchar_iterator iter (c_buf, type->length (), encoding, type->length ());
+  wchar_iterator iter (c_buf.data (), type->length (), encoding,
+		       type->length ());
 
   /* This holds the printable form of the wchar_t data.  */
   auto_obstack wchar_buf;
@@ -2667,16 +2667,15 @@ val_print_string (struct type *elttype, const char *encoding,
 					  width, byte_order) == 0;
   if (len == -1 && !found_nul)
     {
-      gdb_byte *peekbuf;
-
       /* We didn't find a NUL terminator we were looking for.  Attempt
 	 to peek at the next character.  If not successful, or it is not
 	 a null byte, then force ellipsis to be printed.  */
 
-      peekbuf = (gdb_byte *) alloca (width);
+      gdb::byte_vector peekbuf (width);
 
-      if (target_read_memory (addr, peekbuf, width) == 0
-	  && extract_unsigned_integer (peekbuf, width, byte_order) != 0)
+      if (target_read_memory (addr, peekbuf.data (), width) == 0
+	  && extract_unsigned_integer (peekbuf.data (),
+				       width, byte_order) != 0)
 	force_ellipsis = 1;
     }
   else if ((len >= 0 && err != 0) || (len > bytes_read / width))
-- 
2.25.4

[PATCH 13/13] gdb: remove a use of alloca from symfile.c

[Andrew Burgess]

Remove a use of alloca from symfile.c and replace it with
gdb::byte_vector.

There is another use of alloca in this file, but this is for a type
other than gdb_byte, so will need something other than
gdb::byte_vector.

There should be no user visible changes after this commit.
---
 gdb/symfile.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 373f5592107..1a9f811f0ba 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3429,13 +3429,12 @@ static void
 read_target_long_array (CORE_ADDR memaddr, unsigned int *myaddr,
 			int len, int size, enum bfd_endian byte_order)
 {
-  /* FIXME (alloca): Not safe if array is very large.  */
-  gdb_byte *buf = (gdb_byte *) alloca (len * size);
-  int i;
+  gdb::byte_vector buf (len * size);
 
-  read_memory (memaddr, buf, len * size);
-  for (i = 0; i < len; i++)
-    myaddr[i] = extract_unsigned_integer (size * i + buf, size, byte_order);
+  read_memory (memaddr, buf.data (), len * size);
+  for (int i = 0; i < len; i++)
+    myaddr[i] = extract_unsigned_integer (size * i + buf.data (), size,
+					  byte_order);
 }
 
 /* Find and grab a copy of the target _ovly_table
-- 
2.25.4

Re: [PATCH 00/13] Remove a bunch of alloca uses

[Simon Marchi]

On 2/27/23 16:29, Andrew Burgess via Gdb-patches wrote:
> Continuing effort to replace alloca calls with C++ data structures,
> here's a bunch of places where gdb::byte_vector can be used.
> 
> This isn't all of them, there's still plenty to work through, but I
> thought I'd see how this lot is received.

Every time I spot an alloca in the wild, I'm also tempted to switch it
to vector or something like that.  But then I think, it's an extra
dynamic allocation for no real gain, so it would just make things worse.

When we know the maximum size a buffer can have, instead of using
alloca, it would better to statically allocate that size on the stack,
and then user an array_view to define the effectively used portion of
it.

For the cases where we don't (I'm thinking of buffer the size of
registers, often small, but there isn't a statically known maximum
length), perhaps an std::vector-like structure with small size
optimization (like std::string has) would be nice.  So you could define
a buffer that would use stack memory up to let's say 128 bytes, and heap
memory above that.  Some references:

  https://stoyannk.wordpress.com/2017/11/18/small-vector-optimization/
  https://llvm.org/doxygen/classllvm_1_1SmallVector.html
  https://github.com/facebook/folly/blob/main/folly/docs/small_vector.md
  https://www.boost.org/doc/libs/1_60_0/doc/html/boost/container/small_vector.html

With something like this, I think we could change pretty much all
allocas without feeling bad about it.

If the license allows, I'd be all for just importing an existing
implementation in our code base, to avoid reinventing the wheel.

Simon

Re: [PATCH 00/13] Remove a bunch of alloca uses

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Every time I spot an alloca in the wild, I'm also tempted to switch it
Simon> to vector or something like that.  But then I think, it's an extra
Simon> dynamic allocation for no real gain, so it would just make things worse.

I'd imagine these allocations probably don't matter in most cases, but
on the other hand, alloca is a hazard, as it's easy to accidentally blow
up the stack.

Simon> When we know the maximum size a buffer can have, instead of using
Simon> alloca, it would better to statically allocate that size on the stack,
Simon> and then user an array_view to define the effectively used portion of
Simon> it.

True, and for registers in particular we could just have a #define
somewhere probably.  At least, assuming they have some maximum size
across all supported arches.

Simon> If the license allows, I'd be all for just importing an existing
Simon> implementation in our code base, to avoid reinventing the wheel.

This would also be fine with me but I tend to think in most cases it's
probably unnecessary.  Like, I wouldn't blink if new code came in using
a local vector<> but I might mention a new alloca in a review.

Tom