From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7659 invoked by alias); 17 Feb 2010 09:02:18 -0000 Received: (qmail 7465 invoked by uid 22791); 17 Feb 2010 09:02:16 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (212.99.106.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 17 Feb 2010 09:02:13 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id EBCB6CB02C8; Wed, 17 Feb 2010 10:02:10 +0100 (CET) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id itlmi1oEVq0G; Wed, 17 Feb 2010 10:02:10 +0100 (CET) Received: from ulanbator.act-europe.fr (ulanbator.act-europe.fr [10.10.1.67]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id DAF74CB02BD; Wed, 17 Feb 2010 10:02:10 +0100 (CET) Subject: Re: a review and questions on avr_scan_prologue() Mime-Version: 1.0 (Apple Message framework v1077) Content-Type: text/plain; charset=iso-8859-1 From: Tristan Gingold In-Reply-To: <6a6f635a1002131556sc428adfu6033489930eca7b6@mail.gmail.com> Date: Wed, 17 Feb 2010 09:02:00 -0000 Cc: gdb@sourceware.org Content-Transfer-Encoding: quoted-printable Message-Id: <919F238C-4ED8-4507-8510-E5479E94B32D@adacore.com> References: <6a6f635a1002131556sc428adfu6033489930eca7b6@mail.gmail.com> To: =?iso-8859-1?Q?Petr_Hluz=EDn?= X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2010-02/txt/msg00114.txt.bz2 On Feb 14, 2010, at 12:56 AM, Petr Hluz=EDn wrote: > Hello Hi, > I took a look at avr-tdep.c [1] and I found some places which are > either bug or are not clear to me. Here it goes: Thanks for doing reviews! > else if (len >=3D sizeof (img) - 2 > && memcmp (img + 2, prologue, sizeof (img) - 2) =3D=3D 0) > { > info->prologue_type =3D AVR_PROLOGUE_SIG; > vpc +=3D sizeof (img) - 2; > info->saved_regs[AVR_SREG_REGNUM].addr =3D 3; > info->saved_regs[0].addr =3D 2; > info->saved_regs[1].addr =3D 1; > - info->size +=3D 3; > + info->size +=3D 2; > } >=20 > Since the "img + 2" skips "push r1" I believe the scan should record > smaller size. Yes, you're right. I will fix that. > if (vpc >=3D AVR_MAX_PROLOGUE_SIZE) > fprintf_unfiltered (gdb_stderr, > _("Hit end of prologue while scanning pushes\n")); >=20 > This condition is never true due to a way `len' is calculated and > `vpc' always being less than `len'. (This is not a bug but per se but > the author might expected something what is not true.) I will change that to an assert. > else if (insn =3D=3D 0x920f) /* push r0 */ > { > info->size +=3D 1; > vpc +=3D 2; > } >=20 > The condition is never true because of the preceding "Scan pushes > (saved registers)" loop's exit condition. I don't think so. You can have: rcall .+0 push r0 > Also: > The avr_scan_prologue()'s recognizes several well-known prologues. Is > there a reason why it does not use the general prologue analysis > algorithm as described in the documentation [2]? Historical reason: it was written before the general prologue analysis. Then, when AdaCore did its AVR port, I fixed all issues I found, but didn't= rewrite it from scratch. > I think universal prologue analysis is quite easy with AVR arch. The > code might be shorter (though less clear). > I might try to write the code if you are interested. > (The current prologue scan code chokes on hand-crafted assembly.) Feel free to work on that. Improvements are always welcome! Tristan.