Message ID | 53288819.7080903@embecosm.com |
---|---|
State | Committed |
Headers |
Return-Path: <x14314964@homiemail-mx22.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (caibbdcaabij.dreamhost.com [208.113.200.189]) by wilcox.dreamhost.com (Postfix) with ESMTP id AA5D23600A5 for <siddhesh@wilcox.dreamhost.com>; Tue, 18 Mar 2014 10:53:39 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14314964) id 583654E6311D; Tue, 18 Mar 2014 10:53:39 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 397C74E6311C for <gdb@patchwork.siddhesh.in>; Tue, 18 Mar 2014 10:53:39 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type; q=dns; s=default; b=HdRyemJoFE4j6djofW48yxKQpgXwM UC7jg0sjoKRUY0rwzcJmmqqJl2TY8egSGvU6YNqb1c4lNkgwOaAGzajqYbBSNa/T mwaW0Qc5lUaRVcXI+oja6hEh/Jh8LHf14cX6d0FxhpFxOQHNlFOLexXkcvFHZPpO kBKLHb0WdV+2jU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type; s=default; bh=HwNiaIy3tqnrS5rDRbytookL/Dk=; b=KPp 5TRagwLqZGgjOUOPTZPwW0957k6nY70almMmqgC+8zGKADtAQPq3e9XZhn8Sb1em h6tsN682+HsiYcw4U3uyBi5YfNedb4ZYavISCQbTYTZ6ODNnP7rWYVyP3wtuVPfa JFhI9VrHRdwDmh7/m+Sunslmw1AsZj47KKKLOZr4= Received: (qmail 4808 invoked by alias); 18 Mar 2014 17:53:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-gdb=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4798 invoked by uid 89); 18 Mar 2014 17:53:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-wi0-f170.google.com Received: from mail-wi0-f170.google.com (HELO mail-wi0-f170.google.com) (209.85.212.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 18 Mar 2014 17:53:34 +0000 Received: by mail-wi0-f170.google.com with SMTP id bs8so2985488wib.1 for <gdb-patches@sourceware.org>; Tue, 18 Mar 2014 10:53:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:content-type; bh=CC3FDEuU2+Gg0Zw4eAY7X3U2zj4+Pgr5Y77JcgfY/UI=; b=Cs7hEBoJ9x54iAgaG4a3Ay+DuOs/IW4PnHRABfSYKVLXN3AhMvGtk2mUNT6vAP6WbI mSO0Px62XlPT5i8fM1DDSPcCBuMUFNY2N1vqpCAAP38ixVXSHhHrK2SuB8wnTgQDYW+e nCZoY3PcQ1h7oAk5NC/I91F3YhRI1FsH6Tlui+C0bXL2kcoWMftj9XXTVnXgkqj/dPan K1VspMzBrq1sRFZQLFKy6jHte/H9wS55PqrEU6gaLlYRXdOVp4Ru1OgIrCAH+Je+m2JM a2ZK5nkH9v508NkN5b9ayZYa5K9Td0t7VistdmXaTEo1w/9oA1rGGRUyj98lUmOlzG2a o/KQ== X-Gm-Message-State: ALoCoQnFq3VCsOBpZddrTg+fxTRDXaEGanDgM2iZFu7acU7RYDwpKTwNQ9me56VNOVZLAp4Hb2sy X-Received: by 10.194.9.99 with SMTP id y3mr2883492wja.60.1395165211767; Tue, 18 Mar 2014 10:53:31 -0700 (PDT) Received: from [192.168.0.134] (cust64-dsl91-135-5.idnet.net. [91.135.5.64]) by mx.google.com with ESMTPSA id t6sm36026939wix.4.2014.03.18.10.53.29 for <gdb-patches@sourceware.org> (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 18 Mar 2014 10:53:30 -0700 (PDT) Message-ID: <53288819.7080903@embecosm.com> Date: Tue, 18 Mar 2014 17:53:29 +0000 From: Pierre Langlois <pierre.langlois@embecosm.com> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: [PATCH][PR/backtrace 16721] Fix erroneous backtrace on AVR Content-Type: multipart/mixed; boundary="------------030809030506060702010500" X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in |
Commit Message
Pierre Langlois
March 18, 2014, 5:53 p.m. UTC
Hi all, Looking at stack unwinding on AVR, I noticed the current frame was not always correctly detected from the function prologue. This bug occurs only with the following prologue, referred to as "Method 2: Adjust stack pointer" in GCC: gcc/config/avr/avr.c (avr_prologue_setup_frame). --> push the old frame pointer push r28 push r29 --> allocate new space rcall .+0 push r1 --> move fp <- sp in r28, 0x3d in r29, 0x3e GCC uses "rcall .+0" and "push r1" to adjust the stack pointer, "rcall" pushing automatically 2 or 3 bytes on the stack, depending on the target. GDB should scan this prologue and find out the size of the frame but it is incorrect by one because it expects "push r0" and not "push r1". I believe this register was changed in GCC withcommit 915f904be. Best, Pierre 2014-03-18 Pierre Langlois <pierre.langlois@embecosm.com> * avr-tdep.c (avr_scan_prologue): Accept push r1 instruction for small stack allocation. ----------------------------------------------------------------------------------------- GNU gdb (AVR 8-bit toolchain (built 20140310)) 7.7.50.20140318-cvs Copyright (C) 2014 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "--host=x86_64-unknown-linux-gnu --target=avr". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.atmel.com>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word". (gdb) file atmega-test.elf Reading symbols from atmega-test.elf...done. (gdb) target remote :51000 Remote debugging using :51000 0x00000116 in multiply (a=25, b=8) at main.c:4 4 return a * b; (gdb) monitor reset (gdb) load Loading section .text, size 0x1a4 lma 0x0 Start address 0x0, load size 420 Transfer rate: 3360 bits in <1 sec, 210 bytes/write. (gdb) b multiply Breakpoint 1 at 0x114: file main.c, line 4. (gdb) c Continuing. Program received signal SIGTRAP, Trace/breakpoint trap. 0x00000116 in multiply (a=25, b=8) at main.c:4 4 return a * b; (gdb) bt #0 0x00000116 in multiply (a=25, b=8) at main.c:4 #1 0x01e00000 in ?? () (gdb) q A debugging session is active. Inferior 1 [Remote target] will be killed. Quit anyway? (y or n)
Comments
On 18 Mar 2014, at 18:53, Pierre Langlois <pierre.langlois@embecosm.com> wrote: > Hi all, > > Looking at stack unwinding on AVR, I noticed the current frame was not always correctly > detected from the function prologue. > > This bug occurs only with the following prologue, referred to as "Method 2: Adjust stack pointer" > in GCC: gcc/config/avr/avr.c (avr_prologue_setup_frame). > > --> push the old frame pointer > push r28 > push r29 > > --> allocate new space > rcall .+0 > push r1 > > --> move fp <- sp > in r28, 0x3d > in r29, 0x3e > > GCC uses "rcall .+0" and "push r1" to adjust the stack pointer, "rcall" pushing > automatically 2 or 3 bytes on the stack, depending on the target. > > GDB should scan this prologue and find out the size of the frame but it is incorrect by one > because it expects "push r0" and not "push r1". > > I believe this register was changed in GCC withcommit 915f904be. Looks good to me. Tristan (as former AVR maintainer) > > Best, > > Pierre > > 2014-03-18 Pierre Langlois <pierre.langlois@embecosm.com> > > * avr-tdep.c (avr_scan_prologue): Accept push r1 instruction for small > stack allocation. > > ----------------------------------------------------------------------------------------- > > GNU gdb (AVR 8-bit toolchain (built 20140310)) 7.7.50.20140318-cvs > Copyright (C) 2014 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> > This is free software: you are free to change and redistribute it. > There is NO WARRANTY, to the extent permitted by law. Type "show copying" > and "show warranty" for details. > This GDB was configured as "--host=x86_64-unknown-linux-gnu --target=avr". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <http://www.atmel.com>. > Find the GDB manual and other documentation resources online at: > <http://www.gnu.org/software/gdb/documentation/>. > For help, type "help". > Type "apropos word" to search for commands related to "word". > (gdb) file atmega-test.elf > Reading symbols from atmega-test.elf...done. > (gdb) target remote :51000 > Remote debugging using :51000 > 0x00000116 in multiply (a=25, b=8) at main.c:4 > 4 return a * b; > (gdb) monitor reset > (gdb) load > Loading section .text, size 0x1a4 lma 0x0 > Start address 0x0, load size 420 > Transfer rate: 3360 bits in <1 sec, 210 bytes/write. > (gdb) b multiply > Breakpoint 1 at 0x114: file main.c, line 4. > (gdb) c > Continuing. > > Program received signal SIGTRAP, Trace/breakpoint trap. > 0x00000116 in multiply (a=25, b=8) at main.c:4 > 4 return a * b; > (gdb) bt > #0 0x00000116 in multiply (a=25, b=8) at main.c:4 > #1 0x01e00000 in ?? () > (gdb) q > A debugging session is active. > > Inferior 1 [Remote target] will be killed. > > Quit anyway? (y or n) > > <pr-16721.patch>
> 2014-03-18 Pierre Langlois <pierre.langlois@embecosm.com> > > * avr-tdep.c (avr_scan_prologue): Accept push r1 instruction for small > stack allocation. Now that Tristan approved the patch, do you have an FSF copyright assignment on file? This patch is small that we can push it in without; but if you are planning on sending more, you'll need one. Just let me know and I will get you a message to get you started (it takes a while to complete, so the sooner the better). One remark on the ChangeLog; the formatting is off a bit. It should be indented by one tab, and the indentation should be the same, regardless of whether the line starts with a '*' or not - no extra indentation for continuation lines. Thus: * avr-tdep.c (avr_scan_prologue): Accept push r1 instruction for small stack allocation. (in emails, it's not important to be using 8 spaces or 1 tab, but in the CL, we tend to be picky about it). Thanks!
Hi Joel, Sorry for the late reply. > Now that Tristan approved the patch, do you have an FSF copyright > assignment on file? This patch is small that we can push it in without; > but if you are planning on sending more, you'll need one. Just let me > know and I will get you a message to get you started (it takes a while > to complete, so the sooner the better). I am doing this work on behalf of Embecosm, we have an FSF copyright assignment on file. > One remark on the ChangeLog; the formatting is off a bit. It should be > indented by one tab, and the indentation should be the same, regardless > of whether the line starts with a '*' or not - no extra indentation > for continuation lines. Thus: > > * avr-tdep.c (avr_scan_prologue): Accept push r1 instruction for small > stack allocation. > > (in emails, it's not important to be using 8 spaces or 1 tab, but in > the CL, we tend to be picky about it). I'll make sure my email client is properly configured next time. Thank you very much! Pierre
> Sorry for the late reply. No worries. > I am doing this work on behalf of Embecosm, we have an FSF copyright > assignment on file. Perfect! > > * avr-tdep.c (avr_scan_prologue): Accept push r1 instruction for small > > stack allocation. I can push this patch for you, but you now also meet the requirements for write access to the GDB repository (FSF assignment + 1 good patch). If you are planning on contributing more patches, I suggest we get you write access and let you commit yourself. WDYT? If not interested, just let me know and I will push the patch. Otherwise, can you contact me in private for further instructions on how to achieve that? Thanks,
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c index 6e58f04..7fb16d1 100644 --- a/gdb/avr-tdep.c +++ b/gdb/avr-tdep.c @@ -720,7 +720,7 @@ avr_scan_prologue (struct gdbarch *gdbarch, CORE_ADDR pc_beg, CORE_ADDR pc_end, info->size += gdbarch_tdep (gdbarch)->call_length; vpc += 2; } - else if (insn == 0x920f) /* push r0 */ + else if (insn == 0x920f || insn == 0x921f) /* push r0 or push r1 */ { info->size += 1; vpc += 2;