Re: [PATCH 05/22] Implement locals TUI window

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> [2021-03-06 18:34:00 +0100]:

I feel there should be more of a commit message here.

> PR tui/17849

I was going to say the PR number should be attached to the ChangeLog
entry, and it was only then that I realised none of the patches have
had a ChangeLog entry in the commit message.

> ---
>  gdb/data-directory/Makefile.in            |   1 +
>  gdb/python/lib/gdb/command/tui_windows.py | 562 ++++++++++++++++++++++

I'll make some general observations here.  My two observations on
coding style, from a quick scan of the code would be:

  - Comments, and
  - Line length.

I think as in GDB, every function should be commented.  Looking at the
other Python code in this directory the preference seems to be for
Pythons doc-string style, which makes sense.  So you should probably
go with that.

I'm not sure we need to limit ourselves to 80 characters as in GDB,
but I think some of your lines in this file are too long.  My
preference would be target 80 wherever possible, but set a hard limit
of, maybe 100.  However, this is just my opinion, not a general GDB
policy, so others might disagree.

In terms of content, I think you need to add a documentation entry for
this feature, as well as a NEWS entry.

I also think we need some tests written.

Finally, I notice that there's a click handler.  I haven't tried to
figure out what its doing, once you written a commit message & docs
that will be obvious, but I wonder if we should consider adding a
command driven approach to access the same functionality?

So you might add a command 'tui locals .....' that allows the user to
achieve the same thing as a mouse click, even if (potentially) less
efficiently?  Like I say, I made no attempt to figure out what the
click is offering at this point, I just think this is something to
consider.

Thanks,
Andrew

