dwarf2-frame.c question for maintainers

dwarf2-frame.c question for maintainers

[Martin M. Hunt]

I'm working on some dwarf2 fixes with kevinb.  In
dwarf2_build_frame_info(), Kevin added 
+unit.signed_addr_p = bfd_get_sign_extend_vma (unit.abfd);

This is useful for fixing several bugs where addresses needed to be
sign-extended.  However, I found that read_reg() also needs to
sign-extend its result.  Passing a pointer to the CU all the way down to
read_reg doesn't seem practical.

I don't have any understanding of the overall structure of this code or
where it is going. I can see several possibilities, including the
obvious one; using a global. So how do I solve this to get the patch
accepted?

Martin

Re: dwarf2-frame.c question for maintainers

[Andrew Cagney]

> I'm working on some dwarf2 fixes with kevinb.  In
> dwarf2_build_frame_info(), Kevin added 
> +unit.signed_addr_p = bfd_get_sign_extend_vma (unit.abfd);
> 
> This is useful for fixing several bugs where addresses needed to be
> sign-extended.  However, I found that read_reg() also needs to
> sign-extend its result.  Passing a pointer to the CU all the way down to
> read_reg doesn't seem practical.
> 
> I don't have any understanding of the overall structure of this code or
> where it is going. I can see several possibilities, including the
> obvious one; using a global. So how do I solve this to get the patch
> accepted?

(The global is out :-)

How come extract_typed_address, in read_reg, doesn't sign extend?

