From patchwork Tue Jan 12 01:00:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Frysinger X-Patchwork-Id: 10346 Received: (qmail 129538 invoked by alias); 12 Jan 2016 01:00:31 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 129501 invoked by uid 89); 12 Jan 2016 01:00:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.0 required=5.0 tests=BAYES_20, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=Entry, 001, sk:elf_int, sk:Elf_Int X-HELO: smtp.gentoo.org Received: from smtp.gentoo.org (HELO smtp.gentoo.org) (140.211.166.183) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 12 Jan 2016 01:00:27 +0000 Received: from vapier.lan (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with SMTP id 6BFA0340A4A; Tue, 12 Jan 2016 01:00:25 +0000 (UTC) Date: Mon, 11 Jan 2016 20:00:25 -0500 From: Mike Frysinger To: Steve Ellcey Cc: gdb-patches@sourceware.org Subject: Re: MIPS simulator is broken Message-ID: <20160112010025.GE4894@vapier.lan> Mail-Followup-To: Steve Ellcey , gdb-patches@sourceware.org References: <5f31ca78-325c-4c18-9abf-16de50bac964@BAMAIL02.ba.imgtec.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5f31ca78-325c-4c18-9abf-16de50bac964@BAMAIL02.ba.imgtec.org> X-IsSubscribed: yes 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: 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 --- 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); } }