Re: [BUG] print_address_numeric

[David Taylor <taylor at cygnus dot com>]

    Date: Thu, 03 Aug 2000 11:26:36 +1000
    From: Andrew Cagney <ac131313@cygnus.com>

    David Taylor wrote:

    >     The d10v (which has an identical problem) currently does this and it has
    >     proven to be useable.
    > 
    > Does it print the right value?  Or does the user just cope with it?

    The d10v had TARGET_PTR_BIT == 4 bytes so it avoids the problem you
    encountered.  This is why, in various places you see the code:

	    if (D10V and pointer)
	      len = 2;

Right.  And if TARGET_PTR_BIT was 16 bits instead of 32, then the
calculated length should be 2 bytes rather than 4 and it shouldn't
need to be special cased...  at least, that is the theory.

    > I don't have a d10v build sitting around, but I would expect the that
    > on the d10v, if I set a breakpoint at address 0x2000004, it would
    > print it as 4 rather than 0x2000004 or 1.  I was thinking that both
    > the processor I'm working on and the d10v would set TARGET_ADDR_BIT to
    > 32 and TARGET_PTR_BIT to 16.

    It should but it didn't.  It has all those other D10V_* macros instead.

Agreed.

    >     Was there where else you thought you'd need the TARGET_ADDR_BIT macro?
    > 
    > Hmmm, I hadn't specifically checked.  It looks like...
    > 
    > TARGET_PTR_BIT ==> TARGET_ADDR_BIT:
    >     2 places in read_var_value (findvar.c)

    I don't think these need changing.  They are reading target pointers and
    not target h/w addresses?
    Hmm, actually I'm not sure.  Is your definition of TARGET_ADDR_BIT the
    number of significant bits in a CORE_ADDR needed to represent the target
    address space or the actual address size of the target?

The former, i.e., the number of bits in a CORE_ADDR needed to
represent the target address space.

For the latter, we already have TARGET_PTR_BIT.

Hmmm, looking at them again, I think you're right; but I think that
the variable 'addr' is playing a dual role -- it's both an address and
a pointer.

If I understand this correctly

      addr = SYMBOL_VALUE_ADDRESS (var);

Here, addr is an ADDRESS.

      addr = read_memory_unsigned_integer
	(addr, TARGET_PTR_BIT / TARGET_CHAR_BIT);

On the left, it's a POINTER, on the right it's an ADDRESS.

      addr += SYMBOL_VALUE (var);
      addr = read_memory_unsigned_integer
	(addr, TARGET_PTR_BIT / TARGET_CHAR_BIT);

ditto for here.

And here:

  VALUE_ADDRESS (v) = addr;

it's an address.  So, a POINTER_TO_ADDRESS probably needs to be thrown
in there.

There are a number of places where GDB assumes that the gdb
representation (== address) and the target representation (== pointer)
are identical.  And for many processors, it is a valid assumption.

    >     1 place in build_gdbtypes (gdbtypes.c)
    builtin_type_CORE_ADDR? Yes (it isn't actually used though).

    > I suspect that with the addition of the TARGET_ADDR_BIT macro and the
    > proper changes of TARGET_PTR_BIT vs TARGET_ADDR_BIT and extract_* /
    > store_* vs POINTER_TO_ADDRESS / ADDRESS_TO_POINTER that many uses of
    > GDB_TARGET_IS_D10V could be eliminated.

    > Are you unsure how multi-arching TARGET_ADDR_BIT would go?  Or
    > unsure about how multi-arching ADDRESS_TO_PRINTABLE would go?

    TARGET_ADDR_BIT - it needs to (post) initializing it with somthing from
    the architecture vector.  I've never tried it ...

Making the change and regenerating gdbarch.[ch], the function
verify_gdbarch now has the lines:

  if ((GDB_MULTI_ARCH >= 1)
      && (gdbarch->ptr_bit == 0))
    internal_error ("gdbarch: verify_gdbarch: ptr_bit invalid");
  if (gdbarch->addr_bit == 0)
    gdbarch->addr_bit = gdbarch->ptr_bit;

So, it did the right thing.

Unless there are objections, I'm going to be committing the following
patch to gdbarch.sh and regenerate gdbarch.[ch].

Index: gdbarch.sh
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/gdbarch.sh,v
retrieving revision 2.62
diff -c -r2.62 gdbarch.sh
*** gdbarch.sh	2000/08/01 17:38:48	2.62
--- gdbarch.sh	2000/08/03 16:57:13
***************
*** 275,281 ****
--- 275,292 ----
  i:2:TARGET_BYTE_ORDER:int:byte_order::::BIG_ENDIAN
  #
  v:1:TARGET_BFD_VMA_BIT:int:bfd_vma_bit::::8 * sizeof (void*):TARGET_ARCHITECTURE->bits_per_address::0
+ # For most targets, a pointer on the target and its representation as an
+ # address in GDB have the same size and "look the same".  For such a
+ # target, you need only set TARGET_PTR_BIT / ptr_bit and TARGET_ADDR_BIT
+ # / addr_bit will be set from it.
+ #
+ # If TARGET_PTR_BIT and TARGET_ADDR_BIT are different, you'll probably
+ # also need to set POINTER_TO_ADDRESS and ADDRESS_TO_POINTER as well.
+ #
+ # ptr_bit is the size of a pointer on the target
  v:1:TARGET_PTR_BIT:int:ptr_bit::::8 * sizeof (void*):0
+ # addr_bit is the size of a target address as represented in gdb
+ v:1:TARGET_ADDR_BIT:int:addr_bit::::8 * sizeof (void*):0:gdbarch->ptr_bit
  #v:1:TARGET_CHAR_BIT:int:char_bit::::8 * sizeof (char):0
  v:1:TARGET_SHORT_BIT:int:short_bit::::8 * sizeof (short):0
  v:1:TARGET_INT_BIT:int:int_bit::::8 * sizeof (int):0
***************
*** 472,484 ****
  
  /* This file was created with the aid of \`\`gdbarch.sh''.
  
!    The bourn shell script \`\`gdbarch.sh'' creates the files
     \`\`new-gdbarch.c'' and \`\`new-gdbarch.h and then compares them
     against the existing \`\`gdbarch.[hc]''.  Any differences found
     being reported.
  
     If editing this file, please also run gdbarch.sh and merge any
!    changes into that script. Conversely, when makeing sweeping changes
     to this file, modifying gdbarch.sh and using its output may prove
     easier. */
  
--- 483,495 ----
  
  /* This file was created with the aid of \`\`gdbarch.sh''.
  
!    The Bourne shell script \`\`gdbarch.sh'' creates the files
     \`\`new-gdbarch.c'' and \`\`new-gdbarch.h and then compares them
     against the existing \`\`gdbarch.[hc]''.  Any differences found
     being reported.
  
     If editing this file, please also run gdbarch.sh and merge any
!    changes into that script. Conversely, when making sweeping changes
     to this file, modifying gdbarch.sh and using its output may prove
     easier. */