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

Simon Marchi
2020-01-13T20:03:01Z

On 2020-01-13 2:40 p.m., Pedro Alves wrote:
> On 1/12/20 8:17 PM, Simon Marchi wrote:
>> - 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 (),
>> (file_ptr) 0, size))
>> {
>
> gdb::byte_vector

I used std::vector<char> because the original code also used char, and
m_core_vec->core_read_registers also accepts a char * (although I
could/should have used gdb::char_vector anyway). However, gdb_byte
probably makes more sense here. I would push the patch below as a fixup
if it looks good to you.

Simon

From 6758886d9f1f0ddce518ffec5269cc440d2fe4f5 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Mon, 13 Jan 2020 14:57:38 -0500
Subject: [PATCH] gdb: use gdb::byte_vector instead of std::vector<char> in
core_target::get_core_register_section

Since the data held by the `contents` variable is arbitrary binary data,
it should have gdb_byte elements, not char elements. Also, using
gdb::byte_vector is preferable, since it doesn't unnecessarily
zero-initialize the values.

Instead of adding a cast in the call to m_core_vec->core_read_registers,
I have changed core_read_registers' argument to be a gdb_byte* instead
of a char*.

gdb/ChangeLog:

* gdbcore.h (struct core_fns) <core_read_registers>: Change
core_reg_sect type to gdb_byte *.
* arm-nbsd-nat.c (fetch_elfcore_registers): Likewise.
* cris-tdep.c (fetch_core_registers): Likewise.
* corelow.c (core_target::get_core_register_section): Change
type of `contents` to gdb::byte_vector.
---
gdb/arm-nbsd-nat.c | 2 +-
gdb/corelow.c | 2 +-
gdb/cris-tdep.c | 2 +-
gdb/gdbcore.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 1d058a99ac1a..00f919194b95 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -397,7 +397,7 @@ arm_nbsd_nat_target::store_registers (struct regcache *regcache, int regno)

static void
fetch_elfcore_registers (struct regcache *regcache,
- char *core_reg_sect, unsigned core_reg_size,
+ gdb_byte *core_reg_sect, unsigned core_reg_size,
int which, CORE_ADDR ignore)
{
struct reg gregset;
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 0418ec2506af..5cd058d5993d 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -621,7 +621,7 @@ core_target::get_core_register_section (struct regcache *regcache,
section_name.c_str ());
}

- std::vector<char> contents (size);
+ gdb::byte_vector contents (size);
if (!bfd_get_section_contents (core_bfd, section, contents.data (),
(file_ptr) 0, size))
{
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index 22b4db917ca0..6885d237f3a3 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -3793,7 +3793,7 @@ cris_supply_gregset (struct regcache *regcache, cris_elf_gregset_t *gregsetp)

static void
fetch_core_registers (struct regcache *regcache,
- char *core_reg_sect, unsigned core_reg_size,
+ gdb_byte *core_reg_sect, unsigned core_reg_size,
int which, CORE_ADDR reg_addr)
{
cris_elf_gregset_t gregset;
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index 5b216e9c1c97..66e81dfe950f 100644
--- a/gdb/gdbcore.h
+++ b/gdb/gdbcore.h
@@ -213,7 +213,7 @@ struct core_fns
address X is at location core_reg_sect+x+reg_addr. */

void (*core_read_registers) (struct regcache *regcache,
- char *core_reg_sect,
+ gdb_byte *core_reg_sect,
unsigned core_reg_size,
int which, CORE_ADDR reg_addr);

--
2.24.1