[PING] [PATCH] Fix segfault when invoking -var-info-path-expression on a dynamic varobj

Jan Vrany
2018-06-18T07:14:26Z

Polite ping.

On Mon, 2018-06-04 at 14:15 +0100, Jan Vrany wrote:
> Invoking -var-info-path-expression on a dynamic varobj lead either in
> wrong
> (nonsense) result or to a segmentation fault in
> cplus_describe_child().
> This was caused by the fact that varobj_get_path_expr() called
> cplus_path_expr_of_child() ignoring the fact the parent of the
> variable
> is dynamic. Then, cplus_describe_child() accessed the underlaying C
> type
> members by index, causing (i) either wrong (nonsense) expression
> being
> returned (since dynamic child may be completely arbibtrary value)
> or (ii) segmentation fault (in case the index higher than number of
> underlaying C type members.
>
> This fixes the problem by checking whether a varobj is a child of a
> dynamic
> varobj and, if so, reporting an error to MI consumer as described in
> the documentation.
>
> gdb/ChangeLog:
>
> * mi/mi-cmd-var.c: Report an error if varobj is a child of
> dynamic
> varobj as stated in documentation.
> * varobj.c: New assert.
>
> gdb/testsuite/Changelog:
>
> * gdb.pythonpy-mi-var-info-path-expression.c: New file.
> * gdb.python/py-mi-var-info-path-expression.py: New file.
> * gdb.python/py-mi-var-info-path-expression.exp: New file.
> ---
> gdb/ChangeLog | 6 ++
> gdb/mi/mi-cmd-var.c | 5 ++
> gdb/testsuite/ChangeLog | 6 ++
> .../py-mi-var-info-path-expression.c | 40 ++++++++++
> .../py-mi-var-info-path-expression.exp | 77
> +++++++++++++++++++
> .../py-mi-var-info-path-expression.py | 45 +++++++++++
> gdb/varobj.c | 6 +-
> 7 files changed, 184 insertions(+), 1 deletion(-)
> create mode 100644 gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.c
> create mode 100644 gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.exp
> create mode 100644 gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.py
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index a292f4c056..fc5edb9a9e 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-06-04 Jan Vrany <jan.vrany@fit.cvut.cz>
> +
> + * mi/mi-cmd-var.c: Report an error if varobj is a child of
> dynamic
> + varobj as stated in documentation.
> + * varobj.c: New assert.
> +
> 2018-06-04 Pedro Alves <palves@redhat.com>
>
> * darwin-nat.c (darwin_ops): Delete.
> diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
> index 38c59c7e66..f278281023 100644
> --- a/gdb/mi/mi-cmd-var.c
> +++ b/gdb/mi/mi-cmd-var.c
> @@ -438,6 +438,11 @@ mi_cmd_var_info_path_expression (const char
> *command, char **argv, int argc)
>
> /* Get varobj handle, if a valid var obj name was specified. */
> var = varobj_get_handle (argv[0]);
> +
> + /* -var-info-path-expression is currently not valid for children
> of
> + a dynamic varobj */
> + if (var->parent != nullptr && varobj_is_dynamic_p (var->parent))
> + error (_("Invalid variable object (child of a dynamic
> varobj)"));
>
> const char *path_expr = varobj_get_path_expr (var);
>
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 8cef172a26..d7ac7b0a62 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-06-04 Jan Vrany <jan.vrany@fit.cvut.cz>
> +
> + * gdb.pythonpy-mi-var-info-path-expression.c: New file.
> + * gdb.python/py-mi-var-info-path-expression.py: New file.
> + * gdb.python/py-mi-var-info-path-expression.exp: New file.
> +
> 2018-06-01 Joel Brobecker <brobecker@adacore.com>
>
> * gdb.ada/bp_fun_addr: New testcase.
> diff --git a/gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.c b/gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.c
> new file mode 100644
> index 0000000000..a4a5f37178
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-mi-var-info-path-expression.c
> @@ -0,0 +1,40 @@
> +/* Implementation of the GDB variable objects API.
> +
> + Copyright (C) 1999-2018 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/license
> s/>. */
> +
> +struct _cons
> +{
> + struct _cons *slots[2];
> +};
> +
> +#define nil ((struct _cons*)0);
> +
> +int
> +main ()
> +{
> + struct _cons c1, c2, c3;
> +
> + c1.slots[0] = nil;
> + c1.slots[1] = &c2;
> +
> + c2.slots[0] = nil;
> + c2.slots[1] = &c3;
> +
> + c3.slots[0] = nil;
> + c3.slots[1] = nil;
> +
> + return 0; /* next line */
> +}
> diff --git a/gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.exp b/gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.exp
> new file mode 100644
> index 0000000000..f4aece9156
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-mi-var-info-path-expression.exp
> @@ -0,0 +1,77 @@
> +# Copyright (C) 2008-2018 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
> />.
> +
> +# Tests whether -var-info-path-expression fails as documented when
> +# invoked on a dynamic varobj.
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if {[mi_gdb_start]} {
> + continue
> +}
> +
> +#
> +# Start here
> +#
> +standard_testfile
> +
> +if {[gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable
> {debug}] != "" } {
> + return -1
> +}
> +
> +mi_gdb_test "source ${srcdir}/${subdir}/${testfile}.py" \
> + ".*\\^done" \
> + "load python file"
> +
> +mi_gdb_test "-enable-pretty-printing" \
> + "\\^done" \
> + "-enable-pretty-printing"
> +
> +mi_gdb_test "set python print-stack full" \
> + ".*\\^done" \
> + "set python print-stack full"
> +
> +
> +mi_run_to_main
> +
> +
> +mi_continue_to_line [gdb_get_line_number "next line" ${srcfile}] \
> + "step to breakpoint"
> +
> +mi_gdb_test "-var-create c1 * &c1" \
> + "\\^done.*" \
> + "-var-create c1 * &c1"
> +
> +mi_gdb_test "-var-info-path-expression c1" \
> + "\\^done,path_expr=\"&c1\"" \
> + "-var-info-path-expression c1"
> +
> +mi_gdb_test "-var-list-children c1" \
> + "\\^done,numchild=\"2\",children=.child=\{name=\"c1.car\".*child=\
> {name=\"c1.cdr\".*" \
> + "-var-list-children c1"
> +
> +mi_gdb_test "-var-info-path-expression c1.cdr" \
> + "\\^error,msg=\".*\"" \
> + "-var-info-path-expression c1.cdr"
> +
> +mi_gdb_test "-var-list-children c1.cdr" \
> + "\\^done,numchild=\"2\",children=.child=\{name=\"c1.cdr.car\".*chi
> ld=\{name=\"c1.cdr.cdr\".*" \
> + "-var-list-children c1.cdr"
> +
> +mi_gdb_test "-var-info-path-expression c1.cdr.cdr" \
> + "\\^error,msg=\".*\"" \
> + "-var-info-path-expression c1.cdr.cdr"
> diff --git a/gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.py b/gdb/testsuite/gdb.python/py-mi-var-info-path-
> expression.py
> new file mode 100644
> index 0000000000..39cb5e2e4a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-mi-var-info-path-expression.py
> @@ -0,0 +1,45 @@
> +# Copyright (C) 2008-2018 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 gdb.types
> +
> +class cons_pp(object):
> + def __init__(self, val):
> + self._val = val
> +
> + def to_string(self):
> + if int(self._val) == 0:
> + return "nil"
> + else:
> + return "(...)"
> +
> + def children(self):
> + if int(self._val) == 0:
> + return []
> + else:
> + return [
> + ('car' , self._val["slots"][0]),
> + ('cdr' , self._val["slots"][1])
> + ]
> +
> +def cons_pp_lookup(val):
> + if str(val.type) == 'struct _cons *':
> + return cons_pp(val)
> + else:
> + return None
> +
> +del gdb.pretty_printers[1:]
> +gdb.pretty_printers.append(cons_pp_lookup)
> \ No newline at end of file
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index a0df485ae9..87bfd44549 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -962,9 +962,13 @@ varobj_get_path_expr (const struct varobj *var)
> /* For root varobjs, we initialize path_expr
> when creating varobj, so here it should be
> child varobj. */
> - struct varobj *mutable_var = (struct varobj *) var;
> gdb_assert (!is_root_p (var));
>
> + /* Computation of full rooted expression for children of
> dynamic
> + varobjs is not supported. */
> + gdb_assert (!varobj_is_dynamic_p(var->parent));
> +
> + struct varobj *mutable_var = (struct varobj *) var;
> mutable_var->path_expr = (*var->root->lang_ops-
> >path_expr_of_child) (var);
> }
>