From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11261 invoked by alias); 14 Dec 2011 23:21:38 -0000 Received: (qmail 11247 invoked by uid 22791); 14 Dec 2011 23:21:37 -0000 X-SWARE-Spam-Status: No, hits=-5.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 14 Dec 2011 23:21:19 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pBENLIew028983 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 14 Dec 2011 18:21:18 -0500 Received: from mesquite.lan (ovpn-113-28.phx2.redhat.com [10.3.113.28]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id pBENLHhU016757 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO) for ; Wed, 14 Dec 2011 18:21:18 -0500 Date: Wed, 14 Dec 2011 23:24:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: Re: [RFC] mips-tdep.c: Fix mips16 bit rot Message-ID: <20111214162117.0b1ce834@mesquite.lan> In-Reply-To: References: <20101213170635.6f0c4356@mesquite.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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: 2011-12/txt/msg00492.txt.bz2 Hi Maciej, Sorry it's taken me so long to get back to you this. I will try to answer your questions to the best of my ability, but it's been a while since I've looked at or thought about this code. I fear that I might not be of much help... On Wed, 23 Nov 2011 12:40:37 +0000 "Maciej W. Rozycki" wrote: > First of all this construct used in a few places: > > + if (is_mips16_addr (pc)) > + pc = unmake_mips16_addr (pc); > > makes me a bit nervous you might be removing the ISA bit for code > references where it is the only means of signalling GDB the address is a > MIPS16 code address -- that would happen if pc pointed to a location that > has no associated symbol information. While in this case the > functionality GDB provides is limited, it still has to work correctly up > to expectations, e.g. instruction-level single-stepping has to work and > where software stepping is used the MIPS16 BREAK instruction encoding has > to be used rather than the MIPS32 one. Have you verified this > functionality has not regressed? Similarly the MIPS16 heuristic frame > unwinder may be affected. > > Otherwise I'd just be tempted to change all these cases into something > functionally equivalent to: > > + if (msymbol_is_special (lookup_minimal_symbol_by_pc (pc))) > + pc = unmake_mips16_addr (pc); > > where the ISA bit is only stripped if there's symbol information > indicating this is a piece of MIPS16 code so there's no information lost. I see your point. You proposed change looks reasonable to me. > Second, you only made corresponding adjustments to > mips_eabi_push_dummy_call and mips_o64_push_dummy_call which are the least > standard and probably the least used MIPS ABIs. Any particular reason you > did not make similar changes to mips_o32_push_dummy_call or > mips_n32n64_push_dummy_call? > > Also I see you only adjust function pointers that are arguments on their > own -- isn't a similar adjustment required for such pointers that are > parts of aggregate types as well? I don't remember enough about what I did roughly a year ago to be able to answer this. I think it's likely that changes should be made to the areas that you've identified. > Overall, what was the rationale behing your change? -- as it's unclear to > me from your e-mail. Did you just want to fix test results you discovered > that were quite poor or does this change address problems you stumbled > across during actual MIPS16 debugging? The former - I was attempting to fix some very bad test results with respect to mips16. Kevin