From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11854 invoked by alias); 13 Jul 2011 17:45:09 -0000 Received: (qmail 11694 invoked by uid 22791); 13 Jul 2011 17:45:03 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_05,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_CP,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 13 Jul 2011 17:44:41 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6DHiePu004696 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 13 Jul 2011 13:44:40 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p6DHieeU028032; Wed, 13 Jul 2011 13:44:40 -0400 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p6DHicVs022354; Wed, 13 Jul 2011 13:44:39 -0400 From: Tom Tromey To: gdb-patches@sourceware.org Subject: RFC: partially available registers Date: Wed, 13 Jul 2011 20:17:00 -0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-07/txt/msg00353.txt.bz2 --=-=-= Content-Type: text/plain Content-length: 2243 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=, b=, c=, d=, e=, f=6, g=7, h=8, i=9) at typeddwarf.c:30 30 } (gdb) p s $1 = However, 's' is not really unavailable: (gdb) info addr s Symbol "s" is a complex DWARF expression: 0: DW_OP_GNU_regval_type [$xmm2] 3: DW_OP_GNU_convert 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 , but that is incorrect. I'll turn the test case into part of the patch when I finalize this change. Tom --=-=-= Content-Type: text/plain Content-Disposition: inline; filename=typeddwarf.c Content-Description: typeddwarf.c Content-length: 3763 /* { 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; } --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=P Content-Description: the patch Content-length: 11893 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 + * 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 + * dwarf2expr.c (execute_stack_op) : 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; } --=-=-=--