| Message ID | 201408271221.s7RCL684028429@d06av02.portsmouth.uk.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 27 Aug 2014, Ulrich Weigand wrote: > Andrew Pinski wrote: > > > I think this patch broke MIPS64 n32 big-endian support. We assert here: > > gdb_assert (!gdbarch_convert_register_p (gdbarch, regnum, type)); > > > > The convert_register_p code for MIPS does: > > return (register_size (gdbarch, regnum) == 8 > > && regnum % num_regs > 0 && regnum % num_regs < 32 > > && TYPE_LENGTH (type) < 8); > > > > > > Since the register size is 8 byte wide (MIPS64) and the type length is > > 4 (pointer), we return true. In MIPS64, the registers are stored > > 64bits but pointers are 32bits. > > > > Here is the code that is used by mips_register_to_value: > > int len = TYPE_LENGTH (type); > > CORE_ADDR offset; > > > > offset = gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG ? 8 - len : 0; > > if (!get_frame_register_bytes (frame, regnum, offset, len, to, > > optimizedp, unavailablep)) > > return 0; > > > > *optimizedp = *unavailablep = 0; > > return 1; > > Huh, I wasn't aware of that conversion. Note that for the register_to_value > case, I don't actually see any difference to the default behavior; it's the > value_to_register routine that's really special (because of the sign-extension > in performs). > > > Is there a way to fix this in a target neutral way? (I might need a > > way like this for AARCH64 ILP32 also). > > I guess it isn't too hard to support gdbarch_convert_register_p in that > routine as well; I just didn't have any target to test on. > > Can you try whether something along the following lines works for you? I'll see if I can push it through testing, though it may take a few days as some of MIPS hardware I use (and especially 64-bit one) is slooow (and I already have a test run under way). I'd expect the issue to be consistent across all ILP32 64-bit ABIs BTW, that is e.g. x32 x86-64 as well. Maciej
On Wed, Aug 27, 2014 at 5:21 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Andrew Pinski wrote: > >> I think this patch broke MIPS64 n32 big-endian support. We assert here: >> gdb_assert (!gdbarch_convert_register_p (gdbarch, regnum, type)); >> >> The convert_register_p code for MIPS does: >> return (register_size (gdbarch, regnum) == 8 >> && regnum % num_regs > 0 && regnum % num_regs < 32 >> && TYPE_LENGTH (type) < 8); >> >> >> Since the register size is 8 byte wide (MIPS64) and the type length is >> 4 (pointer), we return true. In MIPS64, the registers are stored >> 64bits but pointers are 32bits. >> >> Here is the code that is used by mips_register_to_value: >> int len = TYPE_LENGTH (type); >> CORE_ADDR offset; >> >> offset = gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG ? 8 - len : 0; >> if (!get_frame_register_bytes (frame, regnum, offset, len, to, >> optimizedp, unavailablep)) >> return 0; >> >> *optimizedp = *unavailablep = 0; >> return 1; > > Huh, I wasn't aware of that conversion. Note that for the register_to_value > case, I don't actually see any difference to the default behavior; it's the > value_to_register routine that's really special (because of the sign-extension > in performs). > >> Is there a way to fix this in a target neutral way? (I might need a >> way like this for AARCH64 ILP32 also). > > I guess it isn't too hard to support gdbarch_convert_register_p in that > routine as well; I just didn't have any target to test on. > > Can you try whether something along the following lines works for you? Yes this works for me. Thanks, Andrew Pinski > > Bye, > Ulrich > > ChangeLog: > > * findvar.c (address_from_register): Handle targets requiring > a special conversion routine even for plain pointer types. > > diff --git a/gdb/findvar.c b/gdb/findvar.c > index 41887de..ba3dd4d 100644 > --- a/gdb/findvar.c > +++ b/gdb/findvar.c > @@ -764,11 +764,28 @@ address_from_register (int regnum, struct frame_info *frame) > would therefore abort in get_frame_id. However, since we only need > a temporary value that is never used as lvalue, we actually do not > really need to set its VALUE_FRAME_ID. Therefore, we re-implement > - the core of value_from_register, but use the null_frame_id. > + the core of value_from_register, but use the null_frame_id. */ > > - This works only if we do not require a special conversion routine, > - which is true for plain pointer types for all current targets. */ > - gdb_assert (!gdbarch_convert_register_p (gdbarch, regnum, type)); > + /* Some targets require a special conversion routine even for plain > + pointer types. Avoid constructing a value object in those cases. */ > + if (gdbarch_convert_register_p (gdbarch, regnum, type)) > + { > + gdb_byte *buf = alloca (TYPE_LENGTH (type)); > + int optim, unavail, ok; > + > + ok = gdbarch_register_to_value (gdbarch, frame, regnum, type, > + buf, &optim, &unavail); > + if (!ok) > + { > + /* This function is used while computing a location expression. > + Complain about the value being optimized out, rather than > + letting value_as_address complain about some random register > + the expression depends on not being saved. */ > + error_value_optimized_out (); > + } > + > + return unpack_long (type, buf); > + } > > value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id); > read_frame_register_value (value, frame); > > -- > Dr. Ulrich Weigand > GNU/Linux compilers and toolchain > Ulrich.Weigand@de.ibm.com >
On Wed, 27 Aug 2014, Maciej W. Rozycki wrote: > > I guess it isn't too hard to support gdbarch_convert_register_p in that > > routine as well; I just didn't have any target to test on. > > > > Can you try whether something along the following lines works for you? > > I'll see if I can push it through testing, though it may take a few days > as some of MIPS hardware I use (and especially 64-bit one) is slooow (and > I already have a test run under way). I had results a few days ago already, but I had to find some time to filter out intermittent failures. I have done it now and the results look good. For the record, I tested the mips-linux-gnu target and the following multilibs: -EB -EB -msoft-float -EB -mips16 -EB -mips16 -msoft-float -EB -mmicromips -EB -mmicromips -msoft-float -EB -mabi=n32 -EB -mabi=n32 -msoft-float -EB -mabi=64 -EB -mabi=64 -msoft-float and the -EL variants of same. My toolchain defaults to and test hardware used implement the MIPS32r2 or MIPS64r2 ISA as applicable. Maciej
diff --git a/gdb/findvar.c b/gdb/findvar.c index 41887de..ba3dd4d 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -764,11 +764,28 @@ address_from_register (int regnum, struct frame_info *frame) would therefore abort in get_frame_id. However, since we only need a temporary value that is never used as lvalue, we actually do not really need to set its VALUE_FRAME_ID. Therefore, we re-implement - the core of value_from_register, but use the null_frame_id. + the core of value_from_register, but use the null_frame_id. */ - This works only if we do not require a special conversion routine, - which is true for plain pointer types for all current targets. */ - gdb_assert (!gdbarch_convert_register_p (gdbarch, regnum, type)); + /* Some targets require a special conversion routine even for plain + pointer types. Avoid constructing a value object in those cases. */ + if (gdbarch_convert_register_p (gdbarch, regnum, type)) + { + gdb_byte *buf = alloca (TYPE_LENGTH (type)); + int optim, unavail, ok; + + ok = gdbarch_register_to_value (gdbarch, frame, regnum, type, + buf, &optim, &unavail); + if (!ok) + { + /* This function is used while computing a location expression. + Complain about the value being optimized out, rather than + letting value_as_address complain about some random register + the expression depends on not being saved. */ + error_value_optimized_out (); + } + + return unpack_long (type, buf); + } value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id); read_frame_register_value (value, frame);