[PATCH 3/4] gdb, gdbserver: Allocate only a sane amount of buffer when fetching registers.
Felix Willgerodt
felix.willgerodt@intel.com
Fri May 6 12:12:25 GMT 2022
A couple of functions blindly allocate a buffer of the size of
I386_MAX_REGISTER_SIZE. With the addition of AMX, this size has increased
drastically from 64 bytes to 8192. This changes these buffer allocations
to only use the actual amount needed, similar to how it is already done in
amd64-tdep.c (amd64_pseudo_register_read_value).
For the i387_collect_xsave and i387_cache_to_xsave functions any feedback is
welcome. I opted to take the middle ground and only distinguish
between "AMX" and "Not-AMX". That might be unnecessary optimization,
we could alternatively be okay with using an 8kB buffer unconditionally or
be okay with having many smaller buffer allocations.
---
gdb/i386-tdep.c | 21 ++++++++++++++++-----
gdb/i387-tdep.c | 19 ++++++++++++++-----
gdbserver/i387-fp.cc | 8 +++++++-
3 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 921b24ab60f..94106668e50 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2944,7 +2944,7 @@ i386_extract_return_value (struct gdbarch *gdbarch, struct type *type,
{
i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch);
int len = TYPE_LENGTH (type);
- gdb_byte buf[I386_MAX_REGISTER_SIZE];
+ gdb_byte buf[register_size (gdbarch, I386_ST0_REGNUM)];
/* _Float16 and _Float16 _Complex values are returned via xmm0. */
if (((type->code () == TYPE_CODE_FLT) && len == 2)
@@ -3006,7 +3006,7 @@ i386_store_return_value (struct gdbarch *gdbarch, struct type *type,
if (type->code () == TYPE_CODE_FLT)
{
ULONGEST fstat;
- gdb_byte buf[I386_MAX_REGISTER_SIZE];
+ gdb_byte buf[register_size (gdbarch, I386_ST0_REGNUM)];
if (tdep->st0_regnum < 0)
{
@@ -3591,13 +3591,13 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
int regnum,
struct value *result_value)
{
- gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
enum register_status status;
gdb_byte *buf = value_contents_raw (result_value).data ();
if (i386_mmx_regnum_p (gdbarch, regnum))
{
int fpnum = i386_mmx_regnum_to_fp_regnum (regcache, regnum);
+ gdb_byte raw_buf[register_size (gdbarch, regnum)];
/* Extract (always little endian). */
status = regcache->raw_read (fpnum, raw_buf);
@@ -3613,6 +3613,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
if (i386_bnd_regnum_p (gdbarch, regnum))
{
regnum -= tdep->bnd0_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, I387_BND0R_REGNUM (tdep))];
/* Extract (always little endian). Read lower 128bits. */
status = regcache->raw_read (I387_BND0R_REGNUM (tdep) + regnum,
@@ -3636,6 +3637,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
else if (i386_k_regnum_p (gdbarch, regnum))
{
regnum -= tdep->k0_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, tdep->k0_regnum)];
/* Extract (always little endian). */
status = regcache->raw_read (tdep->k0_regnum + regnum, raw_buf);
@@ -3647,6 +3649,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
else if (i386_zmm_regnum_p (gdbarch, regnum))
{
regnum -= tdep->zmm0_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, tdep->zmm0_regnum)];
if (regnum < num_lower_zmm_regs)
{
@@ -3698,6 +3701,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
else if (i386_ymm_regnum_p (gdbarch, regnum))
{
regnum -= tdep->ymm0_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, tdep->ymm0_regnum)];
/* Extract (always little endian). Read lower 128bits. */
status = regcache->raw_read (I387_XMM0_REGNUM (tdep) + regnum,
@@ -3717,6 +3721,8 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
else if (i386_ymm_avx512_regnum_p (gdbarch, regnum))
{
regnum -= tdep->ymm16_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, tdep->ymm0_regnum)];
+
/* Extract (always little endian). Read lower 128bits. */
status = regcache->raw_read (I387_XMM16_REGNUM (tdep) + regnum,
raw_buf);
@@ -3735,6 +3741,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
else if (i386_word_regnum_p (gdbarch, regnum))
{
int gpnum = regnum - tdep->ax_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, gpnum)];
/* Extract (always little endian). */
status = regcache->raw_read (gpnum, raw_buf);
@@ -3747,6 +3754,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
else if (i386_byte_regnum_p (gdbarch, regnum))
{
int gpnum = regnum - tdep->al_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, gpnum % 4)];
/* Extract (always little endian). We read both lower and
upper registers. */
@@ -3784,11 +3792,10 @@ void
i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
int regnum, const gdb_byte *buf)
{
- gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
-
if (i386_mmx_regnum_p (gdbarch, regnum))
{
int fpnum = i386_mmx_regnum_to_fp_regnum (regcache, regnum);
+ gdb_byte raw_buf[register_size (gdbarch, regnum)];
/* Read ... */
regcache->raw_read (fpnum, raw_buf);
@@ -3813,6 +3820,8 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
upper = extract_unsigned_integer (buf + size, size, byte_order);
/* Fetching register buffer. */
+ gdb_byte raw_buf[register_size (gdbarch,
+ I387_BND0R_REGNUM (tdep) + regnum)];
regcache->raw_read (I387_BND0R_REGNUM (tdep) + regnum,
raw_buf);
@@ -3874,6 +3883,7 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
else if (i386_word_regnum_p (gdbarch, regnum))
{
int gpnum = regnum - tdep->ax_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, gpnum)];
/* Read ... */
regcache->raw_read (gpnum, raw_buf);
@@ -3885,6 +3895,7 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
else if (i386_byte_regnum_p (gdbarch, regnum))
{
int gpnum = regnum - tdep->al_regnum;
+ gdb_byte raw_buf[register_size (gdbarch, gpnum)];
/* Read ... We read both lower and upper registers. */
regcache->raw_read (gpnum % 4, raw_buf);
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 38ffa3f967b..56239539402 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -350,7 +350,7 @@ i387_register_to_value (struct frame_info *frame, int regnum,
int *optimizedp, int *unavailablep)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
- gdb_byte from[I386_MAX_REGISTER_SIZE];
+ gdb_byte from[register_size (gdbarch, regnum)];
gdb_assert (i386_fp_regnum_p (gdbarch, regnum));
@@ -384,7 +384,7 @@ i387_value_to_register (struct frame_info *frame, int regnum,
struct type *type, const gdb_byte *from)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
- gdb_byte to[I386_MAX_REGISTER_SIZE];
+ gdb_byte to[register_size (gdbarch, regnum)];
gdb_assert (i386_fp_regnum_p (gdbarch, regnum));
@@ -1419,7 +1419,6 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch);
gdb_byte *p, *regs = (gdb_byte *) xsave;
- gdb_byte raw[I386_MAX_REGISTER_SIZE];
ULONGEST initial_xstate_bv, clear_bv, xstate_bv = 0;
unsigned int i;
/* See the comment in i387_supply_xsave(). */
@@ -1604,6 +1603,14 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
if (regclass == all)
{
+ /* This used to blindly allocate I386_MAX_REGISTER_SIZE of space.
+ With AMX that became a bit much to do unconditionally. For now
+ this seems to be the best trade-off between saving space and
+ the performance penalty for adding individual allocations. */
+ const uint32_t buf_size
+ = (tdep->xcr0 & X86_XSTATE_TILEDATA) ? I386_MAX_REGISTER_SIZE : 64;
+ gdb_byte raw[buf_size];
+
/* Check if the tilecfg register is changed. */
if ((tdep->xcr0 & X86_XSTATE_TILECFG))
{
@@ -1791,6 +1798,7 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
else
{
/* Check if REGNUM is changed. */
+ gdb_byte raw[register_size (gdbarch, regnum)];
regcache->raw_collect (regnum, raw);
switch (regclass)
@@ -1988,10 +1996,11 @@ i387_collect_xsave (const struct regcache *regcache, int regnum,
}
else
{
- int regsize;
+ int regsize = register_size (gdbarch, i);
+ gdb_byte raw[regsize];
regcache->raw_collect (i, raw);
- regsize = regcache_register_size (regcache, i);
+
p = FXSAVE_ADDR (tdep, regs, i);
if (memcmp (raw, p, regsize))
{
diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc
index 2d22b0419f8..0b80d287a47 100644
--- a/gdbserver/i387-fp.cc
+++ b/gdbserver/i387-fp.cc
@@ -265,7 +265,6 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
unsigned long val, val2;
unsigned long long xstate_bv = 0;
unsigned long long clear_bv = 0;
- char raw[8192];
char *p;
/* Amd64 has 16 xmm regs; I386 has 8 xmm regs. */
@@ -348,6 +347,13 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf)
}
}
+ /* This used to blindly allocate 64 bytes of space.
+ With AMX that became a bit much to do unconditionally. For now
+ this seems to be the best trade-off between saving space and
+ the performance penalty for adding individual allocations. */
+ const uint32_t buf_size = (x86_xcr0 & X86_XSTATE_TILEDATA) ? 8192 : 64;
+ char raw[buf_size];
+
/* Check if any x87 registers are changed. */
if ((x86_xcr0 & X86_XSTATE_X87))
{
--
2.34.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
More information about the Gdb-patches
mailing list