Re: [Qemu-devel] [PATCH] disas: Fix printing of addresses in disassembly

[Andreas Färber]

Am 25.06.2012 16:55, schrieb Peter Maydell:
> In our disassembly code, the bfd_vma type is always 64 bits,
> even if the target's virtual address width is only 32 bits. This
> means that when we print out addresses we need to truncate them
> to 32 bits, to avoid odd output which has incorrectly sign-extended
> a value to 64 bits, for instance this ARM example:
>     0x80479a60:  e59f4088     ldr  r4, [pc, #136]  ; 0xffffffff80479a4f
> 
> (It would also be possible to truncate before passing the address
> to info->print_address_func(), but truncating in the final print
> function is the same approach that binutils takes to this problem.)
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  disas.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/disas.c b/disas.c
> index 93d8d30..7b2acc9 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -64,6 +64,22 @@ generic_print_address (bfd_vma addr, struct 
> disassemble_info *info)
>      (*info->fprintf_func) (info->stream, "0x%" PRIx64, addr);
>  }
>  
> +/* Print address in hex, truncated to the width of a target virtual address. 
> */
> +static void
> +generic_print_target_address(bfd_vma addr, struct disassemble_info *info)
> +{
> +    uint64_t mask = ~0ULL >> (64 - TARGET_VIRT_ADDR_SPACE_BITS);
> +    generic_print_address(addr & mask, info);
> +}
> +
> +/* Print address in hex, truncated to the width of a host virtual address. */
> +static void
> +generic_print_host_address(bfd_vma addr, struct disassemble_info *info)
> +{
> +    uint64_t mask = ~0ULL >> (64 - (sizeof(void *) * 8));
> +    generic_print_address(addr & mask, info);
> +}
> +
>  /* Just return the given address.  */
>  
>  int

As usual the inversion and subtracted shift are a bit confusing at
first, but the algorithm looks okay.

I wonder if TARGET_VIRT_ADDR_SPACE_BITS is the correct factor to use
here though? Might sizeof(target_phys_addr_t) * 8 be safer? I'm thinking
of the possibility of having an alias in the 64-bit address space point
into the actual 36/48/... virtual address space. I have a ppc64 ld
instruction in mind, for which a full 64-bit register would be set up
that could not fully be represented in the virtual address space.

But maybe I'm misunderstanding what exactly these functions are being
assigned for below...

Andreas

> @@ -154,6 +170,7 @@ void target_disas(FILE *out, target_ulong code, 
> target_ulong size, int flags)
>      disasm_info.read_memory_func = target_read_memory;
>      disasm_info.buffer_vma = code;
>      disasm_info.buffer_length = size;
> +    disasm_info.print_address_func = generic_print_target_address;
>  
>  #ifdef TARGET_WORDS_BIGENDIAN
>      disasm_info.endian = BFD_ENDIAN_BIG;
> @@ -274,6 +291,7 @@ void disas(FILE *out, void *code, unsigned long size)
>      int (*print_insn)(bfd_vma pc, disassemble_info *info);
>  
>      INIT_DISASSEMBLE_INFO(disasm_info, out, fprintf);
> +    disasm_info.print_address_func = generic_print_host_address;
>  
>      disasm_info.buffer = code;
>      disasm_info.buffer_vma = (uintptr_t)code;
> @@ -386,6 +404,7 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>      monitor_disas_env = env;
>      monitor_disas_is_physical = is_physical;
>      disasm_info.read_memory_func = monitor_read_memory;
> +    disasm_info.print_address_func = generic_print_target_address;
>  
>      disasm_info.buffer_vma = pc;
>  

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg