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

[Daniel Jacobowitz <drow at mvista dot com>]

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