From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30775 invoked by alias); 26 Feb 2016 14:15:46 -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 30756 invoked by uid 89); 26 Feb 2016 14:15:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=occupy, callfuncsexp, callfuncs.exp, UD:callfuncs.exp X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 26 Feb 2016 14:15:40 +0000 Received: from svr-orw-fem-03.mgc.mentorg.com ([147.34.97.39]) by relay1.mentorg.com with esmtp id 1aZJBB-0003V6-Bu from Luis_Gustavo@mentor.com ; Fri, 26 Feb 2016 06:15:37 -0800 Received: from [172.30.3.146] (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.3.224.2; Fri, 26 Feb 2016 06:15:36 -0800 Subject: Re: [patch, avr] Fix incorrect argument register for push dummy call References: <20160225123448.GA10608@CHELT0346> To: Pitchumani Sivanupandi , From: Luis Machado Reply-To: Luis Machado Message-ID: <56D05E07.5000102@codesourcery.com> Date: Fri, 26 Feb 2016 14:15:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160225123448.GA10608@CHELT0346> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00857.txt.bz2 Hi, On 02/25/2016 09:34 AM, Pitchumani Sivanupandi wrote: > There are few failures for avr-gdb in callfuncs.exp. These failures are > due to the incorrect argument passing in case of push dummy call. The > last argument register is calculated incorrectly that caused this issue. > > Attached patch fixes the calculation so that the push dummy call aligns > with the code generated by avr-gcc. > > No new regressions found when tested with internal simulator. > > If ok, could someone commit please? > > Regards, > Pitchumani > > gdb/ChangeLog > > 2016-02-25 Pitchumani Sivanupandi > > * avr-tdep.c (AVR_LAST_ARG_REGNUM): Define. > (avr_push_dummy_call): Correct last needed argument register. > > > avr-last-argument-register-fix.patch > > > diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c > index 597cfb4..18ecfe4 100644 > --- a/gdb/avr-tdep.c > +++ b/gdb/avr-tdep.c > @@ -111,6 +111,7 @@ enum > > AVR_ARG1_REGNUM = 24, /* Single byte argument */ > AVR_ARGN_REGNUM = 25, /* Multi byte argments */ > + AVR_LAST_ARG_REGNUM = 8, /* Last argument register */ > > AVR_RET1_REGNUM = 24, /* Single byte return value */ > AVR_RETN_REGNUM = 25, /* Multi byte return value */ > @@ -1298,12 +1299,14 @@ avr_push_dummy_call (struct gdbarch *gdbarch, struct value *function, > const bfd_byte *contents = value_contents (arg); > int len = TYPE_LENGTH (type); > > - /* Calculate the potential last register needed. */ > - last_regnum = regnum - (len + (len & 1)); > + /* Calculate the potential last register needed. > + E.g. For length 2, registers regnum and regnum-1 (say 25 and 24) Is that how the code works? It seems a bit confusing, because we want to start in even-aligned registers, so we'd use r25:r24, r23:r22 etc. So last_regnum always ends up pointing at an odd-numbered register, so the code can decrement the odd-numbered register if needed (in case we have data of length 1/3/...). /* Skip a register for odd length args. */ if (len & 1) regnum--; > + shall be used. So, last needed register will be regnum-1(24). */ > + last_regnum = regnum - (len + (len & 1)) + 1; > Isn't this going to be incorrect for, say, data of length 3? last_regnum = 25 - (3 + (3 & 1)) + 1 -> 25 - (3 + 1) + 1 -> 22 But it should've been 21, because data would occupy registers 25:24 and 23:22 and then we'd need an even-aligned register to start with? > /* If there are registers available, use them. Once we start putting > stuff on the stack, all subsequent args go on stack. */ > - if ((si == NULL) && (last_regnum >= 8)) > + if ((si == NULL) && (last_regnum >= AVR_LAST_ARG_REGNUM)) > { > ULONGEST val; >