Re: [PATCH] gdb: use std::vector instead of alloca in core_target::get_core_register_section

["Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>]

On Sun, Jan 12, 2020, 15:17 Simon Marchi <simon.marchi@polymtl.ca> wrote:

> As I was trying to compile gdb for an m68k host, I got this error:
>
>   CXX    corelow.o
> In file included from /binutils-gdb/gdb/gdbsupport/common-defs.h:120,
>                  from /binutils-gdb/gdb/defs.h:28,
>                  from /binutils-gdb/gdb/corelow.c:20:
> /binutils-gdb/gdb/corelow.c: In member function 'void
> core_target::get_core_register_section(regcache*, const regset*, const
> char*, int, int, const char*, bool)':
> /binutils-gdb/gdb/../include/libiberty.h:727:36: error: 'alloca' bound is
> unknown [-Werror=alloca-larger-than=]
>   727 | # define alloca(x) __builtin_alloca(x)
>       |                    ~~~~~~~~~~~~~~~~^~~
> /binutils-gdb/gdb/corelow.c:625:23: note: in expansion of macro 'alloca'
>   625 |   contents = (char *) alloca (size);
>       |                       ^~~~~~
>
> We are using alloca to hold the contents of a the core register
> sections.  These sections are typically fairly small, but there is no
> realy guarantee, so I think it would be more reasonable to just use
> dynamic allocation here.
>

This seems to be in a common file, why did it only fail on m68k?


> gdb/ChangeLog:
>
>         * corelow.c (core_target::get_core_register_section): Use
>           std::vector instead of alloca.
> ---
>  gdb/corelow.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index c53bf1df8fd8..e7efa652d62a 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -594,7 +594,6 @@ core_target::get_core_register_section (struct
> regcache *regcache,
>  {
>    struct bfd_section *section;
>    bfd_size_type size;
> -  char *contents;
>    bool variable_size_section = (regset != NULL
>                                 && regset->flags & REGSET_VARIABLE_SIZE);
>
> @@ -622,8 +621,8 @@ core_target::get_core_register_section (struct
> regcache *regcache,
>                section_name.c_str ());
>      }
>
> -  contents = (char *) alloca (size);
> -  if (! bfd_get_section_contents (core_bfd, section, contents,
> +  std::vector<char> contents (size);
> +  if (! bfd_get_section_contents (core_bfd, section, contents.data (),
>

Since you're touching this, isn't usual style not to have a space after the
! ?

                                  (file_ptr) 0, size))
>      {
>        warning (_("Couldn't read %s registers from `%s' section in core
> file."),
> @@ -633,12 +632,12 @@ core_target::get_core_register_section (struct
> regcache *regcache,
>
>    if (regset != NULL)
>      {
> -      regset->supply_regset (regset, regcache, -1, contents, size);
> +      regset->supply_regset (regset, regcache, -1, contents.data (),
> size);
>        return;
>      }
>
>    gdb_assert (m_core_vec != nullptr);
> -  m_core_vec->core_read_registers (regcache, contents, size, which,
> +  m_core_vec->core_read_registers (regcache, contents.data (), size,
> which,
>                                    (CORE_ADDR) bfd_section_vma (section));
>  }
>
> --
> 2.24.1
>
>