From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20544 invoked by alias); 1 Feb 2017 18:38:29 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 20433 invoked by uid 89); 1 Feb 2017 18:38:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 01 Feb 2017 18:38:25 +0000 Received: by simark.ca (Postfix, from userid 33) id DD69D1E805; Wed, 1 Feb 2017 13:38:23 -0500 (EST) To: Luis Machado Subject: Re: [PATCH] Make language setting tests more robust X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Wed, 01 Feb 2017 18:38:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <1485962220-31071-1-git-send-email-lgustavo@codesourcery.com> References: <1485962220-31071-1-git-send-email-lgustavo@codesourcery.com> Message-ID: <331062ebcc7352d7731347260b56dee7@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.3 X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00038.txt.bz2 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_ 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_ 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. > -proc set_lang_d {} { > - if [gdb_test_no_output "set language d"] { > - return 0 > - } > - if [gdb_test "show language" ".* source language is \"d\"." \ > - "set language to \"d\""] { > - return 0 > - } > - return 1 > -} > - > # D version of runto_main. > > proc d_runto_main { } { > diff --git a/gdb/testsuite/lib/fortran.exp > b/gdb/testsuite/lib/fortran.exp > index 867e4fc..2f4820b 100644 > --- a/gdb/testsuite/lib/fortran.exp > +++ b/gdb/testsuite/lib/fortran.exp > @@ -18,17 +18,6 @@ > # Auxiliary function to set the language to fortran. > # The result is 1 (true) for success, 0 (false) for failure. Comment. > -proc set_lang_fortran {} { > - if [gdb_test_no_output "set language fortran"] { > - return 0 > - } > - if [gdb_test "show language" ".* source language is \"fortran\"." > \ > - "set language to \"fortran\""] { > - return 0 > - } > - return 1 > -} > - > proc fortran_int4 {} { > if {[test_compiler_info {gcc-4-[012]-*}]} { > return "int4" > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 48bd7ee..327bce8 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -6018,6 +6018,47 @@ proc multi_line_input { args } { > return [join $args "\n"] > } > > +# 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 :). > + 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? > + > + # 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 .*"'. > + 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. > - > -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} {