From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5366 invoked by alias); 8 Dec 2006 15:51:19 -0000 Received: (qmail 5344 invoked by uid 22791); 8 Dec 2006 15:51:14 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate4.de.ibm.com (HELO mtagate4.de.ibm.com) (195.212.29.153) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 08 Dec 2006 15:51:04 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate4.de.ibm.com (8.13.8/8.13.8) with ESMTP id kB8Fp0de132246 for ; Fri, 8 Dec 2006 15:51:01 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.6/8.13.6/NCO v8.1.1) with ESMTP id kB8FovVM3088472 for ; Fri, 8 Dec 2006 16:50:57 +0100 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id kB8Fou8c011726 for ; Fri, 8 Dec 2006 16:50:57 +0100 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id kB8FoueU011723; Fri, 8 Dec 2006 16:50:56 +0100 Message-Id: <200612081550.kB8FoueU011723@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Fri, 8 Dec 2006 16:50:56 +0100 Subject: [RFC][1/2] Rework value_from_register To: gdb-patches@sourceware.org Date: Fri, 08 Dec 2006 15:51:00 -0000 From: "Ulrich Weigand" Cc: jimb@codesourcery.com, drow@false.org X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: 2006-12/txt/msg00122.txt.bz2 Hello, this patch attempts to clean up register value processing in value_from_register and value_assign. There are two major changes: - value_from_register used to attempt to create a value of type lval_memory when accessing a register that was saved to some stack frame. This appears to be unnecessary these days; as lval_register values are now relative to a particular stack frame, they're just as useful as lval_memory. - I've extracted the code iterating over successive registers and accessing a sub-range of the bytes formed by the concatenation of their contents into two helper functions: get_frame_register_bytes put_frame_register_bytes This allows the lval_register case in value_assign to be written in a very similar way to the lval_memory case. Tested without regressions on spu-elf, s390-ibm-linux, s390x-ibm-linux. OK? Bye, Ulrich ChangeLog: * frame.c (get_frame_register_bytes): New function. (put_frame_register_bytes): Likewise. * frame.h (get_frame_register_bytes): Declare. (put_frame_register_bytes): Likewise. * findvar.c (value_from_register): Always construct lval_register values. Use get_frame_register_bytes. * valops.c (value_assign): Use get_frame_register_bytes and put_frame_register_bytes. diff -urp gdb-orig/gdb/findvar.c gdb-head/gdb/findvar.c --- gdb-orig/gdb/findvar.c 2006-11-24 17:56:28.000000000 +0100 +++ gdb-head/gdb/findvar.c 2006-12-08 13:39:40.750402896 +0100 @@ -599,11 +599,7 @@ addresses have not been bound by the dyn return v; } -/* Return a value of type TYPE, stored in register REGNUM, in frame - FRAME. - - NOTE: returns NULL if register value is not available. - Caller will check return value or die! */ +/* Return a value of type TYPE, stored in register REGNUM, in frame FRAME. */ struct value * value_from_register (struct type *type, int regnum, struct frame_info *frame) @@ -612,30 +608,11 @@ value_from_register (struct type *type, struct value *v = allocate_value (type); CHECK_TYPEDEF (type); - if (TYPE_LENGTH (type) == 0) - { - /* It doesn't matter much what we return for this: since the - length is zero, it could be anything. But if allowed to see - a zero-length type, the register-finding loop below will set - neither mem_stor nor reg_stor, and then report an internal - error. - - Zero-length types can legitimately arise from declarations - like 'struct {}' (a GCC extension, not valid ISO C). GDB may - also create them when it finds bogus debugging information; - for example, in GCC 2.95.4 and binutils 2.11.93.0.2, the - STABS BINCL->EXCL compression process can create bad type - numbers. GDB reads these as TYPE_CODE_UNDEF types, with zero - length. (That bug is actually the only known way to get a - zero-length value allocated to a register --- which is what - it takes to make it here.) - - We'll just attribute the value to the original register. */ - VALUE_LVAL (v) = lval_register; - VALUE_ADDRESS (v) = regnum; - VALUE_REGNUM (v) = regnum; - } - else if (CONVERT_REGISTER_P (regnum, type)) + VALUE_LVAL (v) = lval_register; + VALUE_FRAME_ID (v) = get_frame_id (frame); + VALUE_REGNUM (v) = regnum; + + if (CONVERT_REGISTER_P (regnum, type)) { /* The ISA/ABI need to something weird when obtaining the specified value from this register. It might need to @@ -645,85 +622,27 @@ value_from_register (struct type *type, is that REGISTER_TO_VALUE populates the entire value including the location. */ REGISTER_TO_VALUE (frame, regnum, type, value_contents_raw (v)); - VALUE_LVAL (v) = lval_register; - VALUE_FRAME_ID (v) = get_frame_id (frame); - VALUE_REGNUM (v) = regnum; } else { - int local_regnum; - int mem_stor = 0, reg_stor = 0; - int mem_tracking = 1; - CORE_ADDR last_addr = 0; - CORE_ADDR first_addr = 0; - int first_realnum = regnum; + struct gdbarch *gdbarch = get_frame_arch (frame); int len = TYPE_LENGTH (type); - int value_bytes_copied; - int optimized = 0; - gdb_byte *value_bytes = alloca (len + MAX_REGISTER_SIZE); - - /* Copy all of the data out, whereever it may be. */ - for (local_regnum = regnum, value_bytes_copied = 0; - value_bytes_copied < len; - (value_bytes_copied += register_size (current_gdbarch, local_regnum), - ++local_regnum)) - { - int realnum; - int optim; - enum lval_type lval; - CORE_ADDR addr; - frame_register (frame, local_regnum, &optim, &lval, &addr, - &realnum, value_bytes + value_bytes_copied); - optimized += optim; - if (register_cached (local_regnum) == -1) - return NULL; /* register value not available */ - - if (regnum == local_regnum) - { - first_addr = addr; - first_realnum = realnum; - } - if (lval == lval_register) - reg_stor++; - else - { - mem_stor++; - - /* FIXME: cagney/2004-11-12: I think this is trying to - check that the stored registers are adjacent in - memory. It isn't doing a good job? */ - mem_tracking = (mem_tracking - && (regnum == local_regnum - || addr == last_addr)); - } - last_addr = addr; - } - - if (mem_tracking && mem_stor && !reg_stor) - { - VALUE_LVAL (v) = lval_memory; - VALUE_ADDRESS (v) = first_addr; - } - else - { - VALUE_LVAL (v) = lval_register; - VALUE_FRAME_ID (v) = get_frame_id (frame); - VALUE_REGNUM (v) = regnum; - } - - set_value_optimized_out (v, optimized); - + /* Any structure stored in more than one register will always be an integral number of registers. Otherwise, you need to do some fiddling with the last register copied here for little endian machines. */ if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG - && len < register_size (current_gdbarch, regnum)) + && len < register_size (gdbarch, regnum)) /* Big-endian, and we want less than full size. */ - set_value_offset (v, register_size (current_gdbarch, regnum) - len); + set_value_offset (v, register_size (gdbarch, regnum) - len); else set_value_offset (v, 0); - memcpy (value_contents_raw (v), value_bytes + value_offset (v), len); + + /* Get the data. */ + if (!get_frame_register_bytes (frame, regnum, value_offset (v), len, + value_contents_raw (v))) + set_value_optimized_out (v, 1); } return v; } diff -urp gdb-orig/gdb/frame.c gdb-head/gdb/frame.c --- gdb-orig/gdb/frame.c 2006-11-22 14:36:19.000000000 +0100 +++ gdb-head/gdb/frame.c 2006-12-08 14:03:49.216427736 +0100 @@ -742,6 +742,90 @@ frame_register_read (struct frame_info * return !optimized; } +/* Read LEN bytes from one or multiple registers starting with REGNUM + in frame FRAME, starting at OFFSET, into BUF. */ + +int +get_frame_register_bytes (struct frame_info *frame, int regnum, + CORE_ADDR offset, int len, gdb_byte *myaddr) +{ + struct gdbarch *gdbarch = get_frame_arch (frame); + + /* Skip registers wholly inside of OFFSET. */ + while (offset >= register_size (gdbarch, regnum)) + { + offset -= register_size (gdbarch, regnum); + regnum++; + } + + /* Copy the data. */ + while (len > 0) + { + int curr_len = register_size (gdbarch, regnum) - offset; + if (curr_len > len) + curr_len = len; + + if (curr_len == register_size (gdbarch, regnum)) + { + if (!frame_register_read (frame, regnum, myaddr)) + return 0; + } + else + { + gdb_byte buf[MAX_REGISTER_SIZE]; + if (!frame_register_read (frame, regnum, buf)) + return 0; + memcpy (myaddr, buf + offset, curr_len); + } + + len -= curr_len; + offset = 0; + regnum++; + } + + return 1; +} + +/* Write LEN bytes to one or multiple registers starting with REGNUM + in frame FRAME, starting at OFFSET, into BUF. */ + +void +put_frame_register_bytes (struct frame_info *frame, int regnum, + CORE_ADDR offset, int len, const gdb_byte *myaddr) +{ + struct gdbarch *gdbarch = get_frame_arch (frame); + + /* Skip registers wholly inside of OFFSET. */ + while (offset >= register_size (gdbarch, regnum)) + { + offset -= register_size (gdbarch, regnum); + regnum++; + } + + /* Copy the data. */ + while (len > 0) + { + int curr_len = register_size (gdbarch, regnum) - offset; + if (curr_len > len) + curr_len = len; + + if (curr_len == register_size (gdbarch, regnum)) + { + put_frame_register (frame, regnum, myaddr); + } + else + { + gdb_byte buf[MAX_REGISTER_SIZE]; + frame_register_read (frame, regnum, buf); + memcpy (buf + offset, myaddr, curr_len); + put_frame_register (frame, regnum, buf); + } + + len -= curr_len; + offset = 0; + regnum++; + } +} /* Map between a frame register number and its name. A frame register space is a superset of the cooked register space --- it also diff -urp gdb-orig/gdb/frame.h gdb-head/gdb/frame.h --- gdb-orig/gdb/frame.h 2006-11-22 14:36:19.000000000 +0100 +++ gdb-head/gdb/frame.h 2006-12-08 13:39:06.435426216 +0100 @@ -491,6 +491,15 @@ extern void frame_register (struct frame extern void put_frame_register (struct frame_info *frame, int regnum, const gdb_byte *buf); +/* Get and put bytes into one or multiple registers. */ + +extern int get_frame_register_bytes (struct frame_info *frame, int regnum, + CORE_ADDR offset, int len, + gdb_byte *myaddr); +extern void put_frame_register_bytes (struct frame_info *frame, int regnum, + CORE_ADDR offset, int len, + const gdb_byte *myaddr); + /* Map between a frame register number and its name. A frame register space is a superset of the cooked register space --- it also includes builtin registers. If NAMELEN is negative, use the NAME's diff -urp gdb-orig/gdb/valops.c gdb-head/gdb/valops.c --- gdb-orig/gdb/valops.c 2006-11-24 17:56:28.000000000 +0100 +++ gdb-head/gdb/valops.c 2006-12-08 13:54:02.499410496 +0100 @@ -642,8 +642,7 @@ value_assign (struct value *toval, struc if (!frame) error (_("Value being assigned to is no longer active.")); - if (VALUE_LVAL (toval) == lval_register - && CONVERT_REGISTER_P (VALUE_REGNUM (toval), type)) + if (CONVERT_REGISTER_P (VALUE_REGNUM (toval), type)) { /* If TOVAL is a special machine register requiring conversion of program values to a special raw format. */ @@ -652,59 +651,40 @@ value_assign (struct value *toval, struc } else { - /* TOVAL is stored in a series of registers in the frame - specified by the structure. Copy that value out, - modify it, and copy it back in. */ - int amount_copied; - int amount_to_copy; - gdb_byte *buffer; - int reg_offset; - int byte_offset; - int regno; - - /* Locate the first register that falls in the value that - needs to be transfered. Compute the offset of the - value in that register. */ - { - int offset; - for (reg_offset = value_reg, offset = 0; - offset + register_size (current_gdbarch, reg_offset) <= value_offset (toval); - reg_offset++); - byte_offset = value_offset (toval) - offset; - } - - /* Compute the number of register aligned values that need - to be copied. */ if (value_bitsize (toval)) - amount_to_copy = byte_offset + 1; - else - amount_to_copy = byte_offset + TYPE_LENGTH (type); - - /* And a bounce buffer. Be slightly over generous. */ - buffer = alloca (amount_to_copy + MAX_REGISTER_SIZE); - - /* Copy it in. */ - for (regno = reg_offset, amount_copied = 0; - amount_copied < amount_to_copy; - amount_copied += register_size (current_gdbarch, regno), regno++) - frame_register_read (frame, regno, buffer + amount_copied); - - /* Modify what needs to be modified. */ - if (value_bitsize (toval)) - modify_field (buffer + byte_offset, - value_as_long (fromval), - value_bitpos (toval), value_bitsize (toval)); - else - memcpy (buffer + byte_offset, value_contents (fromval), - TYPE_LENGTH (type)); - - /* Copy it out. */ - for (regno = reg_offset, amount_copied = 0; - amount_copied < amount_to_copy; - amount_copied += register_size (current_gdbarch, regno), regno++) - put_frame_register (frame, regno, buffer + amount_copied); + { + int changed_len; + gdb_byte buffer[sizeof (LONGEST)]; + changed_len = (value_bitpos (toval) + + value_bitsize (toval) + + HOST_CHAR_BIT - 1) + / HOST_CHAR_BIT; + + if (changed_len > (int) sizeof (LONGEST)) + error (_("Can't handle bitfields which don't fit in a %d bit word."), + (int) sizeof (LONGEST) * HOST_CHAR_BIT); + + get_frame_register_bytes (frame, value_reg, + value_offset (toval), + changed_len, buffer); + + modify_field (buffer, value_as_long (fromval), + value_bitpos (toval), value_bitsize (toval)); + + put_frame_register_bytes (frame, value_reg, + value_offset (toval), + changed_len, buffer); + } + else + { + put_frame_register_bytes (frame, value_reg, + value_offset (toval), + TYPE_LENGTH (type), + value_contents (fromval)); + } } + if (deprecated_register_changed_hook) deprecated_register_changed_hook (-1); observer_notify_target_changed (¤t_target); -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com