>  2 files changed, 563 insertions(+)
>  create mode 100644 gdb/python/lib/gdb/command/tui_windows.py
> 
> diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
> index 8b65790cdd9..b8383ea7808 100644
> --- a/gdb/data-directory/Makefile.in
> +++ b/gdb/data-directory/Makefile.in
> @@ -82,6 +82,7 @@ PYTHON_FILE_LIST = \
>  	gdb/command/frame_filters.py \
>  	gdb/command/pretty_printers.py \
>  	gdb/command/prompt.py \
> +	gdb/command/tui_windows.py \
>  	gdb/command/type_printers.py \
>  	gdb/command/unwinders.py \
>  	gdb/command/xmethods.py \
> diff --git a/gdb/python/lib/gdb/command/tui_windows.py b/gdb/python/lib/gdb/command/tui_windows.py
> new file mode 100644
> index 00000000000..cda43743cf1
> --- /dev/null
> +++ b/gdb/python/lib/gdb/command/tui_windows.py
> @@ -0,0 +1,562 @@
> +# Additional TUI windows.
> +# Copyright 2021 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/>.
> +
> +import gdb
> +import sys
> +import re
> +
> +PY3 = sys.version_info[0] == 3
> +
> +custom_windows = {}
> +
> +
> +col_esc_seq_re = re.compile('(\033\[[0-9;]*m)')
> +def escaped_substr(s, n, c):
> +    col_esc_seq = False
> +    init_col_esc_seq = None
> +    last_col_esc_seq = None
> +    sub = ""
> +    for p in col_esc_seq_re.split(s):
> +        t = ""
> +        if n > 0:
> +            if col_esc_seq:
> +                init_col_esc_seq = p
> +            else:
> +                l = len(p)
> +                if l <= n:
> +                    n -= l
> +                else:
> +                    if init_col_esc_seq:
> +                        sub += init_col_esc_seq
> +                    t = p[n:]
> +                    n = 0
> +        else:
> +            t = p
> +        if c > 0:
> +            if col_esc_seq:
> +                sub += t
> +            else:
> +                l = len(t)
> +                if l <= c:
> +                    sub += t
> +                    c -= l
> +                else:
> +                    sub += t[:c]
> +                    c = 0
> +        elif col_esc_seq:
> +            last_col_esc_seq = p
> +        col_esc_seq = not col_esc_seq
> +    if c > 0:
> +        sub += ' ' * c
> +    if last_col_esc_seq:
> +        sub += last_col_esc_seq
> +    return sub
> +
> +class TextWindow(object):
> +    def __init__(self, win, title):
> +        self.win = win
> +        self.line_ofs = 0
> +        self.col_ofs = 0
> +        self.lines = []
> +        win.title = title
> +        global custom_windows
> +        custom_windows[title] = self
> +
> +    def close(self):
> +        global custom_windows
> +        del custom_windows[self.win.title]
> +
> +    def render(self):
> +        self.line_ofs = 0
> +        self.col_ofs = 0
> +
> +    def hscroll(self, num):
> +        prev_col_ofs = self.col_ofs
> +        self.col_ofs = self.col_ofs + num
> +        if self.col_ofs < 0:
> +            self.col_ofs = 0
> +        if self.col_ofs != prev_col_ofs:
> +            self.redraw()
> +
> +    def vscroll(self, num):
> +        prev_line_ofs = self.line_ofs
> +        self.line_ofs = self.line_ofs + num
> +        l = len(self.lines)
> +        if self.line_ofs >= l:
> +            self.line_ofs = l - 1
> +        if self.line_ofs < 0:
> +            self.line_ofs = 0
> +        if self.line_ofs != prev_line_ofs:
> +            self.redraw()
> +
> +    def redraw(self):
> +        l = len(self.lines)
> +        if self.line_ofs > 0 and self.line_ofs + self.win.height > l:
> +            self.line_ofs = l - self.win.height
> +            if self.line_ofs < 0:
> +                self.line_ofs = 0
> +        start = self.line_ofs
> +        stop = self.line_ofs + self.win.height
> +        if stop > l:
> +            stop = l
> +        self.win.erase()
> +        if stop > start:
> +            self.win.write("".join([escaped_substr(l, self.col_ofs, self.win.width) for l in self.lines[start:stop]]))
> +
> +
> +def is_string_instance(s):
> +    if PY3:
> +        return isinstance(s, str)
> +    else:
> +        return isinstance(s, basestring)
> +
> +def val_cmp_color(prev, cur):
> +    if prev is None:
> +        return ("\033[1;32m", "\033[0m", "", "")
> +    elif prev != cur:
> +        return ("", "", "\033[1;31m", "\033[0m")
> +    return ("", "", "", "")
> +
> +def octal_escape(s):
> +    return "".join(c if ord(c) < 128 else "\\%03o" % ord(c) for c in s)
> +
> +def value_string(valstr, hint):
> +    if hint == "string":
> +        is_lazy = type(valstr).__name__ == "LazyString"
> +        if is_lazy:
> +            l = valstr.length
> +        elif not is_string_instance(valstr):
> +            l = 1
> +        else:
> +            l = len(valstr)
> +        if l == 0:
> +            if is_lazy and str(valstr.type) == "wchar_t *":
> +                valstr = "L\"\""
> +            else:
> +                valstr = "\"\""
> +        else:
> +            valstr = gdb.Value(valstr).format_string(symbols=False, address=False)
> +    if isinstance(valstr, gdb.Value):
> +        valstr = valstr.format_string(symbols=False, address=False)
> +    elif not type(valstr) is str:
> +        valstr = str(valstr)
> +    return valstr
> +
> +def is_typedef_of(valtype, name):
> +    if valtype.name == name:
> +        return True
> +    while valtype.code == gdb.TYPE_CODE_TYPEDEF:
> +        valtype = valtype.target()
> +        if valtype.name == name:
> +            return True
> +    return False
> +
> +
> +class VariableWindow(TextWindow):
> +    def __init__(self, win, title, convenience_name):
> +        super(VariableWindow, self).__init__(win, title)
> +        self.prev_vals = {}
> +        self.line_names = []
> +        self.convenience_name = convenience_name
> +
> +    def click(self, x, y, button):
> +        line = y + self.line_ofs
> +        if button == 1 and line < len(self.line_names):
> +            name = self.line_names[line]
> +            if name:
> +                prev = self.prev_vals[name]
> +                if prev is not None and prev[0] is not None:
> +                    prev[0] = not prev[0]
> +                    self.refill(True)
> +        elif button == 3 and line < len(self.line_names):
> +            name = self.line_names[line]
> +            if name:
> +                prev = self.prev_vals[name]
> +                if prev is not None and prev[3] is not None:
> +                    gdb.set_convenience_variable(self.convenience_name, prev[3])
> +                    self.refill(True)
> +
> +    def refill(self, keep_prev=False):
> +        if not self.win.is_valid():
> +            return
> +        self.lines = []
> +        self.line_names = []
> +        cur_vals = {}
> +        for v in self.variables():
> +            name = v.symbol()
> +            value = v.value()
> +            sym_not_init = False
> +            num = 0
> +            fmt = None
> +            error = None
> +            if hasattr(v, "undeclared"):
> +                sym_not_init = v.undeclared()
> +            if hasattr(v, "number"):
> +                num = v.number()
> +            if hasattr(v, "format"):
> +                fmt = v.format()
> +            if hasattr(v, "error"):
> +                error = v.error()
> +            self.add_val(name, name, value, 0, num, cur_vals, keep_prev, False, sym_not_init, fmt, True, error)
> +        self.prev_vals = cur_vals
> +        self.redraw()
> +
> +    def add_val(self, n, fn, v, inset, num, cur_vals, keep_prev, def_expand, sym_not_init, fmt, dyn_type, error):
> +        n2 = fn
> +        if inset == 0:
> +            if num == 0:
> +                count = 1
> +                while n2 in cur_vals:
> +                    count += 1
> +                    n2 = "%s:%d" % (n, count)
> +            else:
> +                n2 = "%d:%s" % (num, n)
> +
> +        cur_entry = [None, False, None, None]
> +        cur_vals[n2] = cur_entry
> +        expand = None
> +        prev_val = None
> +        if n2 in self.prev_vals:
> +            pv = self.prev_vals[n2]
> +            expand = pv[0]
> +            if keep_prev:
> +                prev_val = pv[2]
> +            else:
> +                prev_val = pv[1]
> +            cur_entry[0] = expand
> +            cur_entry[2] = prev_val
> +
> +        spaces = "  " * inset
> +        if num > 0:
> +            numstr = "%d: " % num
> +        else:
> +            numstr = ""
> +
> +        if v is None:
> +            if error is None:
> +                var_col_s, var_col_e = "\033[1;30m", "\033[0m"
> +                if sym_not_init:
> +                    var_col_s, var_col_e = "\033[33m", "\033[0m"
> +                self.lines.append(spaces + numstr + "  " + var_col_s + n + var_col_e)
> +            else:
> +                var_col_s, var_col_e = "", ""
> +                if sym_not_init:
> +                    var_col_s, var_col_e = "\033[33m", "\033[0m"
> +                self.lines.append(spaces + numstr + "  " + var_col_s + n + var_col_e + " = \033[1;30m<" + error + ">\033[0m")
> +            self.line_names.append(n2)
> +            return
> +
> +        if is_string_instance(v):
> +            (var_col_s, var_col_e, val_col_s, val_col_e) = val_cmp_color(prev_val, v)
> +            if sym_not_init:
> +                var_col_s, var_col_e = "\033[33m", "\033[0m"
> +            self.lines.append(spaces + numstr + "  " + var_col_s + n + var_col_e + " = " + val_col_s + v + val_col_e)
> +            self.line_names.append(n2)
> +            cur_entry[1] = v
> +            return
> +
> +        if isinstance(v, gdb.Value):
> +            if v.type.code in [gdb.TYPE_CODE_REF, gdb.TYPE_CODE_RVALUE_REF]:
> +                v = v.referenced_value()
> +
> +            is_optimized_out = False
> +            try:
> +                is_optimized_out = v.is_optimized_out
> +            except:
> +                self.add_val(n, fn, None, inset, num, cur_vals, keep_prev, False, sym_not_init, fmt, False, str(sys.exc_info()[1]))
> +                return
> +
> +            if is_optimized_out and v.type.strip_typedefs().code not in [gdb.TYPE_CODE_STRUCT, gdb.TYPE_CODE_UNION]:
> +                self.add_val(n, fn, None, inset, num, cur_vals, keep_prev, False, sym_not_init, fmt, False, "optimized out")
> +                return
> +
> +            v_addr = v.address
> +        else:
> +            v = gdb.Value(v)
> +
> +        cv_str = ""
> +        if v_addr is not None:
> +            v_addr = v_addr.dereference().reference_value()
> +            cur_entry[3] = v_addr
> +            cv = gdb.convenience_variable(self.convenience_name)
> +            if cv is not None and cv.address == v_addr.address and cv.type == v_addr.type:
> +                cv_str = " = \033[1;36m$" + self.convenience_name + "\033[0m"
> +
> +        try:
> +            pp = gdb.default_visualizer(v)
> +        except:
> +            pp = None
> +        if pp:
> +            valstr = None
> +            try:
> +                if expand is None and hasattr(pp, "children"):
> +                    expand = def_expand
> +                    cur_entry[0] = expand
> +                expand_str = "  "
> +                if expand is not None:
> +                    if expand:
> +                        expand_str = "- "
> +                    else:
> +                        expand_str = "+ "
> +                numstr += expand_str
> +
> +                if hasattr(pp, "to_string"):
> +                    valstr = pp.to_string()
> +                hint = None
> +                if hasattr(pp, "display_hint"):
> +                    hint = pp.display_hint()
> +                if valstr is not None:
> +                    valstr = value_string(valstr, hint)
> +                    valstr = octal_escape(valstr)
> +                    sp = valstr.split("\n")
> +                    (var_col_s, var_col_e, val_col_s, val_col_e) = val_cmp_color(prev_val, valstr)
> +                    if sym_not_init:
> +                        var_col_s, var_col_e = "\033[33m", "\033[0m"
> +                    self.lines.append(spaces + numstr + var_col_s + n + var_col_e + " = " + val_col_s + sp.pop(0) + val_col_e + cv_str)
> +                    self.line_names.append(n2)
> +                    for extra in sp:
> +                        self.lines.append(spaces + "    " + extra)
> +                        self.line_names.append(None)
> +                    cur_entry[1] = valstr
> +                else:
> +                    (var_col_s, var_col_e, val_col_s, val_col_e) = val_cmp_color(prev_val, False)
> +                    if sym_not_init:
> +                        var_col_s, var_col_e = "\033[33m", "\033[0m"
> +                    self.lines.append(spaces + numstr + var_col_s + n + var_col_e + cv_str)
> +                    self.line_names.append(n2)
> +
> +                if expand:
> +                    childs = None
> +                    if hasattr(pp, "children"):
> +                        childs = pp.children()
> +                    if childs:
> +                        if hint == "map":
> +                            count = 0
> +                            key_prev = None
> +                            for c in childs:
> +                                (nc, vc) = c
> +                                count += 1
> +                                fnc = ":".join([n2, nc])
> +                                if (count % 2) == 1:
> +                                    key_prev = None
> +                                    if is_string_instance(vc):
> +                                        key_prev = vc
> +                                    else:
> +                                        vc_check = vc
> +                                        if vc_check.type.code in [gdb.TYPE_CODE_REF, gdb.TYPE_CODE_RVALUE_REF]:
> +                                            vc_check = vc_check.referenced_value()
> +                                        key_pp = gdb.default_visualizer(vc_check)
> +                                        if key_pp:
> +                                            if hasattr(key_pp, "to_string") and not hasattr(key_pp, "children"):
> +                                                vc_check = key_pp.to_string()
> +                                                if vc_check is not None:
> +                                                    hint = None
> +                                                    if hasattr(key_pp, "display_hint"):
> +                                                        hint = key_pp.display_hint()
> +                                                    key_prev = value_string(vc_check, hint)
> +                                        else:
> +                                            t_check = vc_check.type.strip_typedefs()
> +                                            if t_check.code in [gdb.TYPE_CODE_ENUM, gdb.TYPE_CODE_INT, gdb.TYPE_CODE_FLT,
> +                                                    gdb.TYPE_CODE_BOOL, gdb.TYPE_CODE_COMPLEX, gdb.TYPE_CODE_DECFLOAT]:
> +                                                if fmt:
> +                                                    key_prev = vc_check.format_string(symbols=False, raw=True, format=fmt)
> +                                                else:
> +                                                    key_prev = vc_check.format_string(symbols=False, raw=True)
> +                                    if key_prev is not None and "\n" in key_prev:
> +                                        key_prev = None
> +                                    if key_prev is None:
> +                                        self.add_val("key", fnc, vc, inset + 1, 0, cur_vals, keep_prev, False, False, fmt, True, None)
> +                                else:
> +                                    if key_prev is not None:
> +                                        self.add_val("[" + str(key_prev) + "]", fnc, vc, inset + 1, 0, cur_vals, keep_prev, False, False, fmt, True, None)
> +                                    else:
> +                                        self.add_val("value", fnc, vc, inset + 1, 0, cur_vals, keep_prev, False, False, fmt, True, None)
> +                        else:
> +                            for c in childs:
> +                                (nc, vc) = c
> +                                fnc = ":".join([n2, nc])
> +                                self.add_val(nc, fnc, vc, inset + 1, 0, cur_vals, keep_prev, False, False, fmt, True, None)
> +            except:
> +                self.add_val(n, fn, None, inset, num, cur_vals, keep_prev, False, sym_not_init, fmt, False, str(sys.exc_info()[1]))
> +            return
> +
> +        t = v.type.strip_typedefs()
> +
> +        is_string = False
> +        if t.code in [gdb.TYPE_CODE_ARRAY, gdb.TYPE_CODE_PTR]:
> +            target_type = t.target().strip_typedefs()
> +            if target_type.code == gdb.TYPE_CODE_INT and (target_type.name == "char" or is_typedef_of(t.target(), "wchar_t")):
> +                is_string = True
> +        is_array = t.code in [gdb.TYPE_CODE_ARRAY, gdb.TYPE_CODE_RANGE]
> +        is_ptr = t.code == gdb.TYPE_CODE_PTR and not target_type.code in [gdb.TYPE_CODE_FUNC, gdb.TYPE_CODE_METHOD]
> +        name_add = ""
> +        if dyn_type and v.type.code == gdb.TYPE_CODE_STRUCT:
> +            try:
> +                if v.type != v.dynamic_type:
> +                    v = v.cast(v.dynamic_type)
> +                    name_add = " <" + v.type.name + ">"
> +            except:
> +                pass
> +        expand_str = "  "
> +        if is_ptr or is_array or t.code in [gdb.TYPE_CODE_STRUCT, gdb.TYPE_CODE_UNION]:
> +            if expand is None:
> +                expand = def_expand
> +                cur_entry[0] = expand
> +            if expand is not None:
> +                if expand:
> +                    expand_str = "- "
> +                else:
> +                    expand_str = "+ "
> +        numstr += expand_str
> +
> +        if is_string or t.code in [gdb.TYPE_CODE_PTR, gdb.TYPE_CODE_ENUM, gdb.TYPE_CODE_FUNC, gdb.TYPE_CODE_INT,
> +                gdb.TYPE_CODE_FLT, gdb.TYPE_CODE_STRING, gdb.TYPE_CODE_METHOD, gdb.TYPE_CODE_METHODPTR,
> +                gdb.TYPE_CODE_MEMBERPTR, gdb.TYPE_CODE_CHAR, gdb.TYPE_CODE_BOOL, gdb.TYPE_CODE_COMPLEX, gdb.TYPE_CODE_DECFLOAT]:
> +            try:
> +                if fmt and not is_string:
> +                    valstr = v.format_string(symbols=False, raw=True, address=True, format=fmt)
> +                else:
> +                    valstr = v.format_string(symbols=False, raw=True, address=True)
> +                valstr = octal_escape(valstr)
> +                (var_col_s, var_col_e, val_col_s, val_col_e) = val_cmp_color(prev_val, valstr)
> +                if sym_not_init:
> +                    var_col_s, var_col_e = "\033[33m", "\033[0m"
> +                self.lines.append(spaces + numstr + var_col_s + n + var_col_e + " = " + val_col_s + valstr + val_col_e + cv_str)
> +                self.line_names.append(n2)
> +                cur_entry[1] = valstr
> +            except:
> +                self.add_val(n, fn, None, inset, num, cur_vals, keep_prev, False, sym_not_init, fmt, False, str(sys.exc_info()[1]))
> +                return
> +        else:
> +            (var_col_s, var_col_e, val_col_s, val_col_e) = val_cmp_color(prev_val, False)
> +            if sym_not_init:
> +                var_col_s, var_col_e = "\033[33m", "\033[0m"
> +            self.lines.append(spaces + numstr + var_col_s + n + var_col_e + name_add + cv_str)
> +            self.line_names.append(n2)
> +        if t.code == gdb.TYPE_CODE_ENUM:
> +            return
> +
> +        if not expand:
> +            return
> +
> +        if is_array:
> +            (low, high) = t.range()
> +            for i in range(low, high + 1):
> +                nc = "[%d]" % i
> +                fnc = ":".join([n2, nc])
> +                self.add_val(nc, fnc, v[i], inset + 1, 0, cur_vals, keep_prev, False, False, fmt, True, None)
> +            return
> +
> +        if is_ptr:
> +            nc = "[0]"
> +            fnc = ":".join([n2, nc])
> +            try:
> +                v = v.dereference()
> +            except:
> +                v = None
> +            self.add_val(nc, fnc, v, inset + 1, 0, cur_vals, keep_prev, target_type.code != gdb.TYPE_CODE_PTR, False, fmt, True, None)
> +            return
> +
> +        fields = None
> +        if t.code in [gdb.TYPE_CODE_STRUCT, gdb.TYPE_CODE_UNION]:
> +            try:
> +                fields = v.type.fields()
> +            except:
> +                pass
> +        if fields:
> +            num = 1
> +            for f in fields:
> +                if not hasattr(f, "bitpos"):
> +                    continue
> +                try:
> +                    n = f.name
> +                except:
> +                    n = None
> +                if not n:
> +                    n = "<anonymous>"
> +                elif f.is_base_class:
> +                    n = "<" + n + ">"
> +                vf = v[f]
> +                fnc = ":".join([n2, n, "%d" % num])
> +                num += 1
> +                self.add_val(n, fnc, vf, inset + 1, 0, cur_vals, keep_prev, False, False, fmt, False, None)
> +
> +class VarNameValue(object):
> +    def __init__(self, sym, val, undecl, num, fmt, err):
> +        self.sym = sym
> +        self.val = val
> +        self.undecl = undecl
> +        self.num = num
> +        self.fmt = fmt
> +        self.err = err
> +
> +    def symbol(self):
> +        return self.sym
> +
> +    def value(self):
> +        return self.val
> +
> +    def undeclared(self):
> +        return self.undecl
> +
> +    def number(self):
> +        return self.num
> +
> +    def format(self):
> +        return self.fmt
> +
> +    def error(self):
> +        return self.err
> +
> +class LocalsWindow(VariableWindow):
> +    def __init__(self, win):
> +        super(LocalsWindow, self).__init__(win, "locals", "lv")
> +
> +    def variables(self):
> +        thread = gdb.selected_thread()
> +        thread_valid = thread and thread.is_valid()
> +        if thread_valid:
> +            frame = gdb.selected_frame()
> +            cur_line = frame.find_sal().line
> +            try:
> +                block = frame.block()
> +            except:
> +                block = None
> +            while block:
> +                if not block.is_global:
> +                    for symbol in block:
> +                        if symbol.is_argument or symbol.is_variable:
> +                            sym_not_init = symbol.is_variable and symbol.line > 0 and cur_line <= symbol.line
> +                            yield VarNameValue(symbol.name, symbol.value(frame), sym_not_init, 0, None, None)
> +                if block.function:
> +                    break
> +                block = block.superblock
> +
> +
> +gdb.register_window_type("locals", LocalsWindow)
> +
> +
> +def var_change_handler(event=None):
> +    global custom_windows
> +    for key, value in custom_windows.items():
> +        value.refill()
> +
> +gdb.events.before_prompt.connect(var_change_handler)
> +
> +
> +gdb.execute("tui new-layout locals {-horizontal src 2 locals 1} 2 status 0 cmd 1")
> +
> +gdb.execute("alias ll = layout locals")
> -- 
> 2.30.1
>