From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
To: gdb-patches@sourceware.org
Cc: "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>,
Simon Marchi <simark@simark.ca>,
"Maciej W. Rozycki" <macro@orcam.me.uk>
Subject: [PATCH 2/2] GDB: Use gdb::array_view for buffers used in register reading and unwinding
Date: Fri, 10 Jan 2025 13:44:00 -0300 [thread overview]
Message-ID: <20250110164430.3376697-3-thiago.bauermann@linaro.org> (raw)
In-Reply-To: <20250110164430.3376697-1-thiago.bauermann@linaro.org>
This allows checking the size of the given buffer. Changes
frame_register_unwind (), frame_unwind_register (), get_frame_register ()
and deprecated_frame_register_read ().
As pointed out by Baris, in the case of MIPS target code this is best
done by changing a couple of alloca-based buffers in
mips_read_fp_register_single and mips_print_fp_register to
gdb::byte_vector instances.
---
gdb/amd64-windows-tdep.c | 4 ++--
gdb/frame.c | 22 ++++++++++--------
gdb/frame.h | 10 ++++-----
gdb/mips-tdep.c | 48 ++++++++++++++++++++++------------------
4 files changed, 47 insertions(+), 37 deletions(-)
Changes from RFCv4 patch 2:
- Allocated gdb::byte_vectors instead of making array_views in mips-tdep.c,
as suggested by Baris. This required changing raw_buffer in
mips_print_fp_register (the caller of mips_read_fp_register_single and
mips_read_fp_register_double) to a gdb::byte_vector as well, and also
changing the rare_buffer arguments to array views.
- Don't use std::optional for the buffer argument in frame_register_unwind,
as suggested by Simon.
- Changed the gdb_assert in frame_register_unwind to require the buffer
argument to have the same size as the register.
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 9e255bb2d433..ca7b7d96bc22 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -801,7 +801,7 @@ amd64_windows_frame_decode_insns (const frame_info_ptr &this_frame,
std::array<gdb_byte, 8> buf;
int frreg = amd64_windows_w2gdb_regnum[frame_reg];
- get_frame_register (this_frame, frreg, buf.data ());
+ get_frame_register (this_frame, frreg, buf);
save_addr = extract_unsigned_integer (buf, byte_order);
frame_debug_printf (" frame_reg=%s, val=%s",
@@ -1097,7 +1097,7 @@ amd64_windows_frame_cache (const frame_info_ptr &this_frame, void **this_cache)
/* Get current PC and SP. */
pc = get_frame_pc (this_frame);
- get_frame_register (this_frame, AMD64_RSP_REGNUM, buf.data ());
+ get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
cache->sp = extract_unsigned_integer (buf, byte_order);
cache->pc = pc;
diff --git a/gdb/frame.c b/gdb/frame.c
index ba4a07179f64..b64f9067ab9e 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1112,7 +1112,7 @@ frame_save_as_regcache (const frame_info_ptr &this_frame)
{
auto cooked_read = [this_frame] (int regnum, gdb::array_view<gdb_byte> buf)
{
- if (!deprecated_frame_register_read (this_frame, regnum, buf.data ()))
+ if (!deprecated_frame_register_read (this_frame, regnum, buf))
return REG_UNAVAILABLE;
else
return REG_VALID;
@@ -1177,7 +1177,8 @@ void
frame_register_unwind (const frame_info_ptr &next_frame, int regnum,
int *optimizedp, int *unavailablep,
enum lval_type *lvalp, CORE_ADDR *addrp,
- int *realnump, gdb_byte *bufferp)
+ int *realnump,
+ gdb::array_view<gdb_byte> buffer)
{
struct value *value;
@@ -1202,13 +1203,15 @@ frame_register_unwind (const frame_info_ptr &next_frame, int regnum,
else
*realnump = -1;
- if (bufferp)
+ if (!buffer.empty ())
{
+ gdb_assert (buffer.size () == value->type ()->length ());
+
if (!*optimizedp && !*unavailablep)
- memcpy (bufferp, value->contents_all ().data (),
+ memcpy (buffer.data (), value->contents_all ().data (),
value->type ()->length ());
else
- memset (bufferp, 0, value->type ()->length ());
+ memset (buffer.data (), 0, value->type ()->length ());
}
/* Dispose of the new value. This prevents watchpoints from
@@ -1217,7 +1220,8 @@ frame_register_unwind (const frame_info_ptr &next_frame, int regnum,
}
void
-frame_unwind_register (const frame_info_ptr &next_frame, int regnum, gdb_byte *buf)
+frame_unwind_register (const frame_info_ptr &next_frame, int regnum,
+ gdb::array_view<gdb_byte> buf)
{
int optimized;
int unavailable;
@@ -1238,7 +1242,7 @@ frame_unwind_register (const frame_info_ptr &next_frame, int regnum, gdb_byte *b
void
get_frame_register (const frame_info_ptr &frame,
- int regnum, gdb_byte *buf)
+ int regnum, gdb::array_view<gdb_byte> buf)
{
frame_unwind_register (frame_info_ptr (frame->next), regnum, buf);
}
@@ -1482,7 +1486,7 @@ put_frame_register (const frame_info_ptr &next_frame, int regnum,
bool
deprecated_frame_register_read (const frame_info_ptr &frame, int regnum,
- gdb_byte *myaddr)
+ gdb::array_view<gdb_byte> myaddr)
{
int optimized;
int unavailable;
@@ -1541,7 +1545,7 @@ get_frame_register_bytes (const frame_info_ptr &next_frame, int regnum,
int realnum;
frame_register_unwind (next_frame, regnum, optimizedp, unavailablep,
- &lval, &addr, &realnum, buffer.data ());
+ &lval, &addr, &realnum, buffer);
if (*optimizedp || *unavailablep)
return false;
}
diff --git a/gdb/frame.h b/gdb/frame.h
index b265c9bc5bb3..e207c714c560 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -685,14 +685,14 @@ const char *unwind_stop_reason_to_string (enum unwind_stop_reason);
const char *frame_stop_reason_string (const frame_info_ptr &);
/* Unwind the stack frame so that the value of REGNUM, in the previous
- (up, older) frame is returned. If VALUEP is nullptr, don't
+ (up, older) frame is returned. If VALUE is zero-sized, don't
fetch/compute the value. Instead just return the location of the
value. */
extern void frame_register_unwind (const frame_info_ptr &frame, int regnum,
int *optimizedp, int *unavailablep,
enum lval_type *lvalp,
CORE_ADDR *addrp, int *realnump,
- gdb_byte *valuep = nullptr);
+ gdb::array_view<gdb_byte> value = {});
/* Fetch a register from this, or unwind a register from the next
frame. Note that the get_frame methods are wrappers to
@@ -701,9 +701,9 @@ extern void frame_register_unwind (const frame_info_ptr &frame, int regnum,
do return a lazy value. */
extern void frame_unwind_register (const frame_info_ptr &next_frame,
- int regnum, gdb_byte *buf);
+ int regnum, gdb::array_view<gdb_byte> buf);
extern void get_frame_register (const frame_info_ptr &frame,
- int regnum, gdb_byte *buf);
+ int regnum, gdb::array_view<gdb_byte> buf);
struct value *frame_unwind_register_value (const frame_info_ptr &next_frame,
int regnum);
@@ -889,7 +889,7 @@ extern void print_frame_info (const frame_print_options &fp_opts,
extern frame_info_ptr block_innermost_frame (const struct block *);
extern bool deprecated_frame_register_read (const frame_info_ptr &frame, int regnum,
- gdb_byte *buf);
+ gdb::array_view<gdb_byte> buf);
/* From stack.c. */
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index c00efbd02ad0..d217f23631f8 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -951,14 +951,17 @@ mips_register_to_value (const frame_info_ptr &frame, int regnum,
if (mips_convert_register_float_case_p (gdbarch, regnum, type))
{
- get_frame_register (frame, regnum + 0, to + 4);
- get_frame_register (frame, regnum + 1, to + 0);
+ gdb::array_view<gdb_byte> first_half = gdb::make_array_view (to, 4);
+ gdb::array_view<gdb_byte> second_half = gdb::make_array_view (to + 4, 4);
- if (!get_frame_register_bytes (next_frame, regnum + 0, 0, { to + 4, 4 },
+ get_frame_register (frame, regnum + 0, second_half);
+ get_frame_register (frame, regnum + 1, first_half);
+
+ if (!get_frame_register_bytes (next_frame, regnum + 0, 0, second_half,
optimizedp, unavailablep))
return 0;
- if (!get_frame_register_bytes (next_frame, regnum + 1, 0, { to + 0, 4 },
+ if (!get_frame_register_bytes (next_frame, regnum + 1, 0, first_half,
optimizedp, unavailablep))
return 0;
*optimizedp = *unavailablep = 0;
@@ -6252,11 +6255,11 @@ mips_o64_return_value (struct gdbarch *gdbarch, struct value *function,
static void
mips_read_fp_register_single (const frame_info_ptr &frame, int regno,
- gdb_byte *rare_buffer)
+ gdb::array_view<gdb_byte> rare_buffer)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
int raw_size = register_size (gdbarch, regno);
- gdb_byte *raw_buffer = (gdb_byte *) alloca (raw_size);
+ gdb::byte_vector raw_buffer (raw_size);
if (!deprecated_frame_register_read (frame, regno, raw_buffer))
error (_("can't read register %d (%s)"),
@@ -6272,11 +6275,11 @@ mips_read_fp_register_single (const frame_info_ptr &frame, int regno,
else
offset = 0;
- memcpy (rare_buffer, raw_buffer + offset, 4);
+ memcpy (rare_buffer.data (), raw_buffer.data () + offset, 4);
}
else
{
- memcpy (rare_buffer, raw_buffer, 4);
+ memcpy (rare_buffer.data (), raw_buffer.data (), 4);
}
}
@@ -6286,7 +6289,7 @@ mips_read_fp_register_single (const frame_info_ptr &frame, int regno,
static void
mips_read_fp_register_double (const frame_info_ptr &frame, int regno,
- gdb_byte *rare_buffer)
+ gdb::array_view<gdb_byte> rare_buffer)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
int raw_size = register_size (gdbarch, regno);
@@ -6311,13 +6314,14 @@ mips_read_fp_register_double (const frame_info_ptr &frame, int regno,
each register. */
if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
{
- mips_read_fp_register_single (frame, regno, rare_buffer + 4);
+ mips_read_fp_register_single (frame, regno, rare_buffer.slice (4));
mips_read_fp_register_single (frame, regno + 1, rare_buffer);
}
else
{
mips_read_fp_register_single (frame, regno, rare_buffer);
- mips_read_fp_register_single (frame, regno + 1, rare_buffer + 4);
+ mips_read_fp_register_single (frame, regno + 1,
+ rare_buffer.slice (4));
}
}
}
@@ -6327,15 +6331,13 @@ mips_print_fp_register (struct ui_file *file, const frame_info_ptr &frame,
int regnum)
{ /* Do values for FP (float) regs. */
struct gdbarch *gdbarch = get_frame_arch (frame);
- gdb_byte *raw_buffer;
std::string flt_str, dbl_str;
const struct type *flt_type = builtin_type (gdbarch)->builtin_float;
const struct type *dbl_type = builtin_type (gdbarch)->builtin_double;
- raw_buffer
- = ((gdb_byte *)
- alloca (2 * register_size (gdbarch, mips_regnum (gdbarch)->fp0)));
+ gdb::byte_vector raw_buffer (2 * register_size (gdbarch,
+ mips_regnum (gdbarch)->fp0));
gdb_printf (file, "%s:", gdbarch_register_name (gdbarch, regnum));
gdb_printf (file, "%*s",
@@ -6349,10 +6351,11 @@ mips_print_fp_register (struct ui_file *file, const frame_info_ptr &frame,
/* 4-byte registers: Print hex and floating. Also print even
numbered registers as doubles. */
mips_read_fp_register_single (frame, regnum, raw_buffer);
- flt_str = target_float_to_string (raw_buffer, flt_type, "%-17.9g");
+ flt_str = target_float_to_string (raw_buffer.data (), flt_type,
+ "%-17.9g");
get_formatted_print_options (&opts, 'x');
- print_scalar_formatted (raw_buffer,
+ print_scalar_formatted (raw_buffer.data (),
builtin_type (gdbarch)->builtin_uint32,
&opts, 'w', file);
@@ -6361,7 +6364,8 @@ mips_print_fp_register (struct ui_file *file, const frame_info_ptr &frame,
if ((regnum - gdbarch_num_regs (gdbarch)) % 2 == 0)
{
mips_read_fp_register_double (frame, regnum, raw_buffer);
- dbl_str = target_float_to_string (raw_buffer, dbl_type, "%-24.17g");
+ dbl_str = target_float_to_string (raw_buffer.data (), dbl_type,
+ "%-24.17g");
gdb_printf (file, " dbl: %s", dbl_str.c_str ());
}
@@ -6372,13 +6376,15 @@ mips_print_fp_register (struct ui_file *file, const frame_info_ptr &frame,
/* Eight byte registers: print each one as hex, float and double. */
mips_read_fp_register_single (frame, regnum, raw_buffer);
- flt_str = target_float_to_string (raw_buffer, flt_type, "%-17.9g");
+ flt_str = target_float_to_string (raw_buffer.data (), flt_type,
+ "%-17.9g");
mips_read_fp_register_double (frame, regnum, raw_buffer);
- dbl_str = target_float_to_string (raw_buffer, dbl_type, "%-24.17g");
+ dbl_str = target_float_to_string (raw_buffer.data (), dbl_type,
+ "%-24.17g");
get_formatted_print_options (&opts, 'x');
- print_scalar_formatted (raw_buffer,
+ print_scalar_formatted (raw_buffer.data (),
builtin_type (gdbarch)->builtin_uint64,
&opts, 'g', file);
next prev parent reply other threads:[~2025-01-10 16:45 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-10 16:43 [PATCH 0/2] Improvements to a few GDB frame unwind functions Thiago Jung Bauermann
2025-01-10 16:43 ` [PATCH 1/2] GDB: frame: Make VALUEP argument optional in frame_register_unwind Thiago Jung Bauermann
2025-01-10 17:02 ` Simon Marchi
2025-01-10 22:23 ` Thiago Jung Bauermann
2025-01-10 16:44 ` Thiago Jung Bauermann [this message]
2025-01-10 17:45 ` [PATCH 2/2] GDB: Use gdb::array_view for buffers used in register reading and unwinding Simon Marchi
2025-01-10 22:28 ` Thiago Jung Bauermann
2025-01-11 13:54 ` Tom de Vries
2025-01-11 15:46 ` Thiago Jung Bauermann
2025-01-11 16:01 ` Tom de Vries
2025-01-12 0:32 ` Thiago Jung Bauermann
2025-01-13 16:40 ` Tom de Vries
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250110164430.3376697-3-thiago.bauermann@linaro.org \
--to=thiago.bauermann@linaro.org \
--cc=gdb-patches@sourceware.org \
--cc=macro@orcam.me.uk \
--cc=simark@simark.ca \
--cc=tankut.baris.aktemur@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox