[commit/multiarch/hpux] set addr_bits_remove gdbarch method

[commit/multiarch/hpux] set addr_bits_remove gdbarch method

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

HP/UX text addresses are word aligned, and we were already setting
the smash_text_address method for that. So I changed the gdbarch
init routine to set addr_bits_remove as well.

What caused me to look into this was the fact that I discovered that
we define the ADDR_BITS_REMOVE for hppa64, but not for hppa32, and
I think it is an oversight that was covered up by the fact that we
have SMASH_TEXT_ADDRESS was already.

I think we should have a look at getting rid of either one of these 2
methods. There is already a comment to that effect in gdbarch.h...

2003-08-13  J. Brobecker  <brobecker@gnat.com>

        * hppa-tdep.c (hppa_gdbarch_init): Set the addr_bits_remove
        gdbarch method to clear the 2 low bits of text addresses.

Tested on hppa-hpux11.00. Committed in head.

-- 
Joel

[-- Attachment #2: addr_bits_remove.diff --]
[-- Type: text/plain, Size: 810 bytes --]

Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.86
diff -u -p -r1.86 hppa-tdep.c
--- hppa-tdep.c	3 Jul 2003 22:14:42 -0000	1.86
+++ hppa-tdep.c	13 Aug 2003 17:05:46 -0000
@@ -5046,6 +5046,7 @@ hppa_gdbarch_init (struct gdbarch_info i
   set_gdbarch_deprecated_call_dummy_length (gdbarch, INSTRUCTION_SIZE * 28);
   /* set_gdbarch_deprecated_fix_call_dummy (gdbarch, hppa_fix_call_dummy); */
   set_gdbarch_deprecated_push_arguments (gdbarch, hppa_push_arguments);
+  set_gdbarch_addr_bits_remove (gdbarch, hppa_smash_text_address);
   set_gdbarch_smash_text_address (gdbarch, hppa_smash_text_address);
   set_gdbarch_believe_pcc_promotion (gdbarch, 1);
   set_gdbarch_read_pc (gdbarch, hppa_target_read_pc);

Re: [commit/multiarch/hpux] set addr_bits_remove gdbarch method

[Andrew Cagney]

> HP/UX text addresses are word aligned, and we were already setting
> the smash_text_address method for that. So I changed the gdbarch
> init routine to set addr_bits_remove as well.
> 
> What caused me to look into this was the fact that I discovered that
> we define the ADDR_BITS_REMOVE for hppa64, but not for hppa32, and
> I think it is an oversight that was covered up by the fact that we
> have SMASH_TEXT_ADDRESS was already.
> 
> I think we should have a look at getting rid of either one of these 2
> methods. There is already a comment to that effect in gdbarch.h...

Yep: Consider merging SMASH_TEXT_ADDRESS with ADDR_BITS_REMOVE
http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=333

Any one got a preference over which one goes?  Otherwize, Joel, with 
HP/UX cleaned up, it should either way be pretty obvious.

nice,
Andrew

Re: [commit/multiarch/hpux] set addr_bits_remove gdbarch method

[Joel Brobecker]

[Coming back to an old thread. Was it that long ago already? Sigh...]

On Wed, Aug 13, 2003 at 02:39:55PM -0400, Andrew Cagney wrote:
> Yep: Consider merging SMASH_TEXT_ADDRESS with ADDR_BITS_REMOVE
> http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=333
> 
> Any one got a preference over which one goes?  Otherwize, Joel, with 
> HP/UX cleaned up, it should either way be pretty obvious.

I looked a bit at the code, and I have a small preference of
ADDR_BITS_REMOVE over SMASH_TEXT_ADDRESS. In addition, the only
2 targets that set smash_text_address also define addr_bits_remove
(hppa-tdep.c and arm-tdep.c).  So it looked like there would be an easy
transition:

  - s/SMASH_TEXT_ADDRESS/ADDR_BITS_REMOVE/g
    This should be a noop if we assume:
        SMASH_TEXT_ADDRESS <=> ADDR_BITS_REMOVE

  - Remove the two calls to gdb_arch_set_smash_text_address
  - Delete the SMASH_TEXT_ADDRESS gdbarch method

Unfortunately, It doesn't look like these two methods are that
identical, at least in the case of arm:

    static CORE_ADDR
    arm_addr_bits_remove (CORE_ADDR val)
    {
      if (arm_apcs_32)
        return (val & (arm_pc_is_thumb (val) ? 0xfffffffe : 0xfffffffc));
      else
        return (val & 0x03fffffc);
    }
    
    /* When reading symbols, we need to zap the low bit of the address,
       which may be set to 1 for Thumb functions.  */
    static CORE_ADDR
    arm_smash_text_address (CORE_ADDR val)
    {
      return val & ~1;
    }

SMASH_TEXT_ADDRESS is used in coffread, dbxread, dwarfread, elfread,
and somread.

ADDR_BITS_REMOVE is used in arm*.c, buildsym, infrun, monitor, printcmd,
regcache, remote-mips, values.c (I have deliberately excluded mips-tdep,
remote-mips, s390-tdep, and sh64-tdep since these can not be covered
while the arm gdbarch is in effect).

Looks like the merge is not going to be that obvious... We need to see
if we can merge arm_addr_bits_remove and arm_smash_text_address first...

-- 
Joel

Re: [commit/multiarch/hpux] set addr_bits_remove gdbarch method

[Daniel Jacobowitz]

On Thu, Dec 04, 2003 at 06:13:21PM -0800, Joel Brobecker wrote:
> [Coming back to an old thread. Was it that long ago already? Sigh...]
> 
> On Wed, Aug 13, 2003 at 02:39:55PM -0400, Andrew Cagney wrote:
> > Yep: Consider merging SMASH_TEXT_ADDRESS with ADDR_BITS_REMOVE
> > http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=333
> > 
> > Any one got a preference over which one goes?  Otherwize, Joel, with 
> > HP/UX cleaned up, it should either way be pretty obvious.
> 
> I looked a bit at the code, and I have a small preference of
> ADDR_BITS_REMOVE over SMASH_TEXT_ADDRESS. In addition, the only
> 2 targets that set smash_text_address also define addr_bits_remove
> (hppa-tdep.c and arm-tdep.c).  So it looked like there would be an easy
> transition:
> 
>   - s/SMASH_TEXT_ADDRESS/ADDR_BITS_REMOVE/g
>     This should be a noop if we assume:
>         SMASH_TEXT_ADDRESS <=> ADDR_BITS_REMOVE
> 
>   - Remove the two calls to gdb_arch_set_smash_text_address
>   - Delete the SMASH_TEXT_ADDRESS gdbarch method
> 
> Unfortunately, It doesn't look like these two methods are that
> identical, at least in the case of arm:
> 
>     static CORE_ADDR
>     arm_addr_bits_remove (CORE_ADDR val)
>     {
>       if (arm_apcs_32)
>         return (val & (arm_pc_is_thumb (val) ? 0xfffffffe : 0xfffffffc));
>       else
>         return (val & 0x03fffffc);
>     }
>     
>     /* When reading symbols, we need to zap the low bit of the address,
>        which may be set to 1 for Thumb functions.  */
>     static CORE_ADDR
>     arm_smash_text_address (CORE_ADDR val)
>     {
>       return val & ~1;
>     }
> 
> SMASH_TEXT_ADDRESS is used in coffread, dbxread, dwarfread, elfread,
> and somread.
> 
> ADDR_BITS_REMOVE is used in arm*.c, buildsym, infrun, monitor, printcmd,
> regcache, remote-mips, values.c (I have deliberately excluded mips-tdep,
> remote-mips, s390-tdep, and sh64-tdep since these can not be covered
> while the arm gdbarch is in effect).
> 
> Looks like the merge is not going to be that obvious... We need to see
> if we can merge arm_addr_bits_remove and arm_smash_text_address first...

I'd want to hear from an ARM maintainer, but if smash_text_address is
really only used for text symbols (functions), then those two are
equivalent.  !arm_apcs_32 implies APCS-26, which implies that the upper
six bits don't have a useful value in symbol addresses anyway.  And the
second bit is not useful in ARM mode.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer