From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37992 invoked by alias); 1 Oct 2015 00:35:53 -0000 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 Received: (qmail 37982 invoked by uid 89); 1 Oct 2015 00:35:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_50,KAM_STOCKGEN,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 01 Oct 2015 00:35:51 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id BE65491E9A for ; Thu, 1 Oct 2015 00:35:50 +0000 (UTC) Received: from pinnacle.lan ([10.3.113.5]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t910Zola024114 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO) for ; Wed, 30 Sep 2015 20:35:50 -0400 Date: Thu, 01 Oct 2015 00:35:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: Re: [PATCH, FT32] Correctly interpret function prologs Message-ID: <20150930173548.1be446d4@pinnacle.lan> In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg00000.txt.bz2 Hi James, See my comments inline with your patch below. On Tue, 29 Sep 2015 16:38:57 +0000 James Bowman wrote: > The stack unwinder did not understand the function prologs > generated by gcc with -Os. Add code to recognize and interpret the > prolog calls. > > OK to apply? > > > 2015-09-29 James Bowman > > * ft32-tdep.c (ft32_analyze_prologue): Add function prolog > subroutine handling. > > diff --git a/gdb/ft32-tdep.c b/gdb/ft32-tdep.c > index 00cf847..0b51af3 100644 > --- a/gdb/ft32-tdep.c > +++ b/gdb/ft32-tdep.c > @@ -164,33 +164,66 @@ ft32_analyze_prologue (CORE_ADDR start_addr, CORE_ADDR end_addr, > CORE_ADDR next_addr; > ULONGEST inst, inst2; > LONGEST offset; > - int regnum; > + int regnum, pushreg; > + struct bound_minimal_symbol msymbol; > + unsigned prologs[32]; I think that the type of prologs[] should be CORE_ADDR instead of `unsigned'. > > cache->saved_regs[FT32_PC_REGNUM] = 0; > cache->framesize = 0; > I think it'd be useful to have a brief comment somewhere in here describing what a call to __prolog_$rN does. I'm guessing that these functions push a number of registers. It'd be useful to know, for a given N, which registers are pushed and the order in which they're pushed. > + for (regnum = 0; regnum < 32; regnum++) > + { > + char prolog_symbol[32]; > + > + snprintf (prolog_symbol, sizeof (prolog_symbol), "__prolog_$r%02d", > + regnum); > + msymbol = lookup_minimal_symbol (prolog_symbol, NULL, NULL); > + if (msymbol.minsym) > + prologs[regnum] = BMSYMBOL_VALUE_ADDRESS (msymbol); > + else > + prologs[regnum] = 0; > + } > + > if (start_addr >= end_addr) > - return end_addr; > + return end_addr; > > cache->established = 0; > - for (next_addr = start_addr; next_addr < end_addr; ) > + for (next_addr = start_addr; next_addr < end_addr;) > { > inst = read_memory_unsigned_integer (next_addr, 4, byte_order); > > if (FT32_IS_PUSH (inst)) > { > - regnum = FT32_R0_REGNUM + FT32_PUSH_REG (inst); > + pushreg = FT32_PUSH_REG (inst); > cache->framesize += 4; > - cache->saved_regs[regnum] = cache->framesize; > + cache->saved_regs[FT32_R0_REGNUM + pushreg] = cache->framesize; > next_addr += 4; > } > + else if (FT32_IS_CALL (inst)) > + { > + for (regnum = 0; regnum < 32; regnum++) > + { > + if ((4 * (inst & 0x3ffff)) == prologs[regnum]) > + { > + for (pushreg = 13; pushreg <= regnum; pushreg++) This looks strange to me. The outer loop has regnum ranging from 0 thru 31. But this inner loop won't be executed for regnum values between 0 thru 12 due to pushreg starting at 13. Is that really what you want? If so, it seems to me that calls to __prolog_$r01 thru __prolog_$r12 don't contribute anything to the frame. Please add a comment about this if that's truly the case. > + { > + cache->framesize += 4; > + cache->saved_regs[FT32_R0_REGNUM + pushreg] = > + cache->framesize; > + } > + next_addr += 4; > + } > + } > + break; > + } > else > break; > } > for (regnum = FT32_R0_REGNUM; regnum < FT32_PC_REGNUM; regnum++) > { > if (cache->saved_regs[regnum] != REG_UNAVAIL) > - cache->saved_regs[regnum] = cache->framesize - cache->saved_regs[regnum]; > + cache->saved_regs[regnum] = > + cache->framesize - cache->saved_regs[regnum]; > } > cache->saved_regs[FT32_PC_REGNUM] = cache->framesize; >