From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 9D8D03952537 for ; Tue, 19 May 2020 16:30:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9D8D03952537 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 7FD35AB91 for ; Tue, 19 May 2020 16:30:10 +0000 (UTC) Date: Tue, 19 May 2020 18:30:06 +0200 From: Tom de Vries To: gdb-patches@sourceware.org Subject: [PATCH 3/3][gdb/testsuite] Warn about leaked global array Message-ID: <20200519163004.GA9045@delia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Status: No, score=-18.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 May 2020 16:30:10 -0000 Hi, A variable name cannot be used both as scalar and array without an intermediate unset. Trying to do so will result in tcl errors, for example, for: ... set var "bla" set var(1) "bla" ... we get: ... can't set "var(1)": variable isn't array ... and for the reverse statement order we get: ... can't set "var": variable is array ... So, since a global name in one test-case can leak to another test-case, setting a global name in one test-case can result in a tcl error in another test-case that reuses the name in a different way. Warn about leaking a global array from a test-case. Also, add a possibility to skip the warning in a given test-case using variable gdb_skip_check_global_vars, and use it in gdb.mi/mi2-var-child.exp and gdb.mi/mi-var-cp.exp. Tested on x86_64-linux. Any comments? Thanks, - Tom [gdb/testsuite] Warn about leaked global array gdb/testsuite/ChangeLog: 2020-05-18 Tom de Vries PR testsuite/25996 * lib/gdb.exp (global_array_exists, global_unset, save_global_vars) (check_global_vars): New proc. (gdb_init): Call save_global_vars. (gdb_finish): Call check_global_vars. * gdb.mi/mi-var-cp.exp: Set gdb_skip_check_global_vars. * gdb.mi/mi2-var-child.exp: Same. --- gdb/testsuite/gdb.mi/mi-var-cp.exp | 3 ++ gdb/testsuite/gdb.mi/mi2-var-child.exp | 3 ++ gdb/testsuite/lib/gdb.exp | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+) diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp index 8c6a624f80..dc32ddc346 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cp.exp +++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp @@ -23,6 +23,9 @@ if [mi_gdb_start] { continue } +# FIXME. See check_global_vars in lib/gdb.exp. +set gdb_skip_check_global_vars 1 + standard_testfile .cc if [get_compiler_info "c++"] { diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp index e32858fbd3..b96ac41815 100644 --- a/gdb/testsuite/gdb.mi/mi2-var-child.exp +++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp @@ -27,6 +27,9 @@ if [mi_gdb_start] { continue } +# FIXME. See check_global_vars in lib/gdb.exp. +set gdb_skip_check_global_vars 1 + standard_testfile var-cmd.c if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index f7d20bd94f..a0d558c943 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -5048,6 +5048,98 @@ proc standard_testfile {args} { } } +# Returns 1 if GLOBALVARNAME is a global array. + +proc global_array_exists { globalvarname } { + # Introduce local alias of global variable $globalvarname. Using + # "global $globalvarname" instead is simpler, but this may result in a + # clash with local name "globalvarname". + upvar #0 $globalvarname globalvar + return [array exists globalvar] +} + +# Unset global variable GLOBALVARNAME. + +proc global_unset { globalvarname } { + # Introduce local alias of global variable $globalvarname. + upvar #0 $globalvarname globalvar + unset globalvar +} + +# Save global vars to variable gdb_global_vars. + +proc save_global_vars { test_file_name } { + # Save for the warning. + global gdb_test_file_name + set gdb_test_file_name $test_file_name + + # Sample state before running test. + global gdb_global_vars + set gdb_global_vars [info globals] + + global gdb_skip_check_global_vars + if { [info exists gdb_skip_check_global_vars]} { + unset gdb_skip_check_global_vars + } +} + +# Check global variables not in gdb_global_vars. + +proc check_global_vars { } { + global gdb_skip_check_global_vars + if { [info exists gdb_skip_check_global_vars]} { + return + } + # Sample state after running test. + global gdb_global_vars + set vars [info globals] + + set skip [list "expect_out" "spawn_out" "auto_index"] + + foreach var $vars { + if { ![global_array_exists $var] } { + continue + } + + set found [lsearch -exact $gdb_global_vars $var] + if { $found != -1 } { + # Already present before running test. + continue + } + + set found [lsearch -exact $skip $var] + if { $found != -1 } { + continue + } + + # A variable name cannot be used both as scalar and array without an + # intermediate unset. Trying to do so will result in tcl errors, for + # example, for: + # set var "bla" + # set var(1) "bla" + # we get: + # can't set "var(1)": variable isn't array + # and for the reverse statement order we get: + # can't set "var": variable is array + # + # So, since a global name in one test-case can leak to another + # test-case, setting a global name in one test-case can result in + # a tcl error in another test-case that reuses the name in a different + # way. + # + # Warn about leaking a global array from the test-case. + # + # For a description on how to fix this, see "Wrapping test-cases in + # Tcl namespaces" in gdb/testsuite/README. + global gdb_test_file_name + warning "$gdb_test_file_name.exp defined global array $var" + + # If the variable remains set, we won't warn for the next test where + # it's leaked, so unset. + global_unset $var + } +} + # The default timeout used when testing GDB commands. We want to use # the same timeout as the default dejagnu timeout, unless the user has # already provided a specific value (probably through a site.exp file). @@ -5177,10 +5269,12 @@ proc gdb_init { test_file_name } { global gdb_instances set gdb_instances 0 + save_global_vars $test_file_name return [default_gdb_init $test_file_name] } proc gdb_finish { } { + check_global_vars global gdbserver_reconnect_p global gdb_prompt global cleanfiles