[RFA/linespec] wrong line number in breakpoint location
Commit Message
> Thanks, this is fine with me. Just a really small nit, I would suggest
> initializing the line_actual variable to 0 or -1 (an invalid line number)
> prior to calling gdb_test_multiple. This way, if that test fails,
> line_actual will still be defined, and the expression that refers to it will
> generate a FAIL instead of an unreadable tcl backtrace.
Sounds good. Just for kicks, I took at look at what it looks like
when the variable is undefined, and the error message is obvious
about why it fails. When the error is defined, however, you have
to figure out what the difference is, and track the value of that
variable down. What won me over to your suggestion is that the
error can go unnoticed if you just compare .sum files...
Attached is the patch I just pushed (re-tested on x86_64-linux).
Comments
On 01/30/2018 04:09 AM, Joel Brobecker wrote:
>> Thanks, this is fine with me. Just a really small nit, I would suggest
>> initializing the line_actual variable to 0 or -1 (an invalid line number)
>> prior to calling gdb_test_multiple. This way, if that test fails,
>> line_actual will still be defined, and the expression that refers to it will
>> generate a FAIL instead of an unreadable tcl backtrace.
>
> Sounds good. Just for kicks, I took at look at what it looks like
> when the variable is undefined, and the error message is obvious
> about why it fails. When the error is defined, however, you have
> to figure out what the difference is, and track the value of that
> variable down. What won me over to your suggestion is that the
> error can go unnoticed if you just compare .sum files...
My view is that since "using" an undefined variable results in
a TCL error, that is a testsuite bug, plain and simple.
A TCL error is lower level than a dejagnu FAIL, and aborts the
whole testcase, while a dejagnu FAIL indicates that the testing
harness is working and caught GDB behaving unexpectedly in the
particular use case being tested. A FAIL can go on and run
subsequent procedures/tests.
In cases like this where we're extracting variables with expect_out,
we often add failed-to-extract-variable handling after
the gdb_test_multiple, like this:
# start with "impossible" value.
set whatever -1
gdb_test_multiple $test $test {
-re "whatever is ($decimal)\r\n$gdb_prompt $" {
set whatever $expect_out(1,string)
pass $test
}
}
if {$whatever == -1} {
# bail out, no use continuing the procedure.
return
}
So you get a FAIL in the gdb_test_multiple case, and skip
running the rest of the procedure. Sometimes we call "untested"
before returning. It's a bit easier to see usefulness of the
pattern if the tests _are_ put in a procedure called by a
main testcase driver, like:
~~~~~~~~~~~~~~~~~~~~~~~~~~~
proc_with_prefix test_whatever {} {
set whatever -1
gdb_test_multiple $test $test {
-re "whatever is ($decimal)\r\n$gdb_prompt $" {
set whatever $expect_out(1,string)
pass $test
}
}
if {$whatever == -1} {
# bail out, no use continuing the procedure.
return
}
}
proc_with_prefix test_whatever_else {} {
...
}
# The drive code that calls test procedures.
test_whatever
test_whatever_else
~~~~~~~~~~~~~~~~~~~~~~~~~~~
And now test_whatever_else runs even if test_whatever bails out.
Thanks,
Pedro Alves
From fc413dc467e4c46013f6e5a60dc5db24d63f72ea Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 29 Jan 2018 23:03:09 -0500
Subject: [PATCH] gdb.base/break.exp: fix last "info break" test failure on
Ubuntu 16.04
The last test of this testcase fails when run on Ubuntu 16.04 using
the system compiler (16.04):
FAIL: gdb.base/break.exp: verify that they were cleared
This is because the testcase expected that a breakpoint on line 47 of break.c...
printf ("%d\n", factorial (atoi ("6"))); /* set breakpoint 1 here */
... would actually be inserted on an instruction belonging to
that line. However, what actually happens is that system GCC on
that version of Ubuntu ends up inlining everything, including
the call to printf, thus reporting every instruction of generated
for this line of code as belonging to a different function. As
a result, GDB ends up insering the breakpoint on the next line
of code, which is line 49:
(gdb) break break.c:$l
Breakpoint 3 at 0x4005c1: file /[...]/gdb.base/break.c, line 49.
This causes a spurious failure in the "info break" test later on,
as it assumed that the breakpoint above is inserted on line 47:
gdb_test "info break" "$srcfile:$line" "verify that they were cleared"
This patch fixes the issue by saving the actual source location where
the breakpoint was inserted.
gdb/testsuite/ChangeLog:
* gdb.base/break.exp: Save the location where the breakpoint
on break.c:47 was actually inserted when debugging the version
compiled at -O2 and use it in the expected output of the "info
break" test performed soon after.
tested on x86_64-linux, with two configurations:
- Ubuntu 16.04 with the system compiler (breakpoint lands on line 49)
- Ubuntu 16.04 with GCC 7.3.1 (breakpoint lands on line 47)
---
gdb/testsuite/ChangeLog | 7 +++++++
gdb/testsuite/gdb.base/break.exp | 17 +++++++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
@@ -1,3 +1,10 @@
+2018-01-30 Joel Brobecker <brobecker@adacore.com>
+
+ * gdb.base/break.exp: Save the location where the breakpoint
+ on break.c:47 was actually inserted when debugging the version
+ compiled at -O2 and use it in the expected output of the "info
+ break" test performed soon after.
+
2018-01-22 Pedro Alves <palves@redhat.com>
Sergio Durigan Junior <sergiodj@redhat.com>
@@ -847,7 +847,20 @@ gdb_test_multiple "" $test {
#
set line [gdb_get_line_number "set breakpoint 1 here"]
gdb_test_no_output "set \$l = $line"
-gdb_breakpoint ${srcfile}:\$l
+
+set line_actual "-1"
+set test "break ${srcfile}:\$l"
+gdb_test_multiple "$test" $test {
+ -re "Breakpoint $decimal at $hex: file .*break\\.c, line ($decimal)\\.\r\n$gdb_prompt $" {
+ # Save the actual line number on which the breakpoint was
+ # actually set. On some systems (Eg: Ubuntu 16.04 with GCC
+ # version 5.4.0), that line gets completely inlined, including
+ # the call to printf, and so we end up inserting the breakpoint
+ # on one of the following lines instead.
+ set line_actual $expect_out(1,string)
+ pass $test
+ }
+}
gdb_test_no_output "set \$foo=81.5" \
"set convenience variable \$foo to 81.5"
@@ -865,4 +878,4 @@ gdb_test "commands\nend" ">end" "clear breakpoint commands"
# We verify that the commands were cleared by ensuring that the last
# breakpoint's location ends the output -- if there were commands,
# they would have been printed after the location.
-gdb_test "info break" "$srcfile:$line" "verify that they were cleared"
+gdb_test "info break" "$srcfile:$line_actual" "verify that they were cleared"
--
2.1.4