The MIPS (I'm assuming this is for the MIPS) has the dogma that _all_ 
addresses (a.k.a. CORE_ADDRs) are _always_ sign extended.  If you see a 
non sign-extended address in the wild, try tracing it back to where it 
was created.

Andrew

Re: dwarf2-frame.c question for maintainers

[Martin M. Hunt]

On Wed, 2004-07-14 at 10:26, Andrew Cagney wrote:
> > I'm working on some dwarf2 fixes with kevinb.  In
> > dwarf2_build_frame_info(), Kevin added 
> > +unit.signed_addr_p = bfd_get_sign_extend_vma (unit.abfd);
> > 
> > This is useful for fixing several bugs where addresses needed to be
> > sign-extended.  However, I found that read_reg() also needs to
> > sign-extend its result.  Passing a pointer to the CU all the way down to
> > read_reg doesn't seem practical.
> > 
> > I don't have any understanding of the overall structure of this code or
> > where it is going. I can see several possibilities, including the
> > obvious one; using a global. So how do I solve this to get the patch
> > accepted?
> 
> (The global is out :-)
> 
> How come extract_typed_address, in read_reg, doesn't sign extend?

I should have explained that. It does.  However extract_typed_address is
incorrect because it makes the invalid assumption that sizeof(address)
== sizeof(register).  So that has to go and be replaced with something
like
extract_signed_integer (buf, register_size (current_gdbarch, regnum));

> The MIPS (I'm assuming this is for the MIPS) has the dogma that _all_ 
> addresses (a.k.a. CORE_ADDRs) are _always_ sign extended.  If you see a 
> non sign-extended address in the wild, try tracing it back to where it 
> was created.
> 
> Andrew
-- 
Martin M. Hunt <hunt@redhat.com>
Red Hat Inc.

Re: dwarf2-frame.c question for maintainers

[Andrew Cagney]

>>> How come extract_typed_address, in read_reg, doesn't sign extend?
> 
> 
> I should have explained that. It does.  However extract_typed_address is
> incorrect because it makes the invalid assumption that sizeof(address)
> == sizeof(register).  So that has to go and be replaced with something
> like
> extract_signed_integer (buf, register_size (current_gdbarch, regnum));

You mean the builtin_type_void_data_ptr parameter to 
extract_typed_address?  Ah.

I see builtin_type_void_data_ptr dates back to 1.1 (Mark?).  It could 
instead use the register's type?

Andrew

Re: dwarf2-frame.c question for maintainers

[Mark Kettenis]

   Date: Wed, 14 Jul 2004 15:28:49 -0400
   From: Andrew Cagney <cagney@gnu.org>

   >>> How come extract_typed_address, in read_reg, doesn't sign extend?
   > 
   > 
   > I should have explained that. It does.  However extract_typed_address is
   > incorrect because it makes the invalid assumption that sizeof(address)
   > == sizeof(register).  So that has to go and be replaced with something
   > like
   > extract_signed_integer (buf, register_size (current_gdbarch, regnum));

Martin, not all the world's a MIPS.  Sign-extension is wrong here for
most targets.

   You mean the builtin_type_void_data_ptr parameter to 
   extract_typed_address?  Ah.

   I see builtin_type_void_data_ptr dates back to 1.1 (Mark?).  It could 
   instead use the register's type?

Nope.  extract_typed_address only accepts a pointer type, and there's
no guarantee that the register's type is a pointer type.

Mark

Re: dwarf2-frame.c question for maintainers

[Martin M. Hunt]

On Wed, 2004-07-14 at 12:28, Andrew Cagney wrote:
> >>> How come extract_typed_address, in read_reg, doesn't sign extend?
> > 
> > 
> > I should have explained that. It does.  However extract_typed_address is
> > incorrect because it makes the invalid assumption that sizeof(address)
> > == sizeof(register).  So that has to go and be replaced with something
> > like
> > extract_signed_integer (buf, register_size (current_gdbarch, regnum));
> 
> You mean the builtin_type_void_data_ptr parameter to 
> extract_typed_address?  Ah.
> 
> I see builtin_type_void_data_ptr dates back to 1.1 (Mark?).  It could 
> instead use the register's type?

extract_typed_address calls extract_[un]signed_integer with size =
TYPE_LENGTH of builtin_type_void_data_ptr.

Here's exactly what I am seeing.  Maybe you can tell me if read_reg is
the problem.

For example big-endian Mips, with o64 or (eabi and mlong32):
(registers are 64 bits and pointers are 32 bits)

read_reg calls frame_unwind_register (next_frame, regnum, buf)
after that, buf has something like ffffffff801fffb8

Now if you do extract_typed_address(), it knows addresses are 4 bytes
and returns 0xffffffff sign extended to 0xfffffffffffffff

If instead, you call extract_[un]signed_integer((buf, register_size
(current_gdbarch, regnum)), it returns 0xffffffff801fffb8

The real problem here is the the size.  AFAICT, sign-extension here is
unimportant; I get the same test results calling 
extract_unsigned_integer in read_reg() for mips, because, as you can
see, nothing needs extending, just the whole register needs read.
However, I can't prove that is always the case because I am not familiar
enough with the code.

-- 
Martin M. Hunt <hunt@redhat.com>
Red Hat Inc.

Re: dwarf2-frame.c question for maintainers

[Andrew Cagney]

> On Wed, 2004-07-14 at 12:28, Andrew Cagney wrote:
> 
>>>>>> >>> How come extract_typed_address, in read_reg, doesn't sign extend?
>>>
>>>> > 
>>>> > 
>>>> > I should have explained that. It does.  However extract_typed_address is
>>>> > incorrect because it makes the invalid assumption that sizeof(address)
>>>> > == sizeof(register).  So that has to go and be replaced with something
>>>> > like
>>>> > extract_signed_integer (buf, register_size (current_gdbarch, regnum));
>>
>>> 
>>> You mean the builtin_type_void_data_ptr parameter to 
>>> extract_typed_address?  Ah.
>>> 
>>> I see builtin_type_void_data_ptr dates back to 1.1 (Mark?).  It could 
>>> instead use the register's type?
> 
> 
> extract_typed_address calls extract_[un]signed_integer with size =
> TYPE_LENGTH of builtin_type_void_data_ptr.
> 
> Here's exactly what I am seeing.  Maybe you can tell me if read_reg is
> the problem.
> 
> For example big-endian Mips, with o64 or (eabi and mlong32):
> (registers are 64 bits and pointers are 32 bits)
> 
> read_reg calls frame_unwind_register (next_frame, regnum, buf)
> after that, buf has something like ffffffff801fffb8
> 
> Now if you do extract_typed_address(), it knows addresses are 4 bytes
> and returns 0xffffffff sign extended to 0xfffffffffffffff

Right, as it stands, that call is just wrong.

> If instead, you call extract_[un]signed_integer((buf, register_size
> (current_gdbarch, regnum)), it returns 0xffffffff801fffb8
> 
> The real problem here is the the size.  AFAICT, sign-extension here is
> unimportant; I get the same test results calling 
> extract_unsigned_integer in read_reg() for mips, because, as you can
> see, nothing needs extending, just the whole register needs read.
> However, I can't prove that is always the case because I am not familiar
> enough with the code.

Consider o32.  Both the ABI and ISA are 32-bits, but GDB's CORE_ADDR may 
be 64-bits.  Even if it doesn't appear to make a difference, the MIPS 
needs to always sign extend addresses/registers - that's the dogma :-)

Andrew

Re: dwarf2-frame.c question for maintainers

[Martin M. Hunt]

On Thu, 2004-07-15 at 10:58, Andrew Cagney wrote:
> > On Wed, 2004-07-14 at 12:28, Andrew Cagney wrote:
> > 
> >>>>>> >>> How come extract_typed_address, in read_reg, doesn't sign extend?
> >>>
> >>>> > 
> >>>> > 
> >>>> > I should have explained that. It does.  However extract_typed_address is
> >>>> > incorrect because it makes the invalid assumption that sizeof(address)
> >>>> > == sizeof(register).  So that has to go and be replaced with something
> >>>> > like
> >>>> > extract_signed_integer (buf, register_size (current_gdbarch, regnum));
> >>
> >>> 
> >>> You mean the builtin_type_void_data_ptr parameter to 
> >>> extract_typed_address?  Ah.
> >>> 
> >>> I see builtin_type_void_data_ptr dates back to 1.1 (Mark?).  It could 
> >>> instead use the register's type?
> > 
> > 
> > extract_typed_address calls extract_[un]signed_integer with size =
> > TYPE_LENGTH of builtin_type_void_data_ptr.
> > 
> > Here's exactly what I am seeing.  Maybe you can tell me if read_reg is
> > the problem.
> > 
> > For example big-endian Mips, with o64 or (eabi and mlong32):
> > (registers are 64 bits and pointers are 32 bits)
> > 
> > read_reg calls frame_unwind_register (next_frame, regnum, buf)
> > after that, buf has something like ffffffff801fffb8
> > 
> > Now if you do extract_typed_address(), it knows addresses are 4 bytes
> > and returns 0xffffffff sign extended to 0xfffffffffffffff
> 
> Right, as it stands, that call is just wrong.
> 
> > If instead, you call extract_[un]signed_integer((buf, register_size
> > (current_gdbarch, regnum)), it returns 0xffffffff801fffb8
> > 
> > The real problem here is the the size.  AFAICT, sign-extension here is
> > unimportant; I get the same test results calling 
> > extract_unsigned_integer in read_reg() for mips, because, as you can
> > see, nothing needs extending, just the whole register needs read.
> > However, I can't prove that is always the case because I am not familiar
> > enough with the code.
> 
> Consider o32.  Both the ABI and ISA are 32-bits, but GDB's CORE_ADDR may 
> be 64-bits.  Even if it doesn't appear to make a difference, the MIPS 
> needs to always sign extend addresses/registers - that's the dogma :-)

Right.  And so back to the original question.  What is the best way to
have read_reg detect if it should sign-extend?  We agreed that passing a
pointer to the CU was out, as was using a global.  Do I need to add
something to gdbarch?  

-- 
Martin M. Hunt <hunt@redhat.com>
Red Hat Inc.

Re: dwarf2-frame.c question for maintainers

[Andrew Cagney]

>>> Consider o32.  Both the ABI and ISA are 32-bits, but GDB's CORE_ADDR may 
>>> be 64-bits.  Even if it doesn't appear to make a difference, the MIPS 
>>> needs to always sign extend addresses/registers - that's the dogma :-)
> 
> 
> Right.  And so back to the original question.  What is the best way to
> have read_reg detect if it should sign-extend?  We agreed that passing a
> pointer to the CU was out, as was using a global.  Do I need to add
> something to gdbarch?  

Hmm, the architecture vector already has POINTER_TO_ADDRESS and 
register_type that can be used to extract [signed] pointers and 
registers.  Can either of those be used here?

Andrew

Re: dwarf2-frame.c question for maintainers

[Martin M. Hunt]

On Thu, 2004-07-15 at 11:35, Andrew Cagney wrote:
> >>> Consider o32.  Both the ABI and ISA are 32-bits, but GDB's CORE_ADDR may 
> >>> be 64-bits.  Even if it doesn't appear to make a difference, the MIPS 
> >>> needs to always sign extend addresses/registers - that's the dogma :-)
> > 
> > 
> > Right.  And so back to the original question.  What is the best way to
> > have read_reg detect if it should sign-extend?  We agreed that passing a
> > pointer to the CU was out, as was using a global.  Do I need to add
> > something to gdbarch?  
> 
> Hmm, the architecture vector already has POINTER_TO_ADDRESS and 
> register_type that can be used to extract [signed] pointers and 
> registers.  Can either of those be used here?

You mean something like

if (current_gdbarch->pointer_to_address == unsigned_pointer_to_address)
  return extract_unsigned_integer (buf, register_size (current_gdbarch, regnum));
else
  return extract_signed_integer (buf, register_size (current_gdbarch, regnum));

-- 
Martin M. Hunt <hunt@redhat.com>
Red Hat Inc.

Re: dwarf2-frame.c question for maintainers

[Jim Blandy]

Here is a terse way to extract the least-significant end of the
register's value, regardless of endianness.

*** dwarf2-frame.c.~1.36.~	2004-06-17 16:42:41.000000000 -0500
--- dwarf2-frame.c	2004-07-16 14:57:09.000000000 -0500
*************** read_reg (void *baton, int reg)
*** 214,219 ****
--- 214,222 ----
  
    buf = (char *) alloca (register_size (gdbarch, regnum));
    frame_unwind_register (next_frame, regnum, buf);
+   store_unsigned_integer
+     (buf, TYPE_LENGTH (builtin_type_void_data_ptr),
+      extract_unsigned_integer (buf, register_size (gdbarch, regnum)));
    return extract_typed_address (buf, builtin_type_void_data_ptr);
  }

Shouldn't that work?  (Setting aside questions of whether it's
pleasing to extract, store, and then re-extract the value.)