Fix segfault when invoking -var-info-path-expression on a dynamic varobj

Message ID 20180604131503.31945-1-jan.vrany@fit.cvut.cz
State New, archived
Headers

Commit Message

Jan Vrany June 4, 2018, 1:15 p.m. UTC
  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
  

Comments

Jan Vrany June 18, 2018, 7:14 a.m. UTC | #1
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);
>      }
>
  
Simon Marchi June 19, 2018, 12:42 a.m. UTC | #2
On 2018-06-04 09:15 AM, 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.

Hi Jan,

Thanks for the patch.  I think we should also handle the indirect children
of dynamic objects.  When trying this test program:

#include <vector>
struct foo { int a; };
int main() {
    std::vector<foo> vec;
    vec.push_back({ 1 });
    return 0;
}

With the following MI commands:

-enable-pretty-printing
b 6
r
-var-create - * vec
-var-list-children var1
-var-list-children var1.[0]
-var-list-children var1.[0].public
-var-info-path-expression var1.[0].public.a

I get this:

~"/home/simark/src/binutils-gdb/gdb/varobj.c:969: internal-error: const char* varobj_get_path_expr(const varobj*): Assertion `!varobj_is_dynamic_p(var->parent)' failed..."

I noted some small formatting nits below.

> 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.

You are missing a slash here (and in the ChangeLog file itself).

> 	* 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  */

Finish the sentence with a period.

> 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. */

Two spaces after the final period (before the */).

> +      gdb_assert (!varobj_is_dynamic_p(var->parent));

Space after varobj_is_dynamic_p.

Thanks,

Simon
  
Jan Vrany June 19, 2018, 8:59 a.m. UTC | #3
On Mon, 2018-06-18 at 20:42 -0400, Simon Marchi wrote:
> On 2018-06-04 09:15 AM, 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.
> 
> Hi Jan,
> 
> Thanks for the patch.  I think we should also handle the indirect
> children
> of dynamic objects.  When trying this test program:
> 
> #include <vector>
> struct foo { int a; };
> int main() {
>     std::vector<foo> vec;
>     vec.push_back({ 1 });
>     return 0;
> }
> 
> With the following MI commands:
> 
> -enable-pretty-printing
> b 6
> r
> -var-create - * vec
> -var-list-children var1
> -var-list-children var1.[0]
> -var-list-children var1.[0].public
> -var-info-path-expression var1.[0].public.a
> 
> I get this:
> 
> ~"/home/simark/src/binutils-gdb/gdb/varobj.c:969: internal-error:
> const char* varobj_get_path_expr(const varobj*): Assertion
> `!varobj_is_dynamic_p(var->parent)' failed..."
> 
Hi Simon, 

good catch! Will fix that. 


> I noted some small formatting nits below.

Thanks! I appreciate the patience you have with me.
I will address your
comments and send new version.

Best, Jan

> 
> > 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.
> 
> You are missing a slash here (and in the ChangeLog file itself).
> 
> > 	* 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  */
> 
> Finish the sentence with a period.
> 
> > 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. */
> 
> Two spaces after the final period (before the */).
> 
> > +      gdb_assert (!varobj_is_dynamic_p(var->parent));
> 
> Space after varobj_is_dynamic_p.
> 
> Thanks,
> 
> Simon
  

Patch

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/licenses/>.  */
+
+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\".*child=\{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);
     }