From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0DB6B3858D28 for ; Wed, 25 Jan 2023 11:32:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0DB6B3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674646363; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3zdDhiZQxIh7wOHCAQDbrvQ7jfx3A527R6WiZCFfO2U=; b=WSUwDi8TmQjy/bKp+99Vsde3/YfE9PzuY9EkCwGPTeY7lvgxd0pk+5OKHo90NacV7jTSxh E9sgetVECkK3W0EbkNYd/n8uH2qUpw+SuzCPI3k2ZNGB/iLX1GKr7jnwSfAwxoAC25tNhX bx8hfSi519+KXdbpFnhka9gD7aWbhow= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-553-mCKh8uvKN-ehJgX5DR7cfw-1; Wed, 25 Jan 2023 06:32:39 -0500 X-MC-Unique: mCKh8uvKN-ehJgX5DR7cfw-1 Received: by mail-qt1-f197.google.com with SMTP id f2-20020ac80682000000b003b6364059d2so7378629qth.9 for ; Wed, 25 Jan 2023 03:32:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=3zdDhiZQxIh7wOHCAQDbrvQ7jfx3A527R6WiZCFfO2U=; b=ATc7zIYYgoBQKGWe9NzrHmI0v0f8lCV/f0/jkvXSb3ksp7axryamtDhRAmDbMdnVuU 0ByDAMAQAJLcXdCZekQrc5ecaSENUxgBUcAEZTH4czXpEPO9gr5XIv/P8Ze/SzSKMJ3D IVuZvs/dPCB51DXnmAjkU0SrHpkwws3H1MoYOwlDr6WrHPv8uAm2L9JrSMdir9WxU++f LNIElSbQlmJAKb0+tc7eP6t23wU7M8tPJq1UlZvCWTsUKGKSpp5oV3yx/COUe9zLgt8O JmylqD2JUr9ojENNBpQBIOMQDHWInc5PSi/CImdgKtV6oZXhaUEZdRlgdYKZDJchVgI0 o5JQ== X-Gm-Message-State: AFqh2koRuG1kW8Dz064B/IfbxmE6O6azMFb4hvTyDxOSHRBFnvtNx+rj QkZamJHVFsJGj1NDp3jxebOQPs3VCk8ga5UtCJIutj3zjaSaX17LO3XqtLMZfzpzOphp4DAE+NJ o8DeKjVPPEDFNFm7A3g6dYfdSaps/42fP/bN4nJCr/9fGbXUcST2OblzGPq1XM67ni5lIDAXoCA == X-Received: by 2002:ac8:6053:0:b0:3b6:9b37:e03c with SMTP id k19-20020ac86053000000b003b69b37e03cmr24448487qtm.7.1674646358597; Wed, 25 Jan 2023 03:32:38 -0800 (PST) X-Google-Smtp-Source: AMrXdXtWqS7ZagFQAztObKVmBHFuScs8M/sKdgIIudotrNSVjDMUPNzHdLy+im1y2344c9mDAamAfQ== X-Received: by 2002:ac8:6053:0:b0:3b6:9b37:e03c with SMTP id k19-20020ac86053000000b003b69b37e03cmr24448460qtm.7.1674646358124; Wed, 25 Jan 2023 03:32:38 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id b5-20020a37b205000000b00709041c3206sm3281367qkf.103.2023.01.25.03.32.37 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Jan 2023 03:32:37 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: Re: [PATCH 04/15] gdb/testsuite/tui: more testing of the 'focus' command In-Reply-To: References: Date: Wed, 25 Jan 2023 11:32:36 +0000 Message-ID: <87h6wex31n.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Andrew Burgess writes: > I noticed that we didn't have much testing of the tui 'focus' command, > so I started adding some. This exposed a bug in GDB, we are able to > focus windows that should not be focusable, e.g. the 'status' window. > > This is harmless until we then do 'focus next' or 'focus prev', along > this code path we assert that the currently focused window is > focusable, which obviously, is no longer true, so GDB fails with an > assertion error. > > The fix is simple; add some code to the tui_set_focus_command function > that checks if the selected window is focusable. If it is not then an > error is thrown. I noticed that there was some code commented out in this patch which was then uncommented in a later patch in the original series. I've fixed that so that everything is correctly included in this patch. With that fix I've gone ahead and pushed this change. The final version that I pushed is included below. Thanks, Andrew --- commit 24f3aded1d42f515527e2de7e8e9e26f0b77c932 Author: Andrew Burgess Date: Tue Dec 20 15:01:29 2022 +0000 gdb/testsuite/tui: more testing of the 'focus' command I noticed that we didn't have many tests of the tui 'focus' command, so I started adding some. This exposed a bug in GDB; we are able to focus windows that should not be focusable, e.g. the 'status' window. This is harmless until we then do 'focus next' or 'focus prev', along this code path we assert that the currently focused window is focusable, which obviously, is not always true, so GDB fails with an assertion error. The fix is simple; add a check to the tui_set_focus_command function to ensure that the selected window is focusable. If it is not then an error is thrown. The new tests I've added cover this case. diff --git a/gdb/testsuite/gdb.tui/tui-focus.c b/gdb/testsuite/gdb.tui/tui-focus.c new file mode 100644 index 00000000000..3a264f239ed --- /dev/null +++ b/gdb/testsuite/gdb.tui/tui-focus.c @@ -0,0 +1,22 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2023 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 . */ + +int +main () +{ + return 0; +} diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tui-focus.exp new file mode 100644 index 00000000000..9e2a1cd0935 --- /dev/null +++ b/gdb/testsuite/gdb.tui/tui-focus.exp @@ -0,0 +1,68 @@ +# Copyright 2023 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 . + +# Minimal testcase that just checks that the various "layout $foo" +# commands do not cause gdb to crash. + +require allow_tui_tests + +tuiterm_env + +standard_testfile + +if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} { + return -1 +} + +# Run a series of tests based on various test specifications. +# +# Each test specification is a tuple where the first item is the name of a +# window, and the second item is a boolean indicating if we expect that +# window to be present in the default (src) layout. +foreach spec {{src true} {cmd true} {status true} {regs false} {asm false}} { + lassign $spec window valid_p + with_test_prefix "window=$window" { + + Term::clean_restart 24 80 $binfile + if {![Term::prepare_for_tui]} { + unsupported "TUI not supported" + return + } + + Term::command_no_prompt_prefix "focus $window" + + if {$valid_p} { + # The 'status' window is special, it's present in the + # default (src) layout, but is not focusable. + if {$window == "status"} { + Term::check_region_contents "check focus error" 0 16 80 1 \ + "^Window \"$window\" cannot be focused\\s*" + } else { + Term::check_region_contents "check focus message" 0 16 80 1 \ + "^Focus set to $window window\\.\\s*" + } + } else { + Term::check_region_contents "check focus error" 0 16 80 1 \ + "^Unrecognized window name \"$window\"\\s*" + } + + Term::check_box "check src box" 0 0 80 15 + + # At one point the following 'focus prev' command would trigger a + # crash in GDB, GDB was allowing users to set focus to the 'status' + # window, and 'focus prev' would then trigger an assert. + Term::command "focus prev" + } +} diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c index 58372005ff8..9c088899817 100644 --- a/gdb/tui/tui-win.c +++ b/gdb/tui/tui-win.c @@ -723,6 +723,9 @@ tui_set_focus_command (const char *arg, int from_tty) if (!win_info->is_visible ()) error (_("Window \"%s\" is not visible"), arg); + if (!win_info->can_focus ()) + error (_("Window \"%s\" cannot be focused"), arg); + tui_set_win_focus_to (win_info); gdb_printf (_("Focus set to %s window.\n"), tui_win_with_focus ()->name ());