[pushed] Fix Cell/B.E. regression (Re: [PATCH 1/3] Clear non-significant bits of address on memory access)

Message ID 20171220130318.D649DD80341@oc3748833570.ibm.com
State New, archived
Headers

Commit Message

Ulrich Weigand Dec. 20, 2017, 1:03 p.m. UTC
  Yao Qi wrote:

> Nowadays, we strip non-significant bits in address, pass the stripped
> address to target cache and to_xfer_partial.  It works for aarch64, but
> breaks ppc32/spu.  However, in ppc32/spu case, ppu address and spu
> address is mixed together, differentiated by the top bit, so the number
> of significant bits of address is 64, because if we can't remove any of
> them.  IMO, it is reasonable to set significant_addr_bits to 64 in ppc.
> 
> I considered your suggestion that pushing address_significant call down,
> below spu-multiarch target, that means, many target's to_xfer_partial
> need to call address_significant, so I don't do that.  Secondly, in the
> way you suggested, we still pass the original address to target cache,
> which works for ppu/spu, but it doesn't work for aarch64.

I've now pushed the patch below, which fixes the regression for now.

Longer term, I think the correct fix would probably be to make address
spaces explit, e.g. by passing an address space identifer to xfer_partial.
The gdbarch associated with that address space should then determine
whether truncation is required ...

Bye,
Ulrich


gdb/ChangeLog:

	* spu-tdep.c (spu_gdbarch_init): Set set_gdbarch_significant_addr_bit
	to 64 bits.
	(ppc_linux_init_abi): Likewise, if Cell/B.E. is supported.
  

Comments

Yao Qi Dec. 20, 2017, 1:59 p.m. UTC | #1
On Wed, Dec 20, 2017 at 1:03 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> I've now pushed the patch below, which fixes the regression for now.
>

Thank you.

> Longer term, I think the correct fix would probably be to make address
> spaces explit, e.g. by passing an address space identifer to xfer_partial.
> The gdbarch associated with that address space should then determine
> whether truncation is required ...
>

Agreed.
  

Patch

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 0e43a64..5120490 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1809,6 +1809,10 @@  ppc_linux_init_abi (struct gdbarch_info info,
 
       /* Cell/B.E. cross-architecture unwinder support.  */
       frame_unwind_prepend_unwinder (gdbarch, &ppu2spu_unwind);
+
+      /* We need to support more than "addr_bit" significant address bits
+         in order to support SPUADDR_ADDR encoded values.  */
+      set_gdbarch_significant_addr_bit (gdbarch, 64);
     }
 
   set_gdbarch_displaced_step_location (gdbarch,
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index fb9a5d8..dda3011 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -2720,6 +2720,9 @@  spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_address_class_name_to_type_flags
     (gdbarch, spu_address_class_name_to_type_flags);
 
+  /* We need to support more than "addr_bit" significant address bits
+     in order to support SPUADDR_ADDR encoded values.  */
+  set_gdbarch_significant_addr_bit (gdbarch, 64);
 
   /* Inferior function calls.  */
   set_gdbarch_call_dummy_location (gdbarch, ON_STACK);