From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17880 invoked by alias); 12 Jun 2015 13:21:18 -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 17867 invoked by uid 89); 12 Jun 2015 13:21:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 12 Jun 2015 13:21:16 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 7AACB28CA5; Fri, 12 Jun 2015 09:21:14 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id rcYDZXJt9rGZ; Fri, 12 Jun 2015 09:21:14 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 6AD99286D7; Fri, 12 Jun 2015 09:21:14 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id F118248276; Fri, 12 Jun 2015 09:21:13 -0400 (EDT) Date: Fri, 12 Jun 2015 13:21:00 -0000 From: Joel Brobecker To: Martin Simmons Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Prevent internal-error when computing $pc in ARM assembly code Message-ID: <20150612132113.GC2609@adacore.com> References: <201505200949.t4K9nlqt015737@heapu.cam.lispworks.com> <20150609184408.GG2855@adacore.com> <201506111725.t5BHP5LR031293@higson.cam.lispworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201506111725.t5BHP5LR031293@higson.cam.lispworks.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-06/txt/msg00256.txt.bz2 > Thanks for looking at it. > > The code being debugged uses r11 (ARM_FP_REGNUM) as a global pointer > rather than a frame register, so lines 1968...1984 of arm-tdep.c ("We > have no symbol information.") computes bogus values for prologue_start > and prologue_end. OK, so indeed the problem occurs because arm_analyze_prologue was given a bogus PC range. Great! > > Also, for our piece of mind, we normally ask that the change be > > validated by running the testsuite. Did you do that? If yes, > > on which platform? > > Yes, I ran make check-gdb on Raspbian GNU/Linux 7, but it produces > around 1200 failures with or without my change, including some > non-deterministic ones. That's perfect. What we ask is that the failures before and after are the same. > Ah, sorry. Here is the new version. I've also included a new test, which > passes for me on Raspbian GNU/Linux 7. Awesome, we love tests! Thank you. > gdb/ChangeLog: > > * arm-tdep.c (arm_analyze_prologue): Read memory without throwing an > exception, to allow debugging of assembly code. > > gdb/testsuite/ChangeLog: > * gdb.arch/arm-r11-non-pointer.S: New file. > * gdb.arch/arm-r11-non-pointer.exp: New file. Looks good. A few little things in your new testcase... > +++ b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.S > @@ -0,0 +1,45 @@ > +/* Copyright 2010-2015 Free Software Foundation, Inc. Did you really mean for the date range to be 2010-2015? If yes, then great. If not, can you adjust it? > diff --git a/gdb/testsuite/gdb.arch/arm-r11-non-pointer.exp b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.exp > new file mode 100644 > index 0000000..1def957 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.exp > @@ -0,0 +1,69 @@ > +# Copyright 2010-2015 Free Software Foundation, Inc. Same here... > +set testfile "arm-r11-non-pointer" > +set srcfile ${testfile}.S > +set binfile ${objdir}/${subdir}/${testfile} Can you use "standard_testfile .S" instead of the 3 commands above? > +set additional_flags "-Wa,-g" > + > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } { > + untested arm-r11-non-pointer.exp > + return -1 > +} > + > +# Get things started. > + > +clean_restart ${testfile} Can you use prepare_for_testing instead of gdb_compile and clean_restart? > +gdb_test "x/i \$pc" \ > + ".*bl.*0x.*" \ > + "x/i pc" > + > +gdb_test "stepi" \ > + "\\\?\\\? \\\(\\\) at .*$srcfile:.*" \ > + "stepi" > + > +gdb_test "x/i \$pc" \ > + ".*ldr.*r11,.*" \ > + "x/i pc" > + > +gdb_test "stepi" \ > + "\\\?\\\? \\\(\\\) at .*$srcfile:.*" \ > + "stepi" > + > +gdb_test "x/i \$pc" \ > + ".*mov.*r11,.*r11.*" \ > + "x/i pc" > + We require that all test "labels" be unique. Would you mind ammending them to make them so. Eg: replace the first "x/i pc" by "x/i pc at such and such instruction", etc. Same for the "stepi" ones. Thank you, -- Joel