Re: [RFC/RFA?] Should break FILE:LINENO skip prologue?

Joel Brobecker
2008-02-02T01:20:11Z

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

Hi Daniel,

> > 2008-01-09 Joel Brobecker <brobecker@adacore.com>
> >
> > * 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 <brobecker@adacore.com>

* 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 <brobecker@adacore.com>

* 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

[-- Attachment #2: skip-prologue.diff --]
[-- Type: text/plain, Size: 1275 bytes --]

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)

[-- Attachment #3: skip-prologue-tc.diff --]
[-- Type: text/plain, Size: 10318 bytes --]

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 "\"<file>\":$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 \"<fullfilename>\":\$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 \"<fullfilename>\":\$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 "\"<file>\":$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 \"<fullfilename>\":\$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 \"<fullfilename>\":\$line_callee4_head"

mi_gdb_test "204-break-list" \