This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RFC: partially available registers
- From: Tom Tromey <tromey at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 13 Jul 2011 11:44:38 -0600
- Subject: RFC: partially available registers
I have a test case (I've attached the C source) which exhibits bad
behavior in gdb on x86-64:
(gdb) b 30
Breakpoint 1 at 0x400463: file typeddwarf.c, line 30.
(gdb) r
Starting program: /tmp/t
Breakpoint 1, f1 (a=<unavailable>, b=<unavailable>, c=<unavailable>,
d=<unavailable>, e=<unavailable>, f=6, g=7, h=8, i=9) at typeddwarf.c:30
30 }
(gdb) p s
$1 = <unavailable>
However, 's' is not really unavailable:
(gdb) info addr s
Symbol "s" is a complex DWARF expression:
0: DW_OP_GNU_regval_type<double [0x2d]> [$xmm2]
3: DW_OP_GNU_convert<long int [0x42]>
5: DW_OP_GNU_convert<0>
7: DW_OP_stack_value
.
(gdb) p (long) $xmm2.v2_double[0]
$3 = 3
The issue here is that DWARF uses the same register numbers for the XMM
and YMM registers, and in this case the high parts of the YMM registers
are unavailable. This causes the special code in
i386_pseudo_register_read for YMM to return REG_UNAVAILABLE.
This patch fixes the problem by letting an arch register a new
pseudo_register_read_value method, which is responsible for constructing
a struct value for the register. This gives us a chance to mark
just some bits unavailable.
With the modified gdb, the test works:
(gdb) b 30
Breakpoint 1 at 0x400463: file typeddwarf.c, line 30.
(gdb) r
Starting program: /tmp/t
Breakpoint 1, f1 (a=1, b=2, c=3, d=4, e=5, f=6, g=7, h=8, i=9)
at typeddwarf.c:30
30 }
(gdb) p s
$1 = 3
I would appreciate feedback on this patch.
I considered several other approaches:
* Put the XCR0 bits into the regcache. This would let us dynamically
decide whether to return the XMM or YMM register. This seemed to mean
treating XCR0 as a real register (which AFAICT it is not, right now),
which meant also gdbserver changes.
* Rather than a way to return values, have a different API, say one
where gdb requests the first N bytes of a register. This may still be
cleaner, I am not sure. Optionally this could be the only way,
meaning a patch touching most existing callers.
I don't think this patch yet hits all the spots I would need to change.
E.g., "print $ymm2" shows all fields as <unavailable>, but that is
incorrect.
I'll turn the test case into part of the patch when I finalize this
change.
Tom
/* { dg-do run { target { i?86-*-* x86_64-*-* } } } */
/* { dg-options "-g" } */
typedef __SIZE_TYPE__ size_t;
volatile int vv;
extern void *memcpy (void *, const void *, size_t);
__attribute__((noinline, noclone)) void
f1 (double a, double b, double c, float d, float e, int f, unsigned int g, long long h, unsigned long long i)
{
double j = d; /* { dg-final { gdb-test 29 "j" "4" } } */
long long l; /* { dg-final { gdb-test 29 "l" "4616189618054758400" } } */
memcpy (&l, &j, sizeof (l));
long long m; /* { dg-final { gdb-test 29 "m" "4613937818241073152" } } */
memcpy (&m, &c, sizeof (l));
float n = i; /* { dg-final { gdb-test 29 "n" "9" } } */
double o = h; /* { dg-final { gdb-test 29 "o" "8" } } */
float p = g; /* { dg-final { gdb-test 29 "p" "7" } } */
double q = f; /* { dg-final { gdb-test 29 "q" "6" } } */
unsigned long long r = a; /* { dg-final { gdb-test 29 "r" "1" } } */
long long s = c; /* { dg-final { gdb-test 29 "s" "3" } } */
unsigned t = d; /* { dg-final { gdb-test 29 "t" "4" } } */
int u = b; /* { dg-final { gdb-test 29 "u" "2" } } */
float v = a; /* { dg-final { gdb-test 29 "v" "1" } } */
double w = d / 4.0; /* { dg-final { gdb-test 29 "w" "1" } } */
double x = a + b + 1.0; /* { dg-final { gdb-test 29 "x" "4" } } */
double y = b + c + 2.0; /* { dg-final { gdb-test 29 "y" "7" } } */
float z = d + e + 3.0f; /* { dg-final { gdb-test 29 "z" "12" } } */
vv++;
}
__attribute__((noinline, noclone)) void
f2 (double a, double b, double c, float d, float e, int f, unsigned int g, long long h, unsigned long long i)
{
double j = d; /* { dg-final { gdb-test 53 "j" "4" } } */
long long l; /* { dg-final { gdb-test 53 "l" "4616189618054758400" } } */
memcpy (&l, &j, sizeof (l));
long long m; /* { dg-final { gdb-test 53 "m" "4613937818241073152" } } */
memcpy (&m, &c, sizeof (l));
float n = i; /* { dg-final { gdb-test 53 "n" "9" } } */
double o = h; /* { dg-final { gdb-test 53 "o" "8" } } */
float p = g; /* { dg-final { gdb-test 53 "p" "7" } } */
double q = f; /* { dg-final { gdb-test 53 "q" "6" } } */
unsigned long long r = a; /* { dg-final { gdb-test 53 "r" "1" } } */
long long s = c; /* { dg-final { gdb-test 53 "s" "3" } } */
unsigned t = d; /* { dg-final { gdb-test 53 "t" "4" } } */
int u = b; /* { dg-final { gdb-test 53 "u" "2" } } */
float v = a; /* { dg-final { gdb-test 53 "v" "1" } } */
double w = d / 4.0; /* { dg-final { gdb-test 53 "w" "1" } } */
double x = a + b - 3 + 1.0e20;/* { dg-final { gdb-test 53 "x" "1e+20" } } */
double y = b + c * 7.0; /* { dg-final { gdb-test 53 "y" "23" } } */
float z = d + e + 3.0f; /* { dg-final { gdb-test 53 "z" "12" } } */
vv++;
vv = a;
vv = b;
vv = c;
vv = d;
vv = e;
vv = f;
vv = g;
vv = h;
vv = i;
vv = j;
}
__attribute__((noinline, noclone)) void
f3 (long long a, int b, long long c, unsigned d)
{
long long w = (a > d) ? a : d;/* { dg-final { gdb-test 73 "w" "4" } } */
long long x = a + b + 7; /* { dg-final { gdb-test 73 "x" "10" } } */
long long y = c + d + 0x912345678LL;/* { dg-final { gdb-test 73 "y" "38960125567" } } */
int z = (x + y); /* { dg-final { gdb-test 73 "z" "305419913" } } */
vv++;
}
__attribute__((noinline, noclone)) void
f4 (_Decimal32 a, _Decimal64 b, _Decimal128 c)
{
_Decimal32 w = a * 8.0DF + 6.0DF;/* { dg-final { gdb-test 82 "(int)w" "70" } } */
_Decimal64 x = b / 8.0DD - 6.0DD;/* { dg-final { gdb-test 82 "(int)x" "-4" } } */
_Decimal128 y = -c / 8.0DL; /* { dg-final { gdb-test 82 "(int)y" "-8" } } */
vv++;
}
int
main ()
{
f1 (1.0, 2.0, 3.0, 4.0f, 5.0f, 6, 7, 8, 9);
f2 (1.0, 2.0, 3.0, 4.0f, 5.0f, 6, 7, 8, 9);
f3 (1, 2, 3, 4);
f4 (8.0DF, 16.0DD, 64.0DL);
return 0;
}
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4062bf9..b404878 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,20 @@
2011-07-13 Tom Tromey <tromey@redhat.com>
+ * sentinel-frame.c (sentinel_frame_prev_register): Use
+ regcache_cooked_read_into_value.
+ * regcache.h (regcache_cooked_read_into_value): Declare.
+ * regcache.c (regcache_cooked_read_into_value): New function.
+ * i386-tdep.h (i386_pseudo_register_read_value): Declare.
+ * i386-tdep.c (i386_pseudo_register_read_value): New function.
+ (i386_gdbarch_init): Call set_gdbarch_pseudo_register_read_value.
+ * gdbarch.sh (pseudo_register_read_value): New method.
+ * gdbarch.c, gdbarch.h: Rebuild.
+ * findvar.c (value_from_register): Call get_frame_register_value.
+ * amd64-tdep.c (amd64_init_abi): Call
+ set_gdbarch_pseudo_register_read_value.
+
+2011-07-13 Tom Tromey <tromey@redhat.com>
+
* dwarf2expr.c (execute_stack_op) <DW_OP_GNU_regval_type>: Use
value_from_contents for final conversion.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index de62ac7..c728b53 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -2496,6 +2496,8 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
set_gdbarch_pseudo_register_read (gdbarch,
amd64_pseudo_register_read);
+ set_gdbarch_pseudo_register_read_value (gdbarch,
+ i386_pseudo_register_read_value);
set_gdbarch_pseudo_register_write (gdbarch,
amd64_pseudo_register_write);
diff --git a/gdb/findvar.c b/gdb/findvar.c
index a700c02..59e86ef 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -647,14 +647,16 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
else
{
int len = TYPE_LENGTH (type);
+ struct value *v2;
/* Construct the value. */
v = gdbarch_value_from_register (gdbarch, type, regnum, frame);
/* Get the data. */
- ok = get_frame_register_bytes (frame, regnum, value_offset (v), len,
- value_contents_raw (v),
- &optim, &unavail);
+ v2 = get_frame_register_value (frame, regnum);
+
+ value_contents_copy (v, 0, v2, 0, len);
+ ok = 1;
}
if (!ok)
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 1e65c17..0c1bcf0 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -161,6 +161,7 @@ struct gdbarch
gdbarch_write_pc_ftype *write_pc;
gdbarch_virtual_frame_pointer_ftype *virtual_frame_pointer;
gdbarch_pseudo_register_read_ftype *pseudo_register_read;
+ gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value;
gdbarch_pseudo_register_write_ftype *pseudo_register_write;
int num_regs;
int num_pseudo_regs;
@@ -313,6 +314,7 @@ struct gdbarch startup_gdbarch =
0, /* write_pc */
legacy_virtual_frame_pointer, /* virtual_frame_pointer */
0, /* pseudo_register_read */
+ 0, /* pseudo_register_read_value */
0, /* pseudo_register_write */
0, /* num_regs */
0, /* num_pseudo_regs */
@@ -594,6 +596,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
/* Skip verify of write_pc, has predicate. */
/* Skip verify of virtual_frame_pointer, invalid_p == 0 */
/* Skip verify of pseudo_register_read, has predicate. */
+ /* Skip verify of pseudo_register_read_value, has predicate. */
/* Skip verify of pseudo_register_write, has predicate. */
if (gdbarch->num_regs == -1)
fprintf_unfiltered (log, "\n\tnum_regs");
@@ -1085,6 +1088,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
"gdbarch_dump: pseudo_register_read = <%s>\n",
host_address_to_string (gdbarch->pseudo_register_read));
fprintf_unfiltered (file,
+ "gdbarch_dump: gdbarch_pseudo_register_read_value_p() = %d\n",
+ gdbarch_pseudo_register_read_value_p (gdbarch));
+ fprintf_unfiltered (file,
+ "gdbarch_dump: pseudo_register_read_value = <%s>\n",
+ host_address_to_string (gdbarch->pseudo_register_read_value));
+ fprintf_unfiltered (file,
"gdbarch_dump: gdbarch_pseudo_register_write_p() = %d\n",
gdbarch_pseudo_register_write_p (gdbarch));
fprintf_unfiltered (file,
@@ -1700,6 +1709,30 @@ set_gdbarch_pseudo_register_read (struct gdbarch *gdbarch,
}
int
+gdbarch_pseudo_register_read_value_p (struct gdbarch *gdbarch)
+{
+ gdb_assert (gdbarch != NULL);
+ return gdbarch->pseudo_register_read_value != NULL;
+}
+
+int
+gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, struct value *result)
+{
+ gdb_assert (gdbarch != NULL);
+ gdb_assert (gdbarch->pseudo_register_read_value != NULL);
+ if (gdbarch_debug >= 2)
+ fprintf_unfiltered (gdb_stdlog, "gdbarch_pseudo_register_read_value called\n");
+ return gdbarch->pseudo_register_read_value (gdbarch, regcache, cookednum, result);
+}
+
+void
+set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch,
+ gdbarch_pseudo_register_read_value_ftype pseudo_register_read_value)
+{
+ gdbarch->pseudo_register_read_value = pseudo_register_read_value;
+}
+
+int
gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch)
{
gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 50221d7..5f99ded 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -216,6 +216,12 @@ typedef enum register_status (gdbarch_pseudo_register_read_ftype) (struct gdbarc
extern enum register_status gdbarch_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, gdb_byte *buf);
extern void set_gdbarch_pseudo_register_read (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_ftype *pseudo_register_read);
+extern int gdbarch_pseudo_register_read_value_p (struct gdbarch *gdbarch);
+
+typedef int (gdbarch_pseudo_register_read_value_ftype) (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, struct value *result);
+extern int gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, struct value *result);
+extern void set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value);
+
extern int gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch);
typedef void (gdbarch_pseudo_register_write_ftype) (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, const gdb_byte *buf);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index a628f8c..1e2ad53 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -418,6 +418,7 @@ F:void:write_pc:struct regcache *regcache, CORE_ADDR val:regcache, val
m:void:virtual_frame_pointer:CORE_ADDR pc, int *frame_regnum, LONGEST *frame_offset:pc, frame_regnum, frame_offset:0:legacy_virtual_frame_pointer::0
#
M:enum register_status:pseudo_register_read:struct regcache *regcache, int cookednum, gdb_byte *buf:regcache, cookednum, buf
+M:int:pseudo_register_read_value:struct regcache *regcache, int cookednum, struct value *result:regcache, cookednum, result
M:void:pseudo_register_write:struct regcache *regcache, int cookednum, const gdb_byte *buf:regcache, cookednum, buf
#
v:int:num_regs:::0:-1
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 366d0fa..7a16b4a 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2854,6 +2854,47 @@ i386_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
return REG_VALID;
}
+int
+i386_pseudo_register_read_value (struct gdbarch *gdbarch,
+ struct regcache *regcache,
+ int regnum,
+ struct value *result)
+{
+ gdb_byte raw_buf[MAX_REGISTER_SIZE];
+ gdb_byte *buf;
+ struct gdbarch_tdep *tdep;
+ enum register_status status;
+
+ if (!i386_ymm_regnum_p (gdbarch, regnum))
+ return 0;
+
+ tdep = gdbarch_tdep (gdbarch);
+ buf = value_contents_raw (result);
+
+ regnum -= tdep->ymm0_regnum;
+
+ /* Extract (always little endian). Read lower 128bits. */
+ status = regcache_raw_read (regcache,
+ I387_XMM0_REGNUM (tdep) + regnum,
+ raw_buf);
+
+ if (status != REG_VALID)
+ mark_value_bytes_unavailable (result, 0, 16);
+ else
+ memcpy (buf, raw_buf, 16);
+
+ /* Read upper 128bits. */
+ status = regcache_raw_read (regcache,
+ tdep->ymm0h_regnum + regnum,
+ raw_buf);
+ if (status != REG_VALID)
+ mark_value_bytes_unavailable (result, 16, 32);
+ else
+ memcpy (buf + 16, raw_buf, 16);
+
+ return 1;
+}
+
void
i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
int regnum, const gdb_byte *buf)
@@ -7334,6 +7375,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
/* Pseudo registers may be changed by amd64_init_abi. */
set_gdbarch_pseudo_register_read (gdbarch, i386_pseudo_register_read);
+ set_gdbarch_pseudo_register_read_value (gdbarch,
+ i386_pseudo_register_read_value);
set_gdbarch_pseudo_register_write (gdbarch, i386_pseudo_register_write);
set_tdesc_pseudo_register_type (gdbarch, i386_pseudo_register_type);
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 7fc719c..97fa258 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -315,6 +315,12 @@ extern enum register_status i386_pseudo_register_read (struct gdbarch *gdbarch,
struct regcache *regcache,
int regnum,
gdb_byte *buf);
+
+extern int i386_pseudo_register_read_value (struct gdbarch *gdbarch,
+ struct regcache *regcache,
+ int regnum,
+ struct value *result);
+
extern void i386_pseudo_register_write (struct gdbarch *gdbarch,
struct regcache *regcache,
int regnum, const gdb_byte *buf);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 41f218d..2ddcd4c 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -714,6 +714,23 @@ regcache_cooked_read (struct regcache *regcache, int regnum, gdb_byte *buf)
regnum, buf);
}
+void
+regcache_cooked_read_into_value (struct regcache *regcache, int regnum,
+ struct value *result)
+{
+ gdb_assert (regnum >= 0);
+ gdb_assert (regnum < regcache->descr->nr_cooked_registers);
+ if (!gdbarch_pseudo_register_read_value_p (regcache->descr->gdbarch)
+ || !gdbarch_pseudo_register_read_value (regcache->descr->gdbarch,
+ regcache, regnum, result))
+ {
+ if (regcache_cooked_read (regcache, regnum,
+ value_contents_raw (result)) == REG_UNAVAILABLE)
+ mark_value_bytes_unavailable (result, 0,
+ TYPE_LENGTH (value_type (result)));
+ }
+}
+
enum register_status
regcache_cooked_read_signed (struct regcache *regcache, int regnum,
LONGEST *val)
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 3708c86..01ee30b 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -104,6 +104,9 @@ enum register_status regcache_cooked_read (struct regcache *regcache,
void regcache_cooked_write (struct regcache *regcache, int rawnum,
const gdb_byte *buf);
+void regcache_cooked_read_into_value (struct regcache *regcache, int regnum,
+ struct value *result);
+
/* Read a register as a signed/unsigned quantity. */
extern enum register_status
regcache_cooked_read_signed (struct regcache *regcache,
diff --git a/gdb/sentinel-frame.c b/gdb/sentinel-frame.c
index 6c2f3e0..53d82ba 100644
--- a/gdb/sentinel-frame.c
+++ b/gdb/sentinel-frame.c
@@ -62,10 +62,7 @@ sentinel_frame_prev_register (struct frame_info *this_frame,
/* Use the regcache_cooked_read() method so that it, on the fly,
constructs either a raw or pseudo register from the raw
register cache. */
- if (regcache_cooked_read (cache->regcache,
- regnum,
- value_contents_raw (value)) == REG_UNAVAILABLE)
- mark_value_bytes_unavailable (value, 0, TYPE_LENGTH (regtype));
+ regcache_cooked_read_into_value (cache->regcache, regnum, value);
return value;
}