Re: dwarf2-frame.c read_reg problems, again ...

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Daniel Jacobowitz wrote:
> On Wed, Oct 31, 2007 at 02:51:29AM +0100, Ulrich Weigand wrote:
> > Those in turn used to be described as "builtin_type_uint32"
> > by the original rs6000_register_type.  The generic XML-based
> > machinery now apparently uses a signed integer type instead,
> > exposing the problem.
> 
> This was plainly and simply a mistake.  While I agree that changing
> them back is not a real solution to the problem you've found, I didn't
> mean to flip the signedness of all those registers.  If uint32 is in
> any sense more architecturally appropriate, or even for sheer
> tradition, let's flip them back.

I don't see how either way would be architecturally more appropriate
than the other, but it look like Andrew specifically made them 
unsigned to fix what appears to be a similar problem:
http://sourceware.org/ml/gdb-patches/2004-03/msg00176.html

So I don't mind making them unsigned again.  How would that work
with the XML definitions?  Do you have to add an explicit type
attribute everywhere?

> > Now I'm wondering: what was the motivation behind using
> > unpack_long here?   The dwarf2loc.c:dwarf_expr_read_reg 
> > routine, which saves basically the same purpose, now uses
> > address_from_register -- i.e. specifically treats the
> > value as pointer, not integer ...
> 
> I think we have about five too many ways to take a register and make
> it into a number.  On the other hand, dwarf_expr_read_reg
> uses builtin_type_void_data_ptr.  That is probably broken on
> whatever target Michael Snyder was trying to fix in the patch
> you referenced, where the sizes differ.

I'd say targets like that should be able to define a proper conversion to
builtin_type_void_data_ptr via convert_register or value_from_register
for that specific register.

Alternatively, if builtin_type_void_data_ptr is in fact the wrong
type to describe a CFA on some weird platform, we could allow the
gdbarch to define the type to be used for this.
 
> If we use address_from_register, we will end up in a call to
> unpack_long using the provided type.  So I think that is exactly the
> same as what we have now.

Well, but the provided type is a pointer type, not an integer type,
so we'll do the proper conversion via pointer_to_address, and not
rely on the signednesss of the (integral) type.


I think address_from_register (builtin_type_void_data_ptr, ...)
*should* really do the right thing conceptually.  Of course, the
problem with that function is that is requires the ("this") frame
where the register resides, but in the dwarf2-frame.c routine we're
still unwinding, so we only have the "next" frame available.  Maybe
your "lazy" frame unwinding approach would solve this.


> This is the trouble with using a host integer type to represent target
> addresses.  If we did all our arithmetic on opaque CORE_ADDR's, this
> wouldn't happen.  I wonder if there's no getting around the need to
> define a sensible calculus for them...

Hmmm, an opaque CORE_ADDR might be nice, e.g. to at some point
implement multiple address spaces.  On the other hand, having
target-specific semantics of CORE_ADDR would also complicate
a multi-target build of GDB ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com