This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Make language setting tests more robust
On 02/01/2017 12:38 PM, Simon Marchi wrote:
On 2017-02-01 10:17, Luis Machado wrote:
This patch robustifies the setting of the language in gdb. Right now
most of
the tests assume "set language" is a silent command and use
gdb_test_no_output
for testing, but it may output a warning stating the language the user
wants
to select does not match the frame's language. When this happens we
have a
failure. This is also the case with "show language", where it outputs the
language currently-selected but may also output said warning.
One case of the warning being displayed happens when one has debug
information
for glibc, which may cause GDB to identify the frame as having an "asm"
language. Therefore setting it to something else will get GDB's
attention.
This patch addresses the problem by creating a function in lib/gdb.exp to
set the language. That function will also handle potential warnings
and check
to make sure the language was properly selected.
Also, i noticed most of the languages have their own
set_lang_<language> proc,
and they are all the same. Therefore i've removed those and switched
to using
only the new set_language proc.
I tried to confirm why set_lang_<language> was replicated, but my
conclusion
was that it was just the way the code worked and people just
copy/pasted from
an existing language .exp file.
Overall i see no regressions with Ubuntu 16.04 x86-64, though the
number of
tests changed slightly due to the way set_language works. No additional
failures nonetheless.
I have just looked at the stuff in lib/, it looks good to me, that's a
nice cleanup. A few comments below.
diff --git a/gdb/testsuite/lib/d-support.exp
b/gdb/testsuite/lib/d-support.exp
index ed568d3..100fb63 100644
--- a/gdb/testsuite/lib/d-support.exp
+++ b/gdb/testsuite/lib/d-support.exp
@@ -16,17 +16,6 @@
# Auxiliary function to set the language to D.
# The result is 1 (true) for success, 0 (false) for failure.
Remove this comment.
Oops. I was chasing some small issues with the set_language proc and
ended up forgetting the cleanup these files.
Thanks for catching that.
+# Set the language and handle possible warnings output by GDB if we
select a
+# language that differs from the current frame's language.
+#
+# The first argument is the language to set.
+# The second argument is an optional message to be output by the test.
+
+proc set_language { args } {
Instead of "args", it might be clearer to use a default argument for the
second arg:
proc set_language { language { message "" } } {
And lower:
if { $message == "" } {
set message $command
}
Not sure about the exact TCL syntax, but I think you get the point :).
I'm fine with that. I was mostly making it work like gdb_test_no_output.
I'll get this addressed in the next iteration.
+ global gdb_prompt
+
+ set language [lindex $args 0]
+ set command "set language $language"
+ set lang_pattern [string_to_regexp $language]
+
+ # Do we have an optional user-provided test message?
+ if {[llength $args] > 1} {
+ set message [lindex $args 1]
+ } else {
+ set message $command
+ }
+
+ # Switch to the user-selected language.
+ gdb_test_multiple $command $message {
+ -re "Undefined item: \"$lang_pattern\"\.\[\r\n\]+$gdb_prompt" {
+ fail $message
+ return 0
+ }
+ -re "Warning: the current language does not match this
frame.\[\r\n\]+$gdb_prompt $" {
+ }
+ -re "$gdb_prompt $" {}
+ }
Do you want to add a "pass" in there somewhere, or we keep it silent if
it succeeds?
The way the code works at the moment will give a PASS if we set the
language and then verify it has been set correctly. If it is not set
correctly, we get a FAIL.
+
+ # Verify the language has been set correctly. GDB may output a
warning
+ # stating the user-provided language doesn't match the frame's
language.
+ # We ignore that warning for testing purposes.
+ if {$language != "auto"} {
+ if [gdb_test "show language" ".* source language is
\"$lang_pattern\".*" $message] {
+ return 0
+ }
+ }
In the else, you could verify that it matches 'The current source
language is "auto; currently .*"'.
Yeah, that sounds reasonable. I'll address this.
+ return 1
+}
# Always load compatibility stuff.
load_lib future.exp
diff --git a/gdb/testsuite/lib/go.exp b/gdb/testsuite/lib/go.exp
index c29b7c1..cb98bdc 100644
--- a/gdb/testsuite/lib/go.exp
+++ b/gdb/testsuite/lib/go.exp
@@ -19,17 +19,6 @@
# Auxiliary function to set the language to Go.
# The result is 1 (true) for success, 0 (false) for failure.
Comment.
-proc set_lang_go {} {
- if [gdb_test_no_output "set language go"] {
- return 0
- }
- if [gdb_test "show language" ".* source language is \"go\"." \
- "set language to \"go\""] {
- return 0
- }
- return 1
-}
-
# Go version of runto_main.
proc go_runto_main { } {
diff --git a/gdb/testsuite/lib/objc.exp b/gdb/testsuite/lib/objc.exp
index 7189f03..9891758 100644
--- a/gdb/testsuite/lib/objc.exp
+++ b/gdb/testsuite/lib/objc.exp
@@ -17,14 +17,3 @@
# Auxiliary function to set the language to fortran.
# The result is 1 (true) for success, 0 (false) for failure.
Comment. Also, note that it mentions fortran :). Actually, you could
probably just delete that file.
I considered just leaving it there, but with just the copyright.
Deleting may be cleaner though. What others think?
-
-proc set_lang_objc {} {
- if [gdb_test_no_output "set language objective-c"] {
- return 0
- }
- if [gdb_test "show language" ".* source language is
\"objective-c\"." \
- "set language to \"objective-c\""] {
- return 0
- }
- return 1
-}
diff --git a/gdb/testsuite/lib/pascal.exp b/gdb/testsuite/lib/pascal.exp
index a0562c3..f0eb36b 100644
--- a/gdb/testsuite/lib/pascal.exp
+++ b/gdb/testsuite/lib/pascal.exp
@@ -167,17 +167,3 @@ proc gdb_compile_pascal {source destfile type
options} {
return "Pascal compilation failed."
}
}
-
-# Auxiliary function to set the language to pascal.
-# The result is 1 (true) for success, 0 (false) for failure.
-
-proc set_lang_pascal {} {
- if [gdb_test_no_output "set language pascal"] {
- return 0
- }
- if [gdb_test "show language" ".* source language is \"pascal\"." \
- "set language to \"pascal\""] {
- return 0
- }
- return 1
-}
diff --git a/gdb/testsuite/lib/rust-support.exp
b/gdb/testsuite/lib/rust-support.exp
index 2a12cca..a80d60b 100644
--- a/gdb/testsuite/lib/rust-support.exp
+++ b/gdb/testsuite/lib/rust-support.exp
@@ -15,16 +15,6 @@
# Auxiliary function to set the language to Rust.
# The result is 1 (true) for success, 0 (false) for failure.
Comment.
-proc set_lang_rust {} {
- if [gdb_test_no_output "set language rust"] {
- return 0
- }
- if [gdb_test "show language" ".* source language is \"rust\"." \
- "set language to \"rust\""] {
- return 0
- }
- return 1
-}
proc gdb_compile_rust {sources dest options} {
if {[llength $sources] > 1} {
Thanks for the review.