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 8C4693959E56 for ; Thu, 18 Jun 2020 10:16:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8C4693959E56 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 E125EAB64; Thu, 18 Jun 2020 10:16:42 +0000 (UTC) Subject: [PATCH][gdb/testsuite] Move code from gdb_init to default_gdb_init To: Pedro Alves , gdb-patches@sourceware.org References: <20200611143522.GA19667@delia> <7d7ce523-b58c-a77d-15be-8091feb6389a@redhat.com> <18e36963-4f04-f871-33fb-89a5d1683bbd@suse.de> <31b0b6f2-4930-f339-0245-3a328d0d2548@redhat.com> <80251b07-73db-1ca0-7ab6-67285cb6b1d7@suse.de> <605f6d30-4e18-cebc-517c-5a070e477823@redhat.com> From: Tom de Vries Autocrypt: addr=tdevries@suse.de; keydata= xsBNBF0ltCcBCADDhsUnMMdEXiHFfqJdXeRvgqSEUxLCy/pHek88ALuFnPTICTwkf4g7uSR7 HvOFUoUyu8oP5mNb4VZHy3Xy8KRZGaQuaOHNhZAT1xaVo6kxjswUi3vYgGJhFMiLuIHdApoc u5f7UbV+egYVxmkvVLSqsVD4pUgHeSoAcIlm3blZ1sDKviJCwaHxDQkVmSsGXImaAU+ViJ5l CwkvyiiIifWD2SoOuFexZyZ7RUddLosgsO0npVUYbl6dEMq2a5ijGF6/rBs1m3nAoIgpXk6P TCKlSWVW6OCneTaKM5C387972qREtiArTakRQIpvDJuiR2soGfdeJ6igGA1FZjU+IsM5ABEB AAHNH1RvbSBkZSBWcmllcyA8dGRldnJpZXNAc3VzZS5kZT7CwKsEEwEIAD4WIQSsnSe5hKbL MK1mGmjuhV2rbOJEoAUCXSW0JwIbAwUJA8JnAAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAh CRDuhV2rbOJEoBYhBKydJ7mEpsswrWYaaO6FXats4kSgc48H/Ra2lq5p3dHsrlQLqM7N68Fo eRDf3PMevXyMlrCYDGLVncQwMw3O/AkousktXKQ42DPJh65zoXB22yUt8m0g12xkLax98KFJ 5NyUloa6HflLl+wQL/uZjIdNUQaHQLw3HKwRMVi4l0/Jh/TygYG1Dtm8I4o708JS4y8GQxoQ UL0z1OM9hyM3gI2WVTTyprsBHy2EjMOu/2Xpod95pF8f90zBLajy6qXEnxlcsqreMaqmkzKn 3KTZpWRxNAS/IH3FbGQ+3RpWkNGSJpwfEMVCeyK5a1n7yt1podd1ajY5mA1jcaUmGppqx827 8TqyteNe1B/pbiUt2L/WhnTgW1NC1QDOwE0EXSW0JwEIAM99H34Bu4MKM7HDJVt864MXbx7B 1M93wVlpJ7Uq+XDFD0A0hIal028j+h6jA6bhzWto4RUfDl/9mn1StngNVFovvwtfzbamp6+W pKHZm9X5YvlIwCx131kTxCNDcF+/adRW4n8CU3pZWYmNVqhMUiPLxElA6QhXTtVBh1RkjCZQ Kmbd1szvcOfaD8s+tJABJzNZsmO2hVuFwkDrRN8Jgrh92a+yHQPd9+RybW2l7sJv26nkUH5Z 5s84P6894ebgimcprJdAkjJTgprl1nhgvptU5M9Uv85Pferoh2groQEAtRPlCGrZ2/2qVNe9 XJfSYbiyedvApWcJs5DOByTaKkcAEQEAAcLAkwQYAQgAJhYhBKydJ7mEpsswrWYaaO6FXats 4kSgBQJdJbQnAhsMBQkDwmcAACEJEO6FXats4kSgFiEErJ0nuYSmyzCtZhpo7oVdq2ziRKD3 twf7BAQBZ8TqR812zKAD7biOnWIJ0McV72PFBxmLIHp24UVe0ZogtYMxSWKLg3csh0yLVwc7 H3vldzJ9AoK3Qxp0Q6K/rDOeUy3HMqewQGcqrsRRh0NXDIQk5CgSrZslPe47qIbe3O7ik/MC q31FNIAQJPmKXX25B115MMzkSKlv4udfx7KdyxHrTSkwWZArLQiEZj5KG4cCKhIoMygPTA3U yGaIvI/BGOtHZ7bEBVUCFDFfOWJ26IOCoPnSVUvKPEOH9dv+sNy7jyBsP5QxeTqwxC/1ZtNS DUCSFQjqA6bEGwM22dP8OUY6SC94x1G81A9/xbtm9LQxKm0EiDH8KBMLfQ== Message-ID: <2dd9aa3e-3dcf-6fb1-bcc8-89312f107d11@suse.de> Date: Thu, 18 Jun 2020 12:16:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <605f6d30-4e18-cebc-517c-5a070e477823@redhat.com> Content-Type: multipart/mixed; boundary="------------69E2601EE78F39A73DC2614C" Content-Language: en-US X-Spam-Status: No, score=-15.6 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: Thu, 18 Jun 2020 10:16:46 -0000 This is a multi-part message in MIME format. --------------69E2601EE78F39A73DC2614C Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit [ was: Re: [PATCH][gdb/testsuite] Don't abort testrun for invalid command in test-case ] On 6/17/20 4:19 PM, Pedro Alves wrote: > On 6/17/20 3:14 PM, Tom de Vries wrote: >> On 6/16/20 2:47 PM, Pedro Alves wrote: >>> On 6/11/20 11:55 PM, Tom de Vries wrote: >>> >>>> Hmm, what is the distinction between gdb_init and default_gdb_init? >>>> >>>> All the other uses in gdb.exp of pattern foo/default_foo have an >>>> implementation: >>>> ... >>>> proc foo {} { >>>> [default_foo] >>>> } >>>> ... >>>> but gdb_init is much more than that. Why is that different? >>>> >>> >>> I don't know. I guess it shouldn't. I guess people (including me) added to >>> gdb_init over time without realizing they were breaking the pattern. Maybe nobody >>> notices it because whatever is overriding gdb_init renames the original one and >>> then calls it. >> >> Hmm, thanks for the explanation. I feel we need to improve this >> situation somehow, but I'm not sure yet how. > > Move stuff in gdb_init to default_gdb_init, and add a comment to these > functions to not add stuff in there? That seems like the obvious. > I'm not sure why we have that pattern in the first place though, given > that you can rename/override functions in tcl anyhow. This all predates me. How about this patch then? Thanks, - Tom --------------69E2601EE78F39A73DC2614C Content-Type: text/x-patch; charset=UTF-8; name="0001-gdb-testsuite-Move-code-from-gdb_init-to-default_gdb_init.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-gdb-testsuite-Move-code-from-gdb_init-to-default_gdb_in"; filename*1="it.patch" [gdb/testsuite] Move code from gdb_init to default_gdb_init If a baseboard file wants to override a proc foo, but also use the original proc, it'll have to do something like: ... rename foo save_foo proc foo { } { ... set res [save_foo] ... return res } ... This adds a new proc named save_foo, which introduces the risk of clashing with an existing proc. There's a pattern in the gdb testsuite procs, that facilitates this override: ... proc default_foo { } { ... } proc foo { } { return [default_foo] } ... such that in a baseboard file we don't need the rename: ... proc foo { } { ... set res [default_foo] ... return res } ... The exception to the pattern though is gdb_init, which has a default_gdb_init counterpart, but contains much more code than just the call to default_gdb_init. Fix this by moving all but the call to default_gdb_init to default_gdb_init. Tested on x86_64-linux. gdb/testsuite/ChangeLog: 2020-06-18 Tom de Vries * lib/gdb.exp (gdb_init): Move all but call to default_gdb_init to ... (default_gdb_init): ... here. --- gdb/testsuite/lib/gdb.exp | 246 ++++++++++++++++++++++++---------------------- 1 file changed, 127 insertions(+), 119 deletions(-) diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 02867fb5bd..7b7484c031 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -4902,6 +4902,7 @@ proc gdb_continue { function } { return [gdb_test "continue" ".*Breakpoint $decimal, $function .*" "continue to $function"] } +# Default implementation of gdb_init. proc default_gdb_init { test_file_name } { global gdb_wrapper_initialized global gdb_wrapper_target @@ -4909,6 +4910,107 @@ proc default_gdb_init { test_file_name } { global cleanfiles global pf_prefix + # Reset the timeout value to the default. This way, any testcase + # that changes the timeout value without resetting it cannot affect + # the timeout used in subsequent testcases. + global gdb_test_timeout + global timeout + set timeout $gdb_test_timeout + + if { [regexp ".*gdb\.reverse\/.*" $test_file_name] + && [target_info exists gdb_reverse_timeout] } { + set timeout [target_info gdb_reverse_timeout] + } + + # If GDB_INOTIFY is given, check for writes to '.'. This is a + # debugging tool to help confirm that the test suite is + # parallel-safe. You need "inotifywait" from the + # inotify-tools package to use this. + global GDB_INOTIFY inotify_pid + if {[info exists GDB_INOTIFY] && ![info exists inotify_pid]} { + global outdir tool inotify_log_file + + set exclusions {outputs temp gdb[.](log|sum) cache} + set exclusion_re ([join $exclusions |]) + + set inotify_log_file [standard_temp_file inotify.out] + set inotify_pid [exec inotifywait -r -m -e move,create,delete . \ + --exclude $exclusion_re \ + |& tee -a $outdir/$tool.log $inotify_log_file &] + + # Wait for the watches; hopefully this is long enough. + sleep 2 + + # Clear the log so that we don't emit a warning the first time + # we check it. + set fd [open $inotify_log_file w] + close $fd + } + + # Block writes to all banned variables, and invocation of all + # banned procedures... + global banned_variables + global banned_procedures + global banned_traced + if (!$banned_traced) { + foreach banned_var $banned_variables { + global "$banned_var" + trace add variable "$banned_var" write error + } + foreach banned_proc $banned_procedures { + global "$banned_proc" + trace add execution "$banned_proc" enter error + } + set banned_traced 1 + } + + # We set LC_ALL, LC_CTYPE, and LANG to C so that we get the same + # messages as expected. + setenv LC_ALL C + setenv LC_CTYPE C + setenv LANG C + + # Don't let a .inputrc file or an existing setting of INPUTRC mess up + # the test results. Even if /dev/null doesn't exist on the particular + # platform, the readline library will use the default setting just by + # failing to open the file. OTOH, opening /dev/null successfully will + # also result in the default settings being used since nothing will be + # read from this file. + setenv INPUTRC "/dev/null" + + # This disables style output, which would interfere with many + # tests. + setenv TERM "dumb" + + # Ensure that GDBHISTFILE and GDBHISTSIZE are removed from the + # environment, we don't want these modifications to the history + # settings. + unset -nocomplain ::env(GDBHISTFILE) + unset -nocomplain ::env(GDBHISTSIZE) + + # Initialize GDB's pty with a fixed size, to make sure we avoid pagination + # during startup. See "man expect" for details about stty_init. + global stty_init + set stty_init "rows 25 cols 80" + + # Some tests (for example gdb.base/maint.exp) shell out from gdb to use + # grep. Clear GREP_OPTIONS to make the behavior predictable, + # especially having color output turned on can cause tests to fail. + setenv GREP_OPTIONS "" + + # Clear $gdbserver_reconnect_p. + global gdbserver_reconnect_p + set gdbserver_reconnect_p 1 + unset gdbserver_reconnect_p + + # Clear $last_loaded_file + global last_loaded_file + unset -nocomplain last_loaded_file + + # Reset GDB number of instances + global gdb_instances + set gdb_instances 0 + set cleanfiles {} gdb_clear_suppressed @@ -4942,6 +5044,20 @@ proc default_gdb_init { test_file_name } { if [info exists use_gdb_stub] { unset use_gdb_stub } + + gdb_setup_known_globals + + if { [info procs ::gdb_tcl_unknown] != "" } { + # Dejagnu overrides proc unknown. The dejagnu version may trigger in a + # test-case but abort the entire test run. To fix this, we install a + # local version here, which reverts dejagnu's override, and restore + # dejagnu's version in gdb_finish. + rename ::unknown ::dejagnu_unknown + proc unknown { args } { + # Use tcl's unknown. + return [uplevel 1 ::gdb_tcl_unknown $args] + } + } } # Return a path using GDB_PARALLEL. @@ -5188,127 +5304,19 @@ if { [interp eval $temp "info procs ::unknown"] != "" } { interp delete $temp unset temp -proc gdb_init { test_file_name } { - # Reset the timeout value to the default. This way, any testcase - # that changes the timeout value without resetting it cannot affect - # the timeout used in subsequent testcases. - global gdb_test_timeout - global timeout - set timeout $gdb_test_timeout - - if { [regexp ".*gdb\.reverse\/.*" $test_file_name] - && [target_info exists gdb_reverse_timeout] } { - set timeout [target_info gdb_reverse_timeout] - } - - # If GDB_INOTIFY is given, check for writes to '.'. This is a - # debugging tool to help confirm that the test suite is - # parallel-safe. You need "inotifywait" from the - # inotify-tools package to use this. - global GDB_INOTIFY inotify_pid - if {[info exists GDB_INOTIFY] && ![info exists inotify_pid]} { - global outdir tool inotify_log_file - - set exclusions {outputs temp gdb[.](log|sum) cache} - set exclusion_re ([join $exclusions |]) - - set inotify_log_file [standard_temp_file inotify.out] - set inotify_pid [exec inotifywait -r -m -e move,create,delete . \ - --exclude $exclusion_re \ - |& tee -a $outdir/$tool.log $inotify_log_file &] - - # Wait for the watches; hopefully this is long enough. - sleep 2 - - # Clear the log so that we don't emit a warning the first time - # we check it. - set fd [open $inotify_log_file w] - close $fd - } - - # Block writes to all banned variables, and invocation of all - # banned procedures... - global banned_variables - global banned_procedures - global banned_traced - if (!$banned_traced) { - foreach banned_var $banned_variables { - global "$banned_var" - trace add variable "$banned_var" write error - } - foreach banned_proc $banned_procedures { - global "$banned_proc" - trace add execution "$banned_proc" enter error - } - set banned_traced 1 - } - - # We set LC_ALL, LC_CTYPE, and LANG to C so that we get the same - # messages as expected. - setenv LC_ALL C - setenv LC_CTYPE C - setenv LANG C - - # Don't let a .inputrc file or an existing setting of INPUTRC mess up - # the test results. Even if /dev/null doesn't exist on the particular - # platform, the readline library will use the default setting just by - # failing to open the file. OTOH, opening /dev/null successfully will - # also result in the default settings being used since nothing will be - # read from this file. - setenv INPUTRC "/dev/null" - - # This disables style output, which would interfere with many - # tests. - setenv TERM "dumb" - - # Ensure that GDBHISTFILE and GDBHISTSIZE are removed from the - # environment, we don't want these modifications to the history - # settings. - unset -nocomplain ::env(GDBHISTFILE) - unset -nocomplain ::env(GDBHISTSIZE) - - # Initialize GDB's pty with a fixed size, to make sure we avoid pagination - # during startup. See "man expect" for details about stty_init. - global stty_init - set stty_init "rows 25 cols 80" - - # Some tests (for example gdb.base/maint.exp) shell out from gdb to use - # grep. Clear GREP_OPTIONS to make the behavior predictable, - # especially having color output turned on can cause tests to fail. - setenv GREP_OPTIONS "" - - # Clear $gdbserver_reconnect_p. - global gdbserver_reconnect_p - set gdbserver_reconnect_p 1 - unset gdbserver_reconnect_p - - # Clear $last_loaded_file - global last_loaded_file - unset -nocomplain last_loaded_file - - # Reset GDB number of instances - global gdb_instances - set gdb_instances 0 - - set res [default_gdb_init $test_file_name] - - gdb_setup_known_globals - - if { [info procs ::gdb_tcl_unknown] != "" } { - # Dejagnu overrides proc unknown. The dejagnu version may trigger in a - # test-case but abort the entire test run. To fix this, we install a - # local version here, which reverts dejagnu's override, and restore - # dejagnu's version in gdb_finish. - rename ::unknown ::dejagnu_unknown - proc unknown { args } { - # Use tcl's unknown. - return [uplevel 1 ::gdb_tcl_unknown $args] - } - } - - return $res +# Gdb implementation of ${tool}_init. Called right before executing the +# test-case. +# Overridable function -- you can override this function in your +# baseboard file. +proc gdb_init { args } { + # A baseboard file overriding this proc and calling the default version + # should behave the same as this proc. So, don't add code here, but to + # the default version instead. + return [default_gdb_init {*}$args] } +# Gdb implementation of ${tool}_finish. Called right after executing the +# test-case. proc gdb_finish { } { global gdbserver_reconnect_p global gdb_prompt --------------69E2601EE78F39A73DC2614C--