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

[Simon Marchi]

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