From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7053 invoked by alias); 4 May 2012 22:41:21 -0000 Received: (qmail 7039 invoked by uid 22791); 4 May 2012 22:41:18 -0000 X-SWARE-Spam-Status: No, hits=-4.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 May 2012 22:41:03 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1SQRBN-0003qP-AC from Maciej_Rozycki@mentor.com ; Fri, 04 May 2012 15:41:01 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Fri, 4 May 2012 15:41:01 -0700 Received: from [172.30.1.40] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Fri, 4 May 2012 23:40:58 +0100 Date: Fri, 04 May 2012 22:41:00 -0000 From: "Maciej W. Rozycki" To: Joel Brobecker CC: Subject: Re: [RFA 1/2] mips: Switch inferior function calls to ON_STACK method. In-Reply-To: <20120504205818.GT15555@adacore.com> Message-ID: References: <1336071802-13599-1-git-send-email-brobecker@adacore.com> <1336071802-13599-2-git-send-email-brobecker@adacore.com> <20120503214933.GJ15555@adacore.com> <20120504205818.GT15555@adacore.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-05/txt/msg00140.txt.bz2 On Fri, 4 May 2012, Joel Brobecker wrote: > Now I know why I was told you are a MIPS expert :-). I never really > had the chance or need to delve into the details of any specific > architecture. Even for the ia64-hpux port, I could do with just > a superficial knowledge of that CPU. It just started the other way round for me -- I got pulled into toolchain hacking as a result of my hopeless attempts to bring up the then current version of Linux kernel on my DECstation box (a MIPS R3000 class system). Of course you can't really use a kernel without userland and as it turned out that meant porting upcoming glibc 2.2 unless I wanted to downgrade back to long obsolete 2.0 (which I did not). Of course binutils at that point did not support symbol versioning in the MIPS port yet (it couldn't use the generic ELF versioning stuff all the other ports used because of some peculiarities of the IRIX ELF variation), so I had to fix that first and that's how it started rolling... So I'm really a low-level kernel developer who just happens to do something else. ;) I still have that DECstation and can check any original MIPS I ISA stuff at the run time should there be such a need. > > * newly-added 48-bit instructions. > > I am wondering if this addition is going to hurt in terms of our > support... From what I could tell from my mips64 manual, even > on this CPU the instructions are still 32bit long... But I'm > digressing, sorry. My GDB patch handles it just fine as the major opcode for any 48-bit instructions has been already allocated by the architecture (you can see some parts in my change that classify instructions according to their length for example), except there has been no hardware or simulator available to test it yet. QEMU might be closest to getting there. For the purpose of software breakpoints any 48-bit instruction is I reckon replaced with a 32-bit breakpoint. This is safe because 48-bit instructions cannot be placed in any fixed-width branch delay slots, so the choice of the width of the breakpoint does not matter (not that we put any breakpoints in delay slots anyway, but it doesn't hurt to play safe anyway). Worse, however, that the lack of permission for C99 code in opcodes precludes the use of the long long type to hold instruction patterns and opcode masks there. And redefining "struct mips_opcode" to split instructions/masks across multiple members (or worse yet, using some hack to split them acros multiple opcode table entries) means rewriting lots of stuff at least in opcodes itself and in GAS. That's sort of discouraging for a single instruction (and no 64-bit microMIPS hardware yet), especially given that I think we will eventually permit at least the type itself if not other C99 features sometime. This is even more disappointing given these members are defined as the long type -- already wasting 32-bits twice per each entry throughout the table on 64-bit hosts... The end result is binutils do not support the LI32 instruction now, so while we do in GDB, there's actually no reasonable way to produce it (of course you can still handcode it as data with the .half directive -- note that just like MIPS16 extended instructions (and unlike standard MIPS ones) 32-bit and 48-bit microMIPS instructions are encoded as sequences of 16-bit halfwords each of which is in the processor's endianness). > > Coincidentally all-zeroes is a 32-bit NOP instruction both in the > > standard MIPS and the microMIPS mode -- there's a 16-bit encoding of NOP > > in the microMIPS mode naturally as well. > > I'm wondering if you'd like me to rename "null_insn" into "nop_insn" > in my patch. I didn't do it, because I'd expect the instruction size > to depend on the mode. As of today, we know that the breakpoint we > are inserting is always going to be at an even address, so it's always > going to be 4 bytes. So maybe it does make sense to rename it. Let > me know. I think "nop_insn" might be better; we handle different encodings of the NOP intruction in GAS already (used for .align padding among others) but from my understanding of this piece this is never going to be necessary here as we'll only ever need a standard MIPS and a 32-bit microMIPS breakpoint here. I think however that initialising it like this: static gdb_byte nop_insn[] = { 0, 0, 0, 0 }; may express the intent a bit better (this is what we do in mips_breakpoint_from_pc where we initialise all array elements explicitly even where we could let the compiler default to zeros). > > Understood, but I'd be happier if the comment you're removing or a > > similar stayed in place. If by trap you mean SIGTRAP, then I think this > > is not going to be the case. > > I think you refer to the comment from Andrew Cagney? I've put it back > as is. Yes, however following Mark's note I think it'll be better to state that it's OK if we get SIGSEGV for a non-executable stack and refer to handle_inferior_event (look for SIGSEGV there for a detailed explanation). Furthermore, given that it is supported I think it should simply be documented in the function's description instead. > OK to commit, modulo the possible rename above? OK with these changes; once you've checked your patch in I'll update the outstanding microMIPS change according to my earlier notes (and will probably come back with issues and haunt everybody here). Maciej