From: Andrew Burgess <aburgess@redhat.com>
Date: Mon, 27 Feb 2023 13:47:10 +0000 (+0000)
Subject: gdb: remove two uses of alloca from printcmd.c
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=1a36815e2f1ad39acf894660759cf1a0bef39fcc;p=binutils-gdb.git

gdb: remove two uses of alloca from printcmd.c

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

An earlier variant of this patch was proposed in this thread:

  https://inbox.sourceware.org/gdb-patches/cover.1677533215.git.aburgess@redhat.com/

however, there was push back on that thread due to it adding extra
dynamic allocation, i.e. moving the memory buffers off the stack on to
the heap.

However, of all the patches originally proposed, I think in these two
cases moving off the stack is the correct thing to do.  Unlike all the
other patches in the original series, where the data being read
was (mostly) small in size, a register, or a couple of registers, in
this case we are reading an arbitrary string from the inferior.  This
could be any size, and so should not be placed on the stack.

So in this commit I replace the use of alloca with std::byte_vector
and simplify the logic a little (I think) to take advantage of the
ability of std::byte_vector to dynamically grow in size.

Of course, really, we should probably be checking the max-value-size
setting as we load the string to stop GDB crashing if a corrupted
inferior causes GDB to try read a stupidly large amount of
memory... but I'm leaving that for a follow on patch.

There should be no user visible changes after this commit.
---

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index d8d97493bab..31f87c71e5d 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2447,7 +2447,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)
        || value->type ()->code () == TYPE_CODE_ARRAY)
@@ -2459,11 +2459,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
     {
@@ -2478,31 +2477,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
 }
 
@@ -2521,6 +2519,7 @@ printf_wide_c_string (struct ui_file *stream, const char *format,
   struct type *wctype = lookup_typename (current_language,
 					 "wchar_t", NULL, 0);
   int wcwidth = wctype->length ();
+  gdb::optional<gdb::byte_vector> tem_str;
 
   if (value->lval () == lval_internalvar
       && c_is_string_type_p (value->type ()))
@@ -2543,23 +2542,19 @@ 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);
+      tem_str.emplace ();
 
       for (len = 0;; len += wcwidth)
 	{
 	  QUIT;
-	  read_memory (tem + len, buf, wcwidth);
-	  if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0)
+	  tem_str->resize (tem_str->size () + wcwidth);
+	  gdb_byte *dst = tem_str->data () + len;
+	  read_memory (tem + len, dst, wcwidth);
+	  if (extract_unsigned_integer (dst, wcwidth, byte_order) == 0)
 	    break;
 	}
 
-      /* Copy the string contents into a string inside GDB.  */
-      gdb_byte *tem_str = (gdb_byte *) alloca (len + wcwidth);
-
-      if (len != 0)
-	read_memory (tem, tem_str, len);
-      memset (&tem_str[len], 0, wcwidth);
-      str = tem_str;
+      str = tem_str->data ();
     }
 
   auto_obstack output;