Re: [BUG] print_address_numeric

[David Taylor <taylor at cygnus dot com>]

    Date: Tue, 01 Aug 2000 12:28:42 +1000
    From: Andrew Cagney <ac131313@cygnus.com>

    David Taylor wrote:
    > 
    > In printcmd.c (print_address_numeric), we find the lines:
    > 
    >       int ptr_bit = TARGET_PTR_BIT;
    >       if (ptr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
    >         addr &= ((CORE_ADDR) 1 << ptr_bit) - 1;
    >       print_longest (stream, 'x', use_local, (ULONGEST) addr);

    You forgot the lines:

      /* Truncate address to the size of a target pointer, avoiding shifts
	 larger or equal than the width of a CORE_ADDR.  The local
	 variable PTR_BIT stops the compiler reporting a shift overflow
	 when it won't occure. */
      /* NOTE: This assumes that the significant address information is
	 kept in the least significant bits of ADDR - the upper bits were
	 either zero or sign extended.  Should ADDRESS_TO_POINTER() or
	 some ADDRESS_TO_PRINTABLE() be used to do the conversion?  */

    :-)

Well, those lines didn't change.

With regard to the question:

    "Should ADDRESS_TO_POINTER() or some ADDRESS_TO_PRINTABLE() be used
    to do the conversion?"

ADDRESS_TO_POINTER -- no; ADDRESS_TO_PRINTABLE -- sure, but I don't need it.

    > This code has a bug, namely it assumes that addresses and pointers are
    > the same size.
    > 
    > [address == gdb representation; pointer == target representation]

	    CORE_ADDR - GDB's internal representation of an address
		    it is large enough to fit target address information.

	    pointer - targets pointer representation

	    ??? - minimum number of bits needed to represent an address on the
    target.

Currently, TARGET_PTR_BIT says how many bits pointers take in the
target representation; there is no macro / gdbarch field to say how
many bits addresses take within CORE_ADDR.

My proposed TARGET_ADDR_BIT / addr_bit is to add just such a macro /
field -- with the default value being the value of TARGET_PTR_BIT /
ptr_bit, so that targets that do nothing will work as they do now.

    When printing a symbolic address, should it be displayed using GDB's
    internal format (for MIPS definitly not) or target pointer format.

Currently, it does neither properly.  It takes the GDB representation,
and prints the number of bits that are significant on the target.

For a Harvard architecture chip, it's nice to be able to tell from the
value which address space it lives in.

For example, for the chip I'm working on, instruction memory (on the
target) is word addressed, not byte addressed.

On the target, the first instruction is at address 0, the next at
address 1, the following at address 2, and so on.

In gdb, the first instruction is at address 0x2000000, the next at
0x2000002, the following at 0x2000004, and so on.

If you set a breakpoint at 0x2000004, it will tell you it set it at
address 0x4.

I feel that it should either say

    0x2000004

('cause that is what address the user has to give to set a breakpoint
there) or

    0x2

('cause that is what address the target considers it to be).

And of the two, I feel that it should report an address that the user
would type in, namely the gdb representation, which would be 0x2000004
in this case.

Additionally, it's name suggests that it is printing an address, not a
pointer (print_address_numeric).  And, at least in my case, it is
being given an address (b->address), not a pointer.

    > For some processors they are different.  For the d10v and at least one
    > other processor, the size of a pointer is 2 bytes, but the size of an
    > address is 4 bytes.
    > 
    > I propose the creation of a new macro TARGET_ADDR_BIT, the addition of
    > the lines:
    > 
    >     #if !defined(TARGET_ADDR_BIT)
    >     #define TARGET_ADDR_BIT     TARGET_PTR_BIT
    >     #endif
    > 
    > to defs.h, and that the above lines of print_address_numeric be changed to:
    > 
    >     int addr_bit = TARGET_ADDR_BIT
    >     if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT))
    >       addr & ((CORE_ADDR) 1 << ptr_bit) - 1;
    >     print_longest (stream, 'x', use_local, (ULONGEST) addr);
    > 
    > Comments?

    I think it should be a method (function) rather than a macro.  The
    default function using TARGET_PTR_BIT.  It would give your target
    complete freedom over how the numeric address is printed.

	    Andrew

I have no objection to having a method (function) *in addition* to the
macro TARGET_ADDR_BIT.  But, I feel that the macro should be created
and that print_address_numeric should use it.