Re: [PATCH v5 4/5] gdb: introduce ability to disable frame unwinders

Thiago Jung Bauermann
2024-10-03T02:46:15Z

Guinevere Larsen <guinevere@redhat.com> writes:

> +/* Helper function to both enable and disable frame unwinders.
> + if ENABLE is true, this call will be enabling unwinders,
> + otherwise the unwinders will be disabled. */
> +static void
> +enable_disable_frame_unwinders (const char *args, int from_tty, bool enable)
> +{
> + reinit_frame_cache ();
> + if (args == nullptr)
> + {
> + if (enable)
> + error (_("specify which frame unwinder(s) should be enabled"));
> + else

git complains about this line:

/home/bauermann/src/binutils-gdb/.git/worktrees/binutils-gdb-wt-2/rebase-apply/patch:327: trailing whitespace.
else
warning: 1 line adds whitespace errors.

> + error (_("specify which frame unwinder(s) should be disabled"));
> + }

> diff --git a/gdb/testsuite/gdb.base/frame-unwind-disable.exp b/gdb/testsuite/gdb.base/frame-unwind-disable.exp
> new file mode 100644
> index 00000000000..972d6b70351
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/frame-unwind-disable.exp
> @@ -0,0 +1,140 @@
> +# Copyright 2024 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test multiple situations in which we may use the maintenance command to
> +# disable and enable frame unwinders, and check that they really are
> +# disabled when they say the are.
> +
> +standard_testfile .c
> +
> +# Proc to check if the unwinder of the given name is in the desired state.
> +# STATE can be either Y or N.
> +proc check_unwinder_state { unwinder_name state {testname ""} } {
> + set should_pass false
> + set command "maint info frame-unwinders"
> + if {${testname} == ""} {
> + set testname "checking state ${state} for ${unwinder_name}"
> + }
> + gdb_test_multiple "${command}" "${testname}" -lbl {
> + -re "^${unwinder_name}\\s+\\w+\\s+\\w+\\s+${state}\\s+" {

With 'make check-read1', this test only passes if I remove the '^' from
the beginning of the pattern.

> + set should_pass true
> + exp_continue
> + }
> + -re "${command}" {
> + exp_continue
> + }
> + -re "\\w+\\s+\\w+\\s+\\w+\\s+\\w+\\s+" {
> + exp_continue
> + }
> + -re "${::gdb_prompt} $" {
> + # We can't use -wrap here because it expects the line with start
> + # with \r\n, but the previous commands eat that

If I use -re -wrap "" here things still work (with or without read1), so
this comment probably applied only to a previous version of this test.

It does apply to the prompt pattern in check_unwinder_class, but there's
a way around that. See below.

> + gdb_assert {${should_pass} == true} "${gdb_test_name}"
> + }
> + }
> +}
> +
> +proc check_unwinder_class { unwinder_class state {testname ""} } {
> + set command "maint info frame-unwinders"
> + set seen_arches 0
> + set correct_state 0
> + if {$testname == ""} {
> + set testname "checking if ${unwinder_class} state is ${state}"
> + }
> + gdb_test_multiple "${command}" "${testname}" -lbl {
> + -re "^\[^\r\n\]+\\s+\\w+\\s+${unwinder_class}\\s+\(\[YN\]\)\\s+\r\n" {

You can change the last "\r\n" in the pattern to "(?=\r\n)", which tests
for the presence of the characters without taking them out of the buffer.

> + # The unwinder name may have multiple words, so we need to use the
> + # more generic [^\r\n] pattern to match the unwinders.
> + set should_pass true
> + incr seen_arches
> + set cur_state $expect_out(1,string)
> + if {$cur_state == $state} {
> + incr correct_state
> + }
> + exp_continue
> + }
> + -re "${command}" {
> + exp_continue
> + }
> + -re "\[^\r\n\]+\r\n" {

Same here, though this pattern is very similar to what the -lbl option
adds:

if {$line_by_line} {
append code {
-re "\r\n\[^\r\n\]*(?=\r\n)" {
exp_continue
}
}
}

So is it necessary?

> + exp_continue
> + }
> + -re "${::gdb_prompt} $" {

With the changes above I can use -re -wrap "" here.

> + gdb_assert {${correct_state} == ${seen_arches}} "${gdb_test_name}"
> + }
> + }
> +}

--
Thiago