Re: MIPS simulator is broken

[Mike Frysinger <vapier at gentoo dot org>]

On 08 Jan 2016 14:30, Steve Ellcey  wrote:
> Your December 26th change to the mips simulator has broken it, at least
> for me.  I am trying to compile and run a program with mips32r2 big-endian
> and am using the mti32.ld linker script.
> 
> After your commit my attempts to run a 'hello world' program die with:
> 
> 
> mips-core: 4 byte read to unmapped address 0xffffffff80020004 at 0xffffffff80020004
> program stopped with signal 10 (User defined signal 1).

the mips address logic was fundamentally broken.  my changes merely
uncovered that braindeadness rather than caused it.  if you look at
that commit and the AddressTranslation definition, you'll see:
  /* For a simple (flat) memory model, we simply pass virtual
     addressess through (mostly) unchanged. */
  vAddr &= 0xFFFFFFFF;

this is forcing all addresses to be 32-bit.  when i saw this, i assumed
it was just a dummy function someone didn't get around to finishing.  i
didn't notice that the sim explicitly sets the address space to be 64-bit
in configure.ac for many targets:
mips_addr_bitsize=
case "${target}" in
  mips*-sde-elf*)       mips_bitsize=64 ; mips_msb=63 ;;
...
  mips*-*-*)            mips_bitsize=32 ; mips_msb=31 ;;
...
SIM_AC_OPTION_BITSIZE($mips_bitsize,$mips_msb,$mips_addr_bitsize)

it passes for me because mips-elf is a 32-bit target where that mask is
actually a nop.  but for targets w/64-bit address space, it's getting a
bad 32-bit sign extended address.  the sim is behaving correctly here:
it should be throwing an error because 0xffffffff80010000 is not mapped.

digging further, it looks like the bug is in bfd, not the sim.  the pc
is set to 0xffffffff80010000 during init:
sim-main.c:sim_create_inferior:
  CPU_PC_SET (cpu, (unsigned64) bfd_get_start_address (abfd));

when we look at the bfd init, bfd_set_start_address is called with that
value as well:
elfcode.h:elf_object_p:
  bfd_set_start_address (abfd, i_ehdrp->e_entry);
(gdb) p *i_ehdrp
$2 = {
  e_ident = "\177ELF\001\002\001\000\000\000\000\000\000\000\000",
  e_entry = 0xffffffff80010000,

e_ehdrp is supposed to be representing an Elf32_Ehdr (by way of the
bfd Elf_Internal_Ehdr structure), and e_entry is supposed to be an
Elf32_Addr, and that's supposed to be uint32_t.

looking at include/elf/internal.h, Elf_Internal_Ehdr is:
  bfd_vma e_entry;    /* Entry point virtual address */
and bfd_vma is set to:
  bfd-in.h:typedef bfd_vma bfd_uint64_t;

so that all looks correct.  poking elf_object_p further, the x_ehdr
(Elf_External_Ehdr) is read in correctly too.

the badness starts here:
  elf_swap_ehdr_in (abfd, &x_ehdr, i_ehdrp);
which does:
  int signed_vma = get_elf_backend_data (abfd)->sign_extend_vma;
  if (signed_vma)
    dst->e_entry = H_GET_SIGNED_WORD (abfd, src->e_entry);
  else
    dst->e_entry = H_GET_WORD (abfd, src->e_entry);

and indeed, the mips bfds (for whatever reason) do:
elf32-mips.c:#define elf_backend_sign_extend_vma    TRUE
elf64-mips.c:#define elf_backend_sign_extend_vma    TRUE
elfn32-mips.c:#define elf_backend_sign_extend_vma   TRUE

i don't know why mips wants to do this, and considering literally no
other target does it (ok, sh64 does it, but let's ignore them since
that target is dead & being removed), it's probably not an accident.
so we have to hack around it in the sim:

--- a/sim/mips/interp.c
+++ b/sim/mips/interp.c
@@ -55,6 +55,7 @@ code on the hardware.
 #include "getopt.h"
 #include "libiberty.h"
 #include "bfd.h"
+#include "elf-bfd.h"
 #include "gdb/callback.h"   /* GDB simulator callback interface */
 #include "gdb/remote-sim.h" /* GDB simulator interface */
 
@@ -1020,7 +1021,17 @@ sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
       for (cpu_nr = 0; cpu_nr < sim_engine_nr_cpus (sd); cpu_nr++)
 	{
 	  sim_cpu *cpu = STATE_CPU (sd, cpu_nr);
-	  CPU_PC_SET (cpu, (unsigned64) bfd_get_start_address (abfd));
+	  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);
 	}
     }
 

that allows most tests to pass, but 6 (out of 69) are still failing.
fpu64-ps.s for example has:
mips-core: 8 byte read to unmapped address 0xffffffff80010b18 at 0x80010070

poking this a bit, i'm not sure if the code is bad (it's actually
doing a load from that address), or the mips decode logic has a bad
sign extension in there.  i'll let someone else figure it out.
-mike

Attachment: signature.asc
Description: Digital signature