diff options
author | Andrew Burgess <aburgess@redhat.com> | 2022-09-14 13:51:28 +0100 |
---|---|---|
committer | Andrew Burgess <aburgess@redhat.com> | 2022-09-22 10:34:15 +0100 |
commit | 7abc6ec0a6f7e82bda6f00c1f1ae7c638d1c799c (patch) | |
tree | c25e132a81ec94cf1bb4d21fa844ef0cb35924c4 /gdb | |
parent | 9c5f314314e2d14c7602a79b969e02ebdd8890ea (diff) | |
download | gdb-7abc6ec0a6f7e82bda6f00c1f1ae7c638d1c799c.zip gdb-7abc6ec0a6f7e82bda6f00c1f1ae7c638d1c799c.tar.gz gdb-7abc6ec0a6f7e82bda6f00c1f1ae7c638d1c799c.tar.bz2 |
gdb/python: restrict the names accepted by gdb.register_window_type
I noticed that, from Python, I could register a new TUI window that
had whitespace in its name, like this:
gdb.register_window_type('my window', MyWindowType)
however, it is not possible to then use this window in a new TUI
layout, e.g.:
(gdb) tui new-layout foo my window 1 cmd 1
Unknown window "my"
(gdb) tui new-layout foo "my window" 1 cmd 1
Unknown window ""my"
(gdb) tui new-layout foo my\ window 1 cmd 1
Unknown window "my\"
GDB clearly uses the whitespace to split the incoming command line.
I could fix this by trying to add a mechanism by which we can use
whitespace within a window name, but it seems like an easier solution
if we just forbid whitespace within a window name. Not only is this
easier, but I think this is probably the better solution, identifier
names with spaces in would mean we'd need to audit all the places a
window name could be printed and ensure that the use of a space didn't
make the output ambiguous.
So, having decided to disallow whitespace, I then thought about other
special characters. We currently accept anything as a window name,
and I wondered if this was a good idea.
My concerns were about how special characters used in a window name
might cause confusion, for example, we allow '$' in window names,
which is maybe fine now, but what if one day we wanted to allow
variable expansion when creating new layouts? Or what about starting
a window name with '-'? We already support a '-horizontal' option,
what if we want to add more in the future? Or use of the special
character '{' which has special meaning within a new layout?
In the end I figured it might make sense to place some restrictive
rules in place, and then relax the rules later if/when users complain,
we can consider each relaxation as its requested.
So, I propose that window names should match this regular expression:
[a-zA-Z][-_.a-zA-Z0-9]*
There is a chance that there is user code in the wild which will break
with the addition of this change, but hopefully adapting to the new
restrictions shouldn't be too difficult.
Diffstat (limited to 'gdb')
-rw-r--r-- | gdb/NEWS | 5 | ||||
-rw-r--r-- | gdb/doc/python.texi | 4 | ||||
-rw-r--r-- | gdb/testsuite/gdb.python/tui-window-names.exp | 84 | ||||
-rw-r--r-- | gdb/tui/tui-layout.c | 13 |
4 files changed, 105 insertions, 1 deletions
@@ -193,6 +193,11 @@ GNU/Linux/CSKY (gdbserver) csky*-*linux* gdb.BreakpointLocation objects specifying the locations where the breakpoint is inserted into the debuggee. + ** The gdb.register_window_type method now restricts the set of + acceptable window names. The first character of a window's name + must start with a character in the set [a-zA-Z], every subsequent + character of a window's name must be in the set [-_.a-zA-Z0-9]. + * New features in the GDB remote stub, GDBserver ** GDBserver is now supported on LoongArch GNU/Linux. diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index 7aa9e85..2692211 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -6597,7 +6597,9 @@ factory function with @value{GDBN}. @var{name} is the name of the new window. It's an error to try to replace one of the built-in windows, but other window types can be -replaced. +replaced. The @var{name} should match the regular expression +@code{[a-zA-Z][-_.a-zA-Z0-9]*}, it is an error to try and create a +window with an invalid name. @var{function} is a factory function that is called to create the TUI window. This is called with a single argument of type diff --git a/gdb/testsuite/gdb.python/tui-window-names.exp b/gdb/testsuite/gdb.python/tui-window-names.exp new file mode 100644 index 0000000..8aaf3b4 --- /dev/null +++ b/gdb/testsuite/gdb.python/tui-window-names.exp @@ -0,0 +1,84 @@ +# Copyright (C) 2022 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 that GDB rejects invalid TUI window names, and that valid names +# are allowed. + +load_lib gdb-python.exp + +tuiterm_env + +clean_restart + +if {[skip_tui_tests]} { + return +} + +# Define a function we can use as a window constructor. If this ever +# gets called we'll throw an error, but that's OK, this test doesn't +# actually try to create any windows. +gdb_test_multiline "create a window constructor" \ + "python" "" \ + "def failwin(win):" "" \ + " raise RuntimeError('failwin')" "" \ + "end" "" + +# Check for some of the characters that can't be used within a window +# name. +foreach c {$ * \{ \} ( ) @ #} { + set re [string_to_regexp "$c"] + gdb_test "python gdb.register_window_type('te${c}st', failwin)" \ + [multi_line \ + "gdb.error: invalid character '${re}' in window name" \ + "Error while executing Python code\\." ] + + gdb_test "python gdb.register_window_type('${c}test', failwin)" \ + [multi_line \ + "gdb.error: invalid character '${re}' in window name" \ + "Error while executing Python code\\." ] +} + +# Check that whitespace within a window name is rejected. +foreach c [list " " "\\t" "\\n" "\\r"] { + gdb_test "python gdb.register_window_type('te${c}st', failwin)" \ + [multi_line \ + "gdb.error: invalid whitespace character in window name" \ + "Error while executing Python code\\." ] +} + +# Check some of the characters which are allowed within a window name, +# but are not allowed to be used as the first character. +foreach c {1 _ - .} { + set re [string_to_regexp "$c"] + gdb_test "python gdb.register_window_type('${c}test', failwin)" \ + [multi_line \ + "gdb.error: window name must start with a letter, not '${re}'" \ + "Error while executing Python code\\." ] +} + +# Check different capitalisations. +gdb_test_no_output "python gdb.register_window_type('TEST', failwin)" +gdb_test_no_output "python gdb.register_window_type('test', failwin)" +gdb_test_no_output "python gdb.register_window_type('tEsT', failwin)" +gdb_test_no_output "python gdb.register_window_type('TeSt', failwin)" + +# Check a set of characters that can appear within a name, just not +# necessarily as the first character. We check at both the end of the +# name, and within the name. +foreach c {1 _ - . A} { + set re [string_to_regexp "$c"] + gdb_test_no_output "python gdb.register_window_type('test${c}', failwin)" + gdb_test_no_output "python gdb.register_window_type('te${c}st', failwin)" +} diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index 09887d3..2c4e60a 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -44,6 +44,7 @@ #include "tui/tui-layout.h" #include "tui/tui-source.h" #include "gdb_curses.h" +#include "safe-ctype.h" static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *); @@ -405,6 +406,18 @@ tui_register_window (const char *name, window_factory &&factory) || name_copy == DISASSEM_NAME || name_copy == STATUS_NAME) error (_("Window type \"%s\" is built-in"), name); + for (const char &c : name_copy) + { + if (ISSPACE (c)) + error (_("invalid whitespace character in window name")); + + if (!ISALNUM (c) && strchr ("-_.", c) == nullptr) + error (_("invalid character '%c' in window name"), c); + } + + if (!ISALPHA (name_copy[0])) + error (_("window name must start with a letter, not '%c'"), name_copy[0]); + known_window_types->emplace (std::move (name_copy), std::move (factory)); } |