Re: [PATCH 1/2] [pr gdb/19447] sim: mips: Only truncate sign extension bits for 32-bit target models

[Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>]

On 05 May 2021 04:51, Faraz Shahbazker wrote:
> 64-bit BFD for MIPS applies a standard sign extension on all addresses
> assuming 64-bit target. These bits are required for 64-bit and can only be
> safely truncated for 32-bit target models. This partially reverts commit
> b36d953bced0a4fecdde1823abac70ed7038ee95

can you verify there are no regressions in `make check` for the various
mips targets & build configs ?

if the tests pass, then i'm fine with the patch.

> ---
> 
> Notes:

these notes should be part of the commit message for posterity

>  	  sim_cpu *cpu = STATE_CPU (sd, cpu_nr);
> -	  sim_cia pc = bfd_get_start_address (abfd);
> -
> -	  /* We need to undo brain-dead bfd behavior where it sign-extends
> -	     addresses that are supposed to be unsigned.  See the mips bfd
> -	     sign_extend_vma setting.  We have to check the ELF data itself
> -	     in order to handle o32 & n32 ABIs.  */
> -	  if (abfd->tdata.elf_obj_data->elf_header->e_ident[EI_CLASS] ==
> -	      ELFCLASS32)
> -	    pc = (unsigned32) pc;
> -
> -	  CPU_PC_SET (cpu, pc);
> +	  /* The 64-bit BFD sign-extends MIPS addresses to model
> +	     32-bit compatibility segments with 64-bit addressing.
> +	     These addresses work as is on 64-bit targets but
> +	     can be truncated for 32-bit targets.  */
> +	  if (WITH_TARGET_WORD_BITSIZE == 32)
> +	    CPU_PC_SET (cpu, (unsigned32) bfd_get_start_address (abfd));
> +	  else
> +	    CPU_PC_SET (cpu, bfd_get_start_address (abfd));

style-wise, it seems like you should have just changed the if() statement
and left the rest of the code alone.  it avoids duplicating the rest of the
logic.
-mike