This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Remove MAX_REGISTER_SIZE from frame.c
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Alan Hayward <Alan dot Hayward at arm dot com>
- Cc: "gdb-patches\@sourceware.org" <gdb-patches at sourceware dot org>, nd <nd at arm dot com>
- Date: Wed, 01 Mar 2017 12:32:27 +0000
- Subject: Re: [PATCH] Remove MAX_REGISTER_SIZE from frame.c
- Authentication-results: sourceware.org; auth=none
- References: <E80FFABA-2912-4223-AC55-2F4DE6055F47@arm.com>
Alan Hayward <Alan.Hayward@arm.com> writes:
> @@ -1252,7 +1252,11 @@ frame_unwind_register_signed (struct frame_info *frame, int regnum)
> struct gdbarch *gdbarch = frame_unwind_arch (frame);
> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> int size = register_size (gdbarch, regnum);
> - gdb_byte buf[MAX_REGISTER_SIZE];
> + gdb_byte buf[sizeof (LONGEST)];
> +
> + if (size > (int) sizeof (LONGEST))
> + error (_("Cannot unwind integers more than %d bytes."),
> + (int) sizeof (LONGEST));
>
We apply the restriction of extract_signed_integer to its caller here.
People will wonder why do we have such check until he/she digs into
extract_signed_integer. My first reaction is to add some comments to
explain why do we do so, but the recent gdb::function_view reminds me
that we can do something differently (and better, IMO).
Current pattern of using extract_unsigned_integer is
1) allocate an array on stack,
2) read data from regcache or frame into the array,
3) pass the array to extract_unsigned_integer
we can pass a callable function_view as a content provider to
extract_unsigned_integer, so that we don't need step 1). The code
becomes,
return extract_unsigned_integer ([&] (gdb_byte *buf, size_t size)
{
frame_unwind_register (frame, regnum, buf);
}, size, byte_order);
We can remove some uses of MAX_REGISTER_SIZE in this way. Do you (Alan
and others) like the patch below?
> @@ -1460,11 +1473,15 @@ put_frame_register_bytes (struct frame_info *frame, int regnum,
> }
> else
> {
> - gdb_byte buf[MAX_REGISTER_SIZE];
> -
> - deprecated_frame_register_read (frame, regnum, buf);
> - memcpy (buf + offset, myaddr, curr_len);
> - put_frame_register (frame, regnum, buf);
> + struct value *value = frame_unwind_register_value (frame->next,
> + regnum);
> + gdb_assert (value != NULL);
> +
> + memcpy ((char *) value_contents_all (value) + offset, myaddr,
> + curr_len);
Use value_contents_writeable,
> + put_frame_register (frame, regnum, value_contents_all (value));
s/value_contents_all/value_contents_raw/
because value_contents_all requires value is available but
deprecated_frame_register_read doesn't.
> + release_value (value);
> + value_free (value);
--
Yao (齐尧)
From 2a76b39ffa87ed015f3637ee4a9d083f682863a0 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Wed, 1 Mar 2017 10:43:29 +0000
Subject: [PATCH] Use content provider in extract_unsigned_integer
---
gdb/ada-lang.c | 9 ++++++---
gdb/defs.h | 4 ++++
gdb/findvar.c | 38 ++++++++++++++++++++++++++++++--------
gdb/frame.c | 7 ++++---
gdb/ia64-tdep.c | 19 +++++++++++++++----
5 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 753409c..07ce04a 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4555,12 +4555,15 @@ value_pointer (struct value *value, struct type *type)
{
struct gdbarch *gdbarch = get_type_arch (type);
unsigned len = TYPE_LENGTH (type);
- gdb_byte *buf = (gdb_byte *) alloca (len);
CORE_ADDR addr;
addr = value_address (value);
- gdbarch_address_to_pointer (gdbarch, type, buf, addr);
- addr = extract_unsigned_integer (buf, len, gdbarch_byte_order (gdbarch));
+ addr = extract_unsigned_integer ([&] (gdb_byte *buf, size_t size)
+ {
+ gdbarch_address_to_pointer (gdbarch,
+ type, buf,
+ addr);
+ }, len, gdbarch_byte_order (gdbarch));
return addr;
}
diff --git a/gdb/defs.h b/gdb/defs.h
index aa58605..a2f8fce 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -628,6 +628,10 @@ extern LONGEST extract_signed_integer (const gdb_byte *, int,
extern ULONGEST extract_unsigned_integer (const gdb_byte *, int,
enum bfd_endian);
+extern ULONGEST
+ extract_unsigned_integer (gdb::function_view<void (gdb_byte *, size_t size)> content_provider,
+ int len, enum bfd_endian byte_order);
+
extern int extract_long_unsigned_integer (const gdb_byte *, int,
enum bfd_endian, LONGEST *);
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 80c709a..6d7d0de 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -81,20 +81,15 @@ That operation is not available on integers of more than %d bytes."),
return retval;
}
-ULONGEST
-extract_unsigned_integer (const gdb_byte *addr, int len,
- enum bfd_endian byte_order)
+static ULONGEST
+extract_unsigned_integer_1 (const gdb_byte *addr, int len,
+ enum bfd_endian byte_order)
{
ULONGEST retval;
const unsigned char *p;
const unsigned char *startaddr = addr;
const unsigned char *endaddr = startaddr + len;
- if (len > (int) sizeof (ULONGEST))
- error (_("\
-That operation is not available on integers of more than %d bytes."),
- (int) sizeof (ULONGEST));
-
/* Start at the most significant end of the integer, and work towards
the least significant. */
retval = 0;
@@ -111,6 +106,33 @@ That operation is not available on integers of more than %d bytes."),
return retval;
}
+ULONGEST
+extract_unsigned_integer (const gdb_byte *addr, int len,
+ enum bfd_endian byte_order)
+{
+ if (len > (int) sizeof (ULONGEST))
+ error (_("\
+That operation is not available on integers of more than %d bytes."),
+ (int) sizeof (ULONGEST));
+
+ return extract_unsigned_integer_1 (addr, len, byte_order);
+}
+
+ULONGEST
+extract_unsigned_integer (gdb::function_view<void (gdb_byte *, size_t size)> content_provider,
+ int len, enum bfd_endian byte_order)
+{
+ if (len > (int) sizeof (ULONGEST))
+ error (_("\
+That operation is not available on integers of more than %d bytes."),
+ (int) sizeof (ULONGEST));
+
+ gdb_byte buf[sizeof (ULONGEST)];
+
+ content_provider (buf, len);
+ return extract_unsigned_integer_1 (buf, len, byte_order);
+}
+
/* Sometimes a long long unsigned integer can be extracted as a
LONGEST value. This is done so that we can print these values
better. If this integer can be converted to a LONGEST, this
diff --git a/gdb/frame.c b/gdb/frame.c
index d98003d..57f82f1 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1270,10 +1270,11 @@ frame_unwind_register_unsigned (struct frame_info *frame, int regnum)
struct gdbarch *gdbarch = frame_unwind_arch (frame);
enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
int size = register_size (gdbarch, regnum);
- gdb_byte buf[MAX_REGISTER_SIZE];
- frame_unwind_register (frame, regnum, buf);
- return extract_unsigned_integer (buf, size, byte_order);
+ return extract_unsigned_integer ([&] (gdb_byte *buf, size_t size)
+ {
+ frame_unwind_register (frame, regnum, buf);
+ }, size, byte_order);
}
ULONGEST
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 4c53bc6..2748dac 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -1516,7 +1516,6 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
else if (qp == 0 && rN == 2
&& ((rM == fp_reg && fp_reg != 0) || rM == 12))
{
- gdb_byte buf[MAX_REGISTER_SIZE];
CORE_ADDR saved_sp = 0;
/* adds r2, spilloffset, rFramePointer
or
@@ -1534,8 +1533,15 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
{
struct gdbarch *gdbarch = get_frame_arch (this_frame);
enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
- get_frame_register (this_frame, sp_regnum, buf);
- saved_sp = extract_unsigned_integer (buf, 8, byte_order);
+
+ saved_sp
+ = extract_unsigned_integer ([&] (gdb_byte * buf,
+ size_t size)
+ {
+ get_frame_register (this_frame,
+ sp_regnum,
+ buf);
+ }, 8, byte_order);
}
spill_addr = saved_sp
+ (rM == 12 ? 0 : mem_stack_frame_size)
@@ -1782,7 +1788,12 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
else if (cfm_reg != 0)
{
get_frame_register (this_frame, cfm_reg, buf);
- cfm = extract_unsigned_integer (buf, 8, byte_order);
+ cfm = extract_unsigned_integer ([&] (gdb_byte * buf, size_t size)
+ {
+ get_frame_register (this_frame,
+ cfm_reg,
+ buf);
+ }, 8, byte_order);
}
cache->prev_cfm = cfm;
--
1.9.1