[RFA/commit] (Ada) -var-update crash for variable whose type is a reference to changeable

  Hello,

Consider the following variable, which is a string whose value
is not known at compile time, because it is the return value
from a function call (Get_Name):

   A : String := Get_Name;

If one tries to create a varobj for that variable, everything works
as expected:

    | (gdb) -var-create a * a
    | ^done,name="a",numchild="19",value="[19] \"Some kind of string\"",type="<ref> array (1 .. 19) of character",thread-id="1",has_more="0"

However, try then to request an update, regardless of whether the string
has changed or not, and we get a crash:

    | -var-update a
    | ~"/[...]/gdb/varobj.c:1379: internal-error: bool install_new_value(varobj*, value*, bool): Assertion `!value_lazy (var->value.get ())' failed.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "

When the varobj gets created (-var-create), the expression is evaluated
and transformed into a value. The debugging information describes our
variables as a reference to an array of characters, so our value has
the corresponding type. We then call varobj.c::install_new_value
to store that value inside our varobj, and we see that this function
pretty starts by determining weither our varobj is changeable, via:

    | changeable = varobj_value_is_changeable_p (var);

(where 'var' is the varobj we are building, and where the function
varobj_value_is_changeable_p simply dispatches to the Ada version
of this routine: ada_value_is_changeable_p).

At this point, the varobj doesn't have a value, yet, but it does
have a type which was provided by varobj_create a little bit before
install_new_value was called. So ada_value_is_changeable_p uses that
to determine whether or not our type is changeable.

Since our type is a reference to an array, and that the value of
such objects is displayed as if there weren't a reference, it means
that our object is changeable -- in other words, if an element of
the string changes, then the "value" field of the varobj will change
accordingly. But unfortunately, ada_value_is_changeable_p mistakenly
returns false, because it is missing the handling of reference types.

As a consequence of this, install_new_value doesn't feel it is
necessary to fetch the value's contents, as explained by the following
comment inside that function:

  /* The new value might be lazy.  If the type is changeable,
     that is we'll be comparing values of this type, fetch the
     value now.  Otherwise, on the next update the old value
     will be lazy, which means we've lost that old value.  */

This means that a lazy value gets installed inside our varobj
as a result of the mistake in ada_value_is_changeable_p.

Another important detail is that, after determining whether
our varobj is changeable or not, it then purposefully removes
the reference layer from our value:

  /* We are not interested in the address of references, and given
     that in C++ a reference is not rebindable, it cannot
     meaningfully change.  So, get hold of the real value.  */
  if (value)
    value = coerce_ref (value);

The consequence of those two facts on shows up only later, when
the user requests an update (-var-update). When doing so, GDB
evaluates the expression again into a value which is once more
a reference to a string, and then calls install_new_value again
to install the new value and report any changes. This time around,
the call to...

    | changeable = varobj_value_is_changeable_p (var);

... now gets a varobj which has a value, and one which had the reference
layer removed! So, this time, we classify the varobj correctly, and
say it is changeable. And because it is changeable, we then go into
the section of code in install_new_value which checks for changes,
where we need the varobj's value to not be lazy, as explained by
the comment we quoted above. That's what the assertion was about.

This patch fixes the issues by teaching ada_value_is_changeable_p
to ignore reference layers when evaluating a given varobj's type.

gdb/ChangeLog:

	* ada-varobj.c (ada_value_is_changeable_p): Add handling of
        TYPE_CODE_REF types.

gdb/testsuite/ChangeLog:

        * gdb.ada/mi_ref_changeable: New testcase.

Prior to this patch, this testcase reports 2 unresolved tests
(due to GDB hitting the internal error). With this patch, all
tests in this testcase pass.

Tested on x86_64-linux, no regression.
I will push in a week or two unless there are comments.

Thank you,

[RFA/commit] (Ada) -var-update crash for variable whose type is a reference to changeable

Commit Message

Comments

Patch