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

[Simon Marchi <simark at simark dot ca>]

On 2018-06-28 05:31, Jan Vrany wrote:
> > diff --git a/gdb/varobj.c b/gdb/varobj.c
> > index f2c10ddc57ff..e601cf0b9780 100644
> > --- a/gdb/varobj.c
> > +++ b/gdb/varobj.c
> > @@ -948,6 +948,9 @@ varobj_get_path_expr_parent (const struct varobj
> > *var)
> >    while (!is_root_p (parent) && !is_path_expr_parent (parent))
> >      parent = parent->parent;
> >
> > +  if (varobj_is_dynamic_p (parent))
> > +    error (_("Invalid variable object (child of a dynamic
> > varobj)"));
> > +
> >    return parent;
> >  }
>
> No, I have not considered this - still learning internals. Generally
> I prefer to validate input as soon as possible rather than go on and
> fail later but I agree it's a matter of taste.

The counter argument would be to avoid spreading the "knowledge" of the 
varobj internals in the MI/frontend code.  Also, if other parts of GDB 
want to find out the path expression parent of a varobj, the same case 
will be properly handled.

> However, varobj_get_path_expr_parent() is indeed much better place for
> the assert (the one that failed to compile :), something like:
>
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 02441410e8..050c240bef 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -948,6 +948,10 @@ varobj_get_path_expr_parent (const struct varobj
> *var)
>    while (!is_root_p (parent) && !is_path_expr_parent (parent))
>      parent = parent->parent;
>
> +  /* Computation of full rooted expression for children of dynamic
> +     varobjs is not supported.  */
> +  gdb_assert (!varobj_is_dynamic_p (parent));
> +
>    return parent;
>  }
>
> What do you think? I'll send a new version once we decide.

Personally, I'd prefer the check in varobj_get_path_expr_parent.

Thanks,

Simon