[gdb/testsuite] Restore breakpoint command in ui-redirect.exp
Commit Message
[ was: Re: [RFC][gdb/testsuite] Remove breakpoint command in
gdb.base/ui-redirect.exp ]
On 04-09-19 13:48, Jan Kratochvil wrote:
> On Tue, 03 Sep 2019 19:43:44 +0200, Tom de Vries wrote:
>> I noticed a bit in gdb.base/ui-redirect.exp that sets a breakpoint
>> command on main, but then due to runto_main, the break point is deleted:
> ...
>> In the original commit ( submission here
>> https://sourceware.org/ml/gdb-patches/2010-09/msg00120.html ) there was
>> no runto_main, and the breakpoint command was not triggered either.
>>
>> Is this some artefact, and can it be removed, or is it actually testing
>> something related to redirection?
>
> It looks to me the patch which added the "runto_main" part:
>
> commit ca1285d17534cff3041c07ac7841288e1b3ba19c
> Author: Alan Hayward <alan.hayward@arm.com>
> Date: Fri May 17 14:15:01 2019 +0100
> Add debug redirect option
>
> disabled the regression testing.
>
> "runto_main" should be removed otherwise the testcase does not test anything
> (or it just tests less).
Attached patch fixes the problem by replacing the initial
"gdb_breakpoint main" with the runto_main, such that the breakpoint
command is preserved throughout.
OK for trunk?
Thanks,
- Tom
Comments
On Thu, 05 Sep 2019 09:34:57 +0200, Tom de Vries wrote:
> Attached patch fixes the problem by replacing the initial
> "gdb_breakpoint main" with the runto_main, such that the breakpoint
> command is preserved throughout.
Yes, the test is now much better than even was the initial one by me.
> OK for trunk?
I cannot approve GDB patches but LGTM.
Thanks,
Jan
On 05-09-19 14:03, Jan Kratochvil wrote:
> On Thu, 05 Sep 2019 09:34:57 +0200, Tom de Vries wrote:
>> Attached patch fixes the problem by replacing the initial
>> "gdb_breakpoint main" with the runto_main, such that the breakpoint
>> command is preserved throughout.
>
> Yes, the test is now much better than even was the initial one by me.
>
>
>> OK for trunk?
>
> I cannot approve GDB patches but LGTM.
>
Thanks. I would at this point commit the patch under the obvious rule,
if it didn't introduce a new proc cmp_file_string in lib/gdb.exp.
Thanks,
- Tom
On 9/5/19 8:34 AM, Tom de Vries wrote:
> +set cmds \
> + [list \
> + "break main" \
> + " commands" \
> + " print 1" \
> + " end" \
> + "break foo" \
> + "break bar"]
> +set cmds [join $cmds "\n"]
Use multi_line_input ?
> +# Compare contents of FILE to string STR. Pass with MSG if equal, otherwise
> +# fail with MSG.
Double space after period.
Thanks,
Pedro Alves
On 05-09-19 14:19, Pedro Alves wrote:
> On 9/5/19 8:34 AM, Tom de Vries wrote:
>
>> +set cmds \
>> + [list \
>> + "break main" \
>> + " commands" \
>> + " print 1" \
>> + " end" \
>> + "break foo" \
>> + "break bar"]
>> +set cmds [join $cmds "\n"]
>
> Use multi_line_input ?
>
>> +# Compare contents of FILE to string STR. Pass with MSG if equal, otherwise
>> +# fail with MSG.
>
> Double space after period.
>
Thanks for the review.
Committed with these two review remarks fixed.
Thanks,
- Tom
[gdb/testsuite] Restore breakpoint command in ui-redirect.exp
In gdb.base/ui-redirect.exp, the "save breakpoint" command is used to write
the current breakpoints to a file, but the actual output is not verified.
Consequently, the test has regressed in that the "print 1" command associated
with a breakpoint on main is removed by a subsequent runto_main, which first
deletes all breakpoints:
...
(gdb) break main
Breakpoint 1 at 0x4004d7: file start.c, line 34.
(gdb) commands
Type commands for breakpoint(s) 1, one per line.
End with a line saying just "end".
> PASS: gdb.base/ui-redirect.exp: commands
print 1
> PASS: gdb.base/ui-redirect.exp: print 1
end
(gdb) PASS: gdb.base/ui-redirect.exp: end
delete breakpoints
Delete all breakpoints? (y or n) y
...
and consequently the "save breakpoint" output is missing the breakpoint
command for main:
...
break main
- commands
- print 1
- end
break foo
break bar
...
Fix this by replacing "gdb_breakpoint main" with runto_main, and verifying the
"save breakpoints" output.
Tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2019-09-05 Tom de Vries <tdevries@suse.de>
* lib/gdb.exp (cmp_file_string): New proc.
* gdb.base/ui-redirect.exp: Replace "gdb_breakpoint main" with
runto_main. Verify save breakpoints output.
---
gdb/testsuite/gdb.base/ui-redirect.exp | 33 +++++++++++++++++++++++++--------
gdb/testsuite/lib/gdb.exp | 27 +++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 8 deletions(-)
@@ -23,7 +23,10 @@ if { [prepare_for_testing "failed to prepare" ui-redirect start.c] } {
return -1
}
-gdb_breakpoint main
+if ![runto_main] {
+ fail "can't run to main"
+ return -1
+}
set test "commands"
gdb_test_multiple $test $test {
@@ -40,18 +43,29 @@ gdb_test_multiple $test $test {
}
gdb_test_no_output "end"
-if ![runto_main] {
- fail "can't run to main"
- return -1
-}
gdb_breakpoint "foo"
gdb_breakpoint "bar"
+set cmds \
+ [list \
+ "break main" \
+ " commands" \
+ " print 1" \
+ " end" \
+ "break foo" \
+ "break bar"]
+set cmds [join $cmds "\n"]
+set cmds "$cmds\n"
+set outdir [standard_output_file {}]
+set cmds_file "$outdir/cmds.txt"
+
with_test_prefix "logging" {
gdb_test_no_output "set logging file /dev/null"
gdb_test "set logging on" \
"Copying output to /dev/null.*Copying debug output to /dev/null\\."
- gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\."
+ gdb_test "save breakpoints $cmds_file" "Saved to file '$cmds_file'\\." \
+ "save breakpoints cmds.txt"
+ cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
gdb_test "set logging off" "Done logging to /dev/null\\."
gdb_test "help" "List of classes of commands:.*"
}
@@ -60,7 +74,8 @@ with_test_prefix "redirect" {
gdb_test "set logging redirect on"
gdb_test "set logging on" \
"Redirecting output to /dev/null.*Copying debug output to /dev/null\\."
- gdb_test_no_output "save breakpoints /dev/null"
+ gdb_test_no_output "save breakpoints $cmds_file" "save breakpoints cmds.txt"
+ cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
gdb_test "set logging off" "Done logging to /dev/null\\."
gdb_test "help" "List of classes of commands:.*"
}
@@ -71,7 +86,9 @@ with_test_prefix "redirect while already logging" {
"Copying output to /dev/null.*Copying debug output to /dev/null\\."
gdb_test "set logging redirect on" \
".*warning: Currently logging .*Turn the logging off and on to make the new setting effective.*"
- gdb_test "save breakpoints /dev/null" "Saved to file '/dev/null'\\."
+ gdb_test "save breakpoints $cmds_file" "Saved to file '$cmds_file'\\." \
+ "save breakpoints cmds.txt"
+ cmp_file_string "$cmds_file" "$cmds" "cmds.txt"
gdb_test "set logging off" "Done logging to /dev/null\\."
gdb_test "help" "List of classes of commands:.*"
gdb_test_no_output "set logging redirect off"
@@ -6668,5 +6668,32 @@ proc gdb_write_cmd_file { cmdline } {
catch "close $cmd_file"
}
+# Compare contents of FILE to string STR. Pass with MSG if equal, otherwise
+# fail with MSG.
+
+proc cmp_file_string { file str msg } {
+ if { ![file exists $file]} {
+ fail "$msg"
+ return
+ }
+
+ set caught_error [catch {
+ set fp [open "$file" r]
+ set file_contents [read $fp]
+ close $fp
+ } error_message]
+ if { $caught_error } then {
+ error "$error_message"
+ fail "$msg"
+ return
+ }
+
+ if { $file_contents == $str } {
+ pass "$msg"
+ } else {
+ fail "$msg"
+ }
+}
+
# Always load compatibility stuff.
load_lib future.exp