From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4352 invoked by alias); 2 Feb 2008 01:20:11 -0000 Received: (qmail 4344 invoked by uid 22791); 2 Feb 2008 01:20:09 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 02 Feb 2008 01:19:52 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 2F9882A9C95 for ; Fri, 1 Feb 2008 20:19:50 -0500 (EST) 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 qMXoHqBsuivI for ; Fri, 1 Feb 2008 20:19:50 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 838FA2A9C7B for ; Fri, 1 Feb 2008 20:19:49 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 422B3E7ACB; Fri, 1 Feb 2008 17:19:46 -0800 (PST) Date: Sat, 02 Feb 2008 01:20:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: Re: [RFC/RFA?] Should break FILE:LINENO skip prologue? Message-ID: <20080202011945.GA13812@adacore.com> References: <20080109151745.GA13181@adacore.com> <20080131220814.GB6326@caradoc.them.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Q68bSM7Ycu6FN28Q" Content-Disposition: inline In-Reply-To: <20080131220814.GB6326@caradoc.them.org> User-Agent: Mutt/1.4.2.2i 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: 2008-02/txt/msg00041.txt.bz2 --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 1668 Hi Daniel, > > 2008-01-09 Joel Brobecker > > > > * breakpoint.c (skip_prologue_sal): New function. > > (resolve_sal_pc): Adjust SAL past prologue if the SAL was > > computed from a line number. > > I tried to reread the thread below this patch, but eventually got > lost. What was the verdict? As I said yesterday, the idea eventually got accepted by everyone. Here is the patch again, together with the associated adjustments to the testsuite. 2008-02-01 Joel Brobecker * breakpoint.c (skip_prologue_sal): New function. (resolve_sal_pc): Adjust SAL past prologue if the SAL was computed from a line number. 2008-02-01 Joel Brobecker * gdb.base/ending-run.exp: Use the first line of code inside function body to test breakpoints. * gdb.mi/mi-break.exp, gdb.mi/mi2-break.exp: Adjust the actual location where the breakpoint is inserted when using the line where a function is declared. Fix typo in the description of one of the tests. * gdb.mi/mi-simplerun.exp, gdb.mi/mi2-simplerun.exp: Likewise. Tested on x86-linux. In the first testcase, the adjustment I made was by avoiding the use of the line where the function is declared or where the open curly brace is. By looking at the testcase, I think it still preserves the intent of the testcase. In the second case, I kept the breakpoints where they are, but adjusted the expected output to reflect the new behavior. That leaves the documentation to be updated (and perhaps a NEWS entry?) OK to apply? Thanks, -- Joel --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="skip-prologue.diff" Content-length: 1275 Index: breakpoint.c =================================================================== --- breakpoint.c (revision 117) +++ breakpoint.c (revision 118) @@ -5446,6 +5446,25 @@ gdb_breakpoint (char *address, char *con 0); } +/* Adjust SAL to the first instruction past the function prologue. + The end of the prologue is determined using the line table from + the debugging information. + + If SAL is already past the prologue, then do nothing. */ + +static void +skip_prologue_sal (struct symtab_and_line *sal) +{ + struct symbol *sym = find_pc_function (sal->pc); + struct symtab_and_line start_sal; + + if (sym == NULL) + return; + + start_sal = find_function_start_sal (sym, 1); + if (sal->pc < start_sal.pc) + *sal = start_sal; +} /* Helper function for break_command_1 and disassemble_command. */ @@ -5460,6 +5479,11 @@ resolve_sal_pc (struct symtab_and_line * error (_("No line %d in file \"%s\"."), sal->line, sal->symtab->filename); sal->pc = pc; + + /* If this SAL corresponds to a breakpoint inserted using + a line number, then skip the function prologue if necessary. */ + if (sal->explicit_line) + skip_prologue_sal (sal); } if (sal->section == 0 && sal->symtab != NULL) --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="skip-prologue-tc.diff" Content-length: 10318 Index: gdb.base/ending-run.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ending-run.exp,v retrieving revision 1.30 diff -u -p -r1.30 ending-run.exp --- gdb.base/ending-run.exp 1 Jan 2008 22:53:18 -0000 1.30 +++ gdb.base/ending-run.exp 2 Feb 2008 01:08:55 -0000 @@ -55,9 +55,9 @@ gdb_load ${binfile} gdb_test "b ending-run.c:1" ".*Breakpoint.*ending-run.c, line 1.*" \ "bpt at line before routine" -gdb_test "b ending-run.c:13" \ - ".*Note.*also.*Breakpoint 2.*ending-run.c, line 13.*" \ - "b ending-run.c:13, one" +gdb_test "b ending-run.c:14" \ + ".*Note.*also.*Breakpoint 2.*ending-run.c, line 14.*" \ + "b ending-run.c:14, one" # Set up to go to the next-to-last line of the program # @@ -67,7 +67,7 @@ gdb_test "b ending-run.c:31" ".*Breakpoi # as line "13". Then try to clear it--this should work. # gdb_run_cmd -gdb_test "" ".*Breakpoint.*1.*callee.*13.*" "run" +gdb_test "" ".*Breakpoint.*1.*callee.*14.*" "run" gdb_test "cle" ".*Deleted breakpoints 1 2.*" "clear worked" send_gdb "i b\n" @@ -86,30 +86,17 @@ gdb_expect { # Test some other "clear" combinations # gdb_test "b ending-run.c:1" ".*Breakpoint.*4.*" -gdb_test "b ending-run.c:13" ".*Note.*also.*Breakpoint.*5.*" "b ending-run.c:13, two" -gdb_test "cle ending-run.c:13" \ - ".*Deleted breakpoint 5.*" "Only cleared 1 by line" - -send_gdb "inf line ending-run.c:13\n" -gdb_expect { - -re ".*address (0x\[0-9a-fA-F]*).*$gdb_prompt $" { - set line_eight $expect_out(1,string) - gdb_test "b 13" ".*Breakpoint.*6.*" - gdb_test "cle *$line_eight" ".*Deleted breakpoints 4 6.*" "Clear 2 by address" - } - -re ".*$gdb_prompt $" { - fail "need to fix test for new compile outcome" - } -} +gdb_test "b ending-run.c:14" ".*Note.*also.*Breakpoint.*5.*" "b ending-run.c:14, two" +gdb_test "cle ending-run.c:14" \ + ".*Deleted breakpoints 4 5.*" "Cleared 2 by line" send_gdb "inf line ending-run.c:14\n" gdb_expect { -re ".*address (0x\[0-9a-fA-F]*).*$gdb_prompt $" { set line_nine $expect_out(1,string) - gdb_test "b ending-run.c:14" ".*Breakpoint 7.*ending-run.c, line 14.*" - gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 8.*" "Breakpoint 7 at *ending-run.c:14" - gdb_test "c" ".*Breakpoint.*7.*callee.*14.*" - gdb_test "cle" ".*Deleted breakpoints 7 8.*" "Clear 2 by default" + gdb_test "b ending-run.c:14" ".*Breakpoint 6.*ending-run.c, line 14.*" + gdb_test "b *$line_nine" ".*Note.*also.*Breakpoint 7.*" "Breakpoint 7 at *ending-run.c:14" + gdb_test "cle" ".*Deleted breakpoints 6 7.*" "Clear 2 by default" } -re ".*$gdb_prompt $" { fail "need to fix test for new compile outcome" Index: gdb.mi/mi-break.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-break.exp,v retrieving revision 1.15 diff -u -p -r1.15 mi-break.exp --- gdb.mi/mi-break.exp 1 Feb 2008 16:24:47 -0000 1.15 +++ gdb.mi/mi-break.exp 2 Feb 2008 01:09:31 -0000 @@ -87,12 +87,12 @@ proc test_tbreak_creation_and_listing {} "insert temp breakpoint at basics.c:callee2" mi_gdb_test "444-break-insert -t basics.c:$line_callee3_head" \ - "444\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",${fullname},line=\"$line_callee3_head\",times=\"0\"\}" \ - "insert temp breakpoint at basics.c:\$line_callee3_body" + "444\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",${fullname},line=\"$line_callee3_body\",times=\"0\"\}" \ + "insert temp breakpoint at basics.c:\$line_callee3_head" # Getting the quoting right is tricky. That is "\"\":$line_callee4_head" mi_gdb_test "555-break-insert -t \"\\\"${srcfile}\\\":$line_callee4_head\"" \ - "555\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",${fullname},line=\"$line_callee4_head\",times=\"0\"\}" \ + "555\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",${fullname},line=\"$line_callee4_body\",times=\"0\"\}" \ "insert temp breakpoint at \"\":\$line_callee4_head" mi_gdb_test "666-break-list" \ Index: gdb.mi/mi-simplerun.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-simplerun.exp,v retrieving revision 1.17 diff -u -p -r1.17 mi-simplerun.exp --- gdb.mi/mi-simplerun.exp 1 Jan 2008 22:53:20 -0000 1.17 +++ gdb.mi/mi-simplerun.exp 2 Feb 2008 01:09:32 -0000 @@ -52,7 +52,9 @@ proc test_breakpoints_creation_and_listi global hex set line_callee4_head [gdb_get_line_number "callee4 ("] + set line_callee4_body [expr $line_callee4_head + 2] set line_callee3_head [gdb_get_line_number "callee3 ("] + set line_callee3_body [expr $line_callee3_head + 2] set line_callee2_head [gdb_get_line_number "callee2 ("] set line_callee2_body [expr $line_callee2_head + 2] set line_main_head [gdb_get_line_number "main ("] @@ -75,11 +77,11 @@ proc test_breakpoints_creation_and_listi "insert breakpoint at basics.c:callee2" mi_gdb_test "202-break-insert basics.c:$line_callee3_head" \ - "202\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",line=\"$line_callee3_head\",times=\"0\"\}" \ + "202\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",line=\"$line_callee3_body\",times=\"0\"\}" \ "insert breakpoint at basics.c:\$line_callee3_head" mi_gdb_test "203-break-insert \"\\\"${srcfile}\\\":$line_callee4_head\"" \ - "203\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"$line_callee4_head\",times=\"0\"\}" \ + "203\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"$line_callee4_body\",times=\"0\"\}" \ "insert breakpoint at \"\":\$line_callee4_head" mi_gdb_test "204-break-list" \ Index: gdb.mi/mi2-break.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-break.exp,v retrieving revision 1.8 diff -u -p -r1.8 mi2-break.exp --- gdb.mi/mi2-break.exp 1 Jan 2008 22:53:20 -0000 1.8 +++ gdb.mi/mi2-break.exp 2 Feb 2008 01:09:33 -0000 @@ -88,12 +88,12 @@ proc test_tbreak_creation_and_listing {} "insert temp breakpoint at basics.c:callee2" mi_gdb_test "444-break-insert -t basics.c:$line_callee3_head" \ - "444\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",${fullname},line=\"$line_callee3_head\",times=\"0\"\}" \ - "insert temp breakpoint at basics.c:\$line_callee3_body" + "444\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",${fullname},line=\"$line_callee3_body\",times=\"0\"\}" \ + "insert temp breakpoint at basics.c:\$line_callee3_head" # Getting the quoting right is tricky. That is "\"\":$line_callee4_head" mi_gdb_test "555-break-insert -t \"\\\"${srcfile}\\\":$line_callee4_head\"" \ - "555\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",${fullname},line=\"$line_callee4_head\",times=\"0\"\}" \ + "555\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"del\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",${fullname},line=\"$line_callee4_body\",times=\"0\"\}" \ "insert temp breakpoint at \"\":\$line_callee4_head" mi_gdb_test "666-break-list" \ Index: gdb.mi/mi2-simplerun.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-simplerun.exp,v retrieving revision 1.8 diff -u -p -r1.8 mi2-simplerun.exp --- gdb.mi/mi2-simplerun.exp 1 Jan 2008 22:53:20 -0000 1.8 +++ gdb.mi/mi2-simplerun.exp 2 Feb 2008 01:09:34 -0000 @@ -52,7 +52,9 @@ proc test_breakpoints_creation_and_listi global hex set line_callee4_head [gdb_get_line_number "callee4 ("] + set line_callee4_body [expr $line_callee4_head + 2] set line_callee3_head [gdb_get_line_number "callee3 ("] + set line_callee3_body [expr $line_callee3_head + 2] set line_callee2_head [gdb_get_line_number "callee2 ("] set line_callee2_body [expr $line_callee2_head + 2] set line_main_head [gdb_get_line_number "main ("] @@ -75,11 +77,11 @@ proc test_breakpoints_creation_and_listi "insert breakpoint at basics.c:callee2" mi_gdb_test "202-break-insert basics.c:$line_callee3_head" \ - "202\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",line=\"$line_callee3_head\",times=\"0\"\}" \ + "202\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee3\",file=\".*basics.c\",line=\"$line_callee3_body\",times=\"0\"\}" \ "insert breakpoint at basics.c:\$line_callee3_head" mi_gdb_test "203-break-insert \"\\\"${srcfile}\\\":$line_callee4_head\"" \ - "203\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"$line_callee4_head\",times=\"0\"\}" \ + "203\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",line=\"$line_callee4_body\",times=\"0\"\}" \ "insert breakpoint at \"\":\$line_callee4_head" mi_gdb_test "204-break-list" \ --Q68bSM7Ycu6FN28Q--