This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] MI: lvalues and variable_editable
> > > 1. In varobj_editable_p you call gdb_evaluate_expression, and I believe
> > > this to be wrong. We call gdb_evaluate_expression when we create
> > > varobj, and it either succeeds, eventually setting varobj->value to
> > > something, or it does not. There's no point to call
> > > gdb_evaluate_expression again.
> >
> > If gdb_evaluate_expression fails in varobj_create, a variable object is
> > still created, but just with an undefined value. It needs to be called to
> > get value for VALUE_LVAL.
>
> Why would a second call to gdb_evaluate_expression succeed? We already tried
> to evaluate expression, we failed, varobj has no value whatsoever. So,
> I'd say we should refuse to assign anything in this case.
The second call succeeds if the first one did (when the varobj does have
a value) - but we seem to be going backwards.
> > > Further,
> > > in varobj_create, gdb_evaluate_expression is called in specific frame,
> > > and varobj_editable_p calls it in current frame.
> >
> > The current frame should be the frame in which the variable object is defined.
> > Can you explain why that's a problem?
>
> Here's the code in question:
>
> /* When the frame is different from the current frame,
> we must select the appropriate frame before parsing
> the expression, otherwise the value will not be current.
> Since select_frame is so benign, just call it for all cases. */
> if (fi != NULL)
> {
> var->root->frame = get_frame_id (fi);
> old_fi = get_selected_frame (NULL);
> select_frame (fi);
> }
>
> /* We definitively need to catch errors here.
> If evaluate_expression succeeds we got the value we wanted.
> But if it fails, we still go on with a call to evaluate_type() */
> if (!gdb_evaluate_expression (var->root->exp, &value))
>
> So it seems to be varobj is not necessary defined in the current frame.
When it's created it can be in a different frame according to the syntax.
However, in Emacs I only create watch expressions in the selected frame (the
manual suggests that "*" sets the value in the current frame but there
are many anomalies like this).
Likewise I think variable assignment should only be done in the selected frame.
Perhaps GDB should only allow this scenario, I don't know. I have used this
patch for several months now without a problem and I'd rather focus on real
use cases than hypothetical pathological ones.
>...
> > > 4. I don't think your test actually tests that the 'editable' attribute
> > > comes out as 'false'.
> >
> > I'm not sure what to say. It shows that if you try to assign a value to a
> > cast GDB says "Variable object is not editable".
> >
> > The error message for a variable object of a cast used to be:
> >
> > &"mi_cmd_var_assign: Could not assign expression to variable object\n"
> > ^error,msg="mi_cmd_var_assign: Could not assign expression to variable object"
> > (gdb)
>
> Is this test failing with current GDB, and passing after your change?
The "not editable" message is new for this case.
> I think
> GDB will refuse to assign value to rvalue anyway -- at least here's what I
> get without your patch:
>
> (gdb)
> -var-create V * (float)argc
> ^done,name="V",numchild="0",value="0",type="float"
> (gdb)
> -var-assign V 10
> &"mi_cmd_var_assign: Could not assign expression to variable object\n"
> ^error,msg="mi_cmd_var_assign: Could not assign expression to variable object"
> (gdb)
That's what I've just said.
> > I could add another test for -var-show-attributes which will now give:
> >
> > ^done,attr="noneditable"
> >
> > for a cast but I never use this command.
>
> I think the biggest value of this patch is that we get this attribute,
> *before* trying to assign the value, so yes, I think a test for this
> specific behaviour would be great.
There's no before or after. It would basically be the same test.
--
Nick http://www.inet.net.nz/~nickrob