From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27670 invoked by alias); 24 Jun 2011 03:55:29 -0000 Received: (qmail 27661 invoked by uid 22791); 24 Jun 2011 03:55:28 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 24 Jun 2011 03:55:10 +0000 Received: (qmail 2238 invoked from network); 24 Jun 2011 03:55:10 -0000 Received: from unknown (HELO ?192.168.0.101?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Jun 2011 03:55:10 -0000 Message-ID: <4E040A9A.5020807@codesourcery.com> Date: Fri, 24 Jun 2011 03:55:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703) References: <000001cc3216$b96ba290$2c42e7b0$@guo@arm.com> In-Reply-To: <000001cc3216$b96ba290$2c42e7b0$@guo@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes 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 X-SW-Source: 2011-06/txt/msg00365.txt.bz2 On 06/24/2011 10:30 AM, Terry Guo wrote: > Hello, > > This patch addresses the bug in gdb/12703 which sets two different function > breakpoints at same pc address. In this patch I enhanced the way to analyze > the ARM thumb prologue to prevent the function breakpoint from being set > outside the function body. > > Patch has been tested against arm-none-eabi with no regressions. OK for > commit? > > Thanks, > > Terry > I am not the people to approve or reject this patch. My $0.2 here. > gdb/ChangLog: > 2011-06-16 Terry Guo > > PR gdb/12703 > * arm-tdep.c (arm_skip_prologue): Don't scan beyond the end of > the current function. > > gdb/testsuite/ChangLog: > 2011-06-16 Terry Guo > > PR gdb/12703 > * gdb.base/break-function.c: New testcase. > * gdb.base/break-function.exp: New script. IMO, this is a target-specific bug, so this PR's component should be tdept, so it should be "PR tdept/12703" instead of "PR gdb/12703". I'd move your test cases break-function.{c,exp} to gdb.arch/ dir, because it is target-dependent fix. I am sure this case is useful to other ports. > --- /dev/null > +++ gdb/testsuite/gdb.base/break-function.c > @@ -0,0 +1,47 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 1992, 1993, 1994, 1995, 1999, 2002, 2003, 2007, 2008, 2009, > 2010, > + 2011 Free Software Foundation, Inc. > + Your test case is a new one, so the year of copyright should be 2011. Please remove the rest of them. > + > + > +unsigned long _etext; > +unsigned long _data; > +unsigned long _edata; > + > +void foo(void) This doesn't comply to GNU coding standard. Please move "foo ()" to next line. > +{ > + while(1) ^ need a space here. > + { > + } > +} /* End of function foo */ Put a "." at the end of comment with two spaces, or one space without ".". > + > +void bar(void) "bar" should be move to next newline. A space is needed ater "bar". > +{ > + unsigned long *pulSrc, *pulDest; > + > + pulSrc = &_etext; > + for(pulDest = &_data; pulDest < &_edata; ) > + { > + *pulDest++ = *pulSrc++; > + } > +} Indentation is not correct here. > +# Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999, > +# 2000, 2002, 2003, 2007, 2008, 2009, 2010, 2011 > +# Free Software Foundation, Inc. > + Only 2011 is needed here, because this case is created in 2011. > + > +set srcfile break-function.c > + > +if { [prepare_for_testing break-function.exp "break-function" > {break-function.c}] } { > + return -1 > +} > + > +set bp_location_boundary [gdb_get_line_number "End of function foo"] > + > +gdb_test_multiple "b foo" "Set breakpoint for function foo" { > + -re "Breakpoint 1 at.*file.*, line (\[0-9\]+).*" { When using gdb_test_multiple, "$gdb_prompt $" is needed at the end of your regex. Please reference gdb_test_multiple used in other places. > + set bp_pos $expect_out(1,string); > + if { $bp_pos > $bp_location_boundary } { > + fail "The location of function breakpoint exceeds the body of > the function.\n" > + } else { > + pass "PASS"; The PASS message and FAIL message should be the same. I had the same problem in my patch yesterday. :) -- Yao (齐尧)