From 1a36815e2f1ad39acf894660759cf1a0bef39fcc Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Mon, 27 Feb 2023 13:47:10 +0000 Subject: [PATCH] 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. --- gdb/printcmd.c | 49 ++++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 27 deletions(-) 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 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; -- 2.30.2