* RFC: preserve line number when skipping prologue
@ 2009-03-26 21:06 Tom Tromey
2009-03-26 23:10 ` Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2009-03-26 21:06 UTC (permalink / raw)
To: gdb-patches
Jeff Johnston noticed a regression in the CDT test suite when using a
recent gdb. The bug is that if you put a breakpoint on a
non-executable line before a function's start, gdb displays the
breakpoint as being on the function's first line.
E.g. consider this program:
#include <stdio.h>
/* Line 3 */
int main()
{
printf ("hi\n");
/* Line 9 */
return 0;
}
If I set a breakpoint on line 3, gdb reports line 7.
(gdb) b 3
Breakpoint 1 at 0x80483c5: file m.c, line 7.
But if I set one on line 9, another non-executable line, gdb reports
line 9:
(gdb) b 9
Breakpoint 2 at 0x80483d1: file m.c, line 9.
I tracked this down to the prologue-skipping code.
This patch fixes the problem by preserving the line number when
skipping the prologue. I am not sure this is really a valid thing to
do -- I'd appreciate comments on that. However, I do think that the
principle of reporting the location that the user requested is a valid
one.
Built and regtested on x86-64 (compile farm).
Tom
2009-03-26 Tom Tromey <tromey@redhat.com>
* breakpoint.c (resolve_sal_pc): Preserve original line number
when skipping prologue.
2009-03-26 Tom Tromey <tromey@redhat.com>
Update for change to prologue skipping:
* gdb.mi/mi2-simplerun.exp: Update.
* gdb.mi/mi2-break.exp: Update.
* gdb.mi/mi-simplerun.exp: Update.
* gdb.mi/mi-break.exp: Update.
* gdb.base/ending-run.exp: Update.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7e50342..750d632 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5754,7 +5754,12 @@ resolve_sal_pc (struct symtab_and_line *sal)
/* 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);
+ {
+ /* Preserve the original line number. */
+ int saved_line = sal->line;
+ skip_prologue_sal (sal);
+ sal->line = saved_line;
+ }
}
if (sal->section == 0 && sal->symtab != NULL)
diff --git a/gdb/testsuite/gdb.base/ending-run.exp b/gdb/testsuite/gdb.base/ending-run.exp
index 42b4577..cf33602 100644
--- a/gdb/testsuite/gdb.base/ending-run.exp
+++ b/gdb/testsuite/gdb.base/ending-run.exp
@@ -69,7 +69,7 @@ gdb_expect {
gdb_test "b ending-run.c:1" ".*Breakpoint.*4.*"
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"
+ ".*Deleted breakpoint 5.*" "Cleared 2 by line"
send_gdb "inf line ending-run.c:14\n"
gdb_expect {
@@ -77,7 +77,7 @@ gdb_expect {
set line_nine $expect_out(1,string)
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"
+ gdb_test "cle" ".*Deleted breakpoints 4 6 7.*" "Clear 2 by default"
}
-re ".*$gdb_prompt $" {
fail "need to fix test for new compile outcome"
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 6ea59fc..0f91af7 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -85,11 +85,11 @@ proc test_tbreak_creation_and_listing {} {
mi_create_breakpoint "-t basics.c:callee2" 2 del callee2 ".*basics.c" $line_callee2_body $hex \
"insert temp breakpoint at basics.c:callee2"
- mi_create_breakpoint "-t basics.c:$line_callee3_head" 3 del callee3 ".*basics.c" $line_callee3_body $hex \
+ mi_create_breakpoint "-t basics.c:$line_callee3_head" 3 del callee3 ".*basics.c" $line_callee3_head $hex \
"insert temp breakpoint at basics.c:\$line_callee3_head"
# Getting the quoting right is tricky. That is "\"<file>\":$line_callee4_head"
- mi_create_breakpoint "-t \"\\\"${srcfile}\\\":$line_callee4_head\"" 4 del callee4 ".*basics.c" $line_callee4_body $hex \
+ mi_create_breakpoint "-t \"\\\"${srcfile}\\\":$line_callee4_head\"" 4 del callee4 ".*basics.c" $line_callee4_head $hex \
"insert temp breakpoint at \"<fullfilename>\":\$line_callee4_head"
mi_gdb_test "666-break-list" \
diff --git a/gdb/testsuite/gdb.mi/mi-simplerun.exp b/gdb/testsuite/gdb.mi/mi-simplerun.exp
index 654146a..14a1446 100644
--- a/gdb/testsuite/gdb.mi/mi-simplerun.exp
+++ b/gdb/testsuite/gdb.mi/mi-simplerun.exp
@@ -74,10 +74,10 @@ proc test_breakpoints_creation_and_listing {} {
mi_create_breakpoint "basics.c:callee2" 2 keep callee2 ".*basics.c" $line_callee2_body $hex \
"insert breakpoint at basics.c:callee2"
- mi_create_breakpoint "basics.c:$line_callee3_head" 3 keep callee3 ".*basics.c" $line_callee3_body $hex \
+ mi_create_breakpoint "basics.c:$line_callee3_head" 3 keep callee3 ".*basics.c" $line_callee3_head $hex \
"insert breakpoint at basics.c:\$line_callee3_head"
- mi_create_breakpoint "\"\\\"${srcfile}\\\":$line_callee4_head\"" 4 keep callee4 ".*basics.c" $line_callee4_body $hex \
+ mi_create_breakpoint "\"\\\"${srcfile}\\\":$line_callee4_head\"" 4 keep callee4 ".*basics.c" $line_callee4_head $hex \
"insert breakpoint at \"<fullfilename>\":\$line_callee4_head"
mi_gdb_test "204-break-list" \
diff --git a/gdb/testsuite/gdb.mi/mi2-break.exp b/gdb/testsuite/gdb.mi/mi2-break.exp
index d08081a..7f43315 100644
--- a/gdb/testsuite/gdb.mi/mi2-break.exp
+++ b/gdb/testsuite/gdb.mi/mi2-break.exp
@@ -85,11 +85,11 @@ proc test_tbreak_creation_and_listing {} {
mi_create_breakpoint "-t basics.c:callee2" 2 del callee2 ".*basics.c" $line_callee2_body $hex \
"insert temp breakpoint at basics.c:callee2"
- mi_create_breakpoint "-t basics.c:$line_callee3_head" 3 del callee3 ".*basics.c" $line_callee3_body $hex \
+ mi_create_breakpoint "-t basics.c:$line_callee3_head" 3 del callee3 ".*basics.c" $line_callee3_head $hex \
"insert temp breakpoint at basics.c:\$line_callee3_head"
# Getting the quoting right is tricky. That is "\"<file>\":$line_callee4_head"
- mi_create_breakpoint "-t \"\\\"${srcfile}\\\":$line_callee4_head\"" 4 del callee4 ".*basics.c" $line_callee4_body $hex \
+ mi_create_breakpoint "-t \"\\\"${srcfile}\\\":$line_callee4_head\"" 4 del callee4 ".*basics.c" $line_callee4_head $hex \
"insert temp breakpoint at \"<fullfilename>\":\$line_callee4_head"
mi_gdb_test "666-break-list" \
diff --git a/gdb/testsuite/gdb.mi/mi2-simplerun.exp b/gdb/testsuite/gdb.mi/mi2-simplerun.exp
index eddb2ed..13443ee 100644
--- a/gdb/testsuite/gdb.mi/mi2-simplerun.exp
+++ b/gdb/testsuite/gdb.mi/mi2-simplerun.exp
@@ -74,10 +74,10 @@ proc test_breakpoints_creation_and_listing {} {
mi_create_breakpoint "basics.c:callee2" 2 keep callee2 ".*basics.c" $line_callee2_body $hex \
"insert breakpoint at basics.c:callee2"
- mi_create_breakpoint "basics.c:$line_callee3_head" 3 keep callee3 ".*basics.c" $line_callee3_body $hex \
+ mi_create_breakpoint "basics.c:$line_callee3_head" 3 keep callee3 ".*basics.c" $line_callee3_head $hex \
"insert breakpoint at basics.c:\$line_callee3_head"
- mi_create_breakpoint "\"\\\"${srcfile}\\\":$line_callee4_head\"" 4 keep callee4 ".*basics.c" $line_callee4_body $hex \
+ mi_create_breakpoint "\"\\\"${srcfile}\\\":$line_callee4_head\"" 4 keep callee4 ".*basics.c" $line_callee4_head $hex \
"insert breakpoint at \"<fullfilename>\":\$line_callee4_head"
mi_gdb_test "204-break-list" \
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: preserve line number when skipping prologue
2009-03-26 21:06 RFC: preserve line number when skipping prologue Tom Tromey
@ 2009-03-26 23:10 ` Joel Brobecker
2009-03-26 23:45 ` Tom Tromey
2009-03-27 2:19 ` Daniel Jacobowitz
0 siblings, 2 replies; 6+ messages in thread
From: Joel Brobecker @ 2009-03-26 23:10 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> If I set a breakpoint on line 3, gdb reports line 7.
>
> (gdb) b 3
> Breakpoint 1 at 0x80483c5: file m.c, line 7.
>
> But if I set one on line 9, another non-executable line, gdb reports
> line 9:
>
> (gdb) b 9
> Breakpoint 2 at 0x80483d1: file m.c, line 9.
I agree that we need to be consistent between the two cases!
I don't know which one I prefer, though. Actually, I think I would
prefer if GDB reported the real line on which it was inserted. But
that would be a change of behavior from before, and that could
have ramifications that could potentially annoys the users
(on the "clear" command, for instance).
> 2009-03-26 Tom Tromey <tromey@redhat.com>
>
> * breakpoint.c (resolve_sal_pc): Preserve original line number
> when skipping prologue.
So I think that your patch is the most reasonable approach
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: preserve line number when skipping prologue
2009-03-26 23:10 ` Joel Brobecker
@ 2009-03-26 23:45 ` Tom Tromey
2009-03-27 0:46 ` Joel Brobecker
2009-03-27 2:19 ` Daniel Jacobowitz
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2009-03-26 23:45 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
Tom> (gdb) b 3
Tom> Breakpoint 1 at 0x80483c5: file m.c, line 7.
Tom> (gdb) b 9
Tom> Breakpoint 2 at 0x80483d1: file m.c, line 9.
Joel> I agree that we need to be consistent between the two cases!
Joel> I don't know which one I prefer, though. Actually, I think I would
Joel> prefer if GDB reported the real line on which it was inserted. But
Joel> that would be a change of behavior from before, and that could
Joel> have ramifications that could potentially annoys the users
Joel> (on the "clear" command, for instance).
Yeah, I think there are reasons to want both.
On the one hand, the line number you asked for is used if the
breakpoint ever needs to be reset. So, it is relevant.
On the other hand, seeing where the breakpoint actually ends up is
also useful, because that tells you what is actually happening at the
moment.
Perhaps we could emit:
(gdb) b 3
Breakpoint 1 at 0x80483c5: file m.c, line 3.
(Line 3 is not executable; actually set at line 7.)
This is probably not relevant for GUIs.
Joel> So I think that your patch is the most reasonable approach
Ok, I'm going to check it in. Thanks for looking at this.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: preserve line number when skipping prologue
2009-03-26 23:45 ` Tom Tromey
@ 2009-03-27 0:46 ` Joel Brobecker
0 siblings, 0 replies; 6+ messages in thread
From: Joel Brobecker @ 2009-03-27 0:46 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> On the one hand, the line number you asked for is used if the
> breakpoint ever needs to be reset. So, it is relevant.
Yes, that's true too.
> (gdb) b 3
> Breakpoint 1 at 0x80483c5: file m.c, line 3.
> (Line 3 is not executable; actually set at line 7.)
I suppose we could. But we've used the current behavior for years,
and I don't feel sufficiently strongly about it that I want to
improve it (yep, i'm lazy :).
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: preserve line number when skipping prologue
2009-03-26 23:10 ` Joel Brobecker
2009-03-26 23:45 ` Tom Tromey
@ 2009-03-27 2:19 ` Daniel Jacobowitz
2009-03-27 16:00 ` Joel Brobecker
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2009-03-27 2:19 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches
On Thu, Mar 26, 2009 at 03:49:49PM -0700, Joel Brobecker wrote:
> I don't know which one I prefer, though. Actually, I think I would
> prefer if GDB reported the real line on which it was inserted. But
> that would be a change of behavior from before, and that could
> have ramifications that could potentially annoys the users
> (on the "clear" command, for instance).
FWIW, a (related) request we often see is for IDEs to report which
lines can have breakpoints set on them. Still not foolproof with
optimized code, but much better. Eclipse JDT (Java) does this, and I
believe at least one vendor has managed to get CDT to do it also.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFC: preserve line number when skipping prologue
2009-03-27 2:19 ` Daniel Jacobowitz
@ 2009-03-27 16:00 ` Joel Brobecker
0 siblings, 0 replies; 6+ messages in thread
From: Joel Brobecker @ 2009-03-27 16:00 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
> FWIW, a (related) request we often see is for IDEs to report which
> lines can have breakpoints set on them. Still not foolproof with
> optimized code, but much better. Eclipse JDT (Java) does this, and I
> believe at least one vendor has managed to get CDT to do it also.
I guess the way they do it is by doing a reverse lookup after having
inserted the breakpoint (by using the "info line *ADDR" or its MI
equivalent)? For IDEs, we could perhaps enhance the output to include
the actual line if different from the requested one.
Or, we could decide to change the break command to report the
actual line. I can poll the AdaCore engineers :).
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-03-27 15:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-26 21:06 RFC: preserve line number when skipping prologue Tom Tromey
2009-03-26 23:10 ` Joel Brobecker
2009-03-26 23:45 ` Tom Tromey
2009-03-27 0:46 ` Joel Brobecker
2009-03-27 2:19 ` Daniel Jacobowitz
2009-03-27 16:00 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox