This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [ob] unbreak MI
On Tue, 2007-11-27 at 10:00 +0300, Vladimir Prus wrote:
> On Tuesday 27 November 2007 09:27:37 Nick Roberts wrote:
> > > > Generally, with a NULL pointer, or and address that can't be dereferenced,
> > > > MI prints out the value field as value="".
> > > >
> > > > What is the problem in this case? Why isn't the right fix to add a
> > > > check_typedef somewhere?
> > >
> > > check_typedef? The original problem was that check_typedef was getting
> > > called on NULL pointer, so adding more check_typedef calls won't help.
> > > Probably:
> > >
> > > if (!gdb_type)
> > > ui_out_field_string (uiout, "value", "");
> > > else if (mi_print_value_p (gdb_type, print_values))
> > > ui_out_field_string (uiout, "value", varobj_get_value (var));
> > >
> > > is the right logic?
> >
> > It's probably the right logic, but it seems to cure the symptom rather than the
> > cause. What I mean't, I guess, was where/how does check_typedef is get passed
> > a NULL pointer? And can't that call be conditioned (i.e. "add a *check* to
> > check_typedef") , e.g., something like:
> >
> > if (!gdb_type)
> > check_typedef (gdb_type)
>
> Just look at mi_print_value_p, and you'll see a call to check_typedef. Actually,
> the code previously looked like:
>
> if (type != NULL)
> type = check_typedef (type);
>
> It was changed in revision 1.38, with the following comment:
>
> 2007-08-28 Michael Snyder <msnyder@access-company.com>
>
> * mi/mi-cmd-var.c (mi_print_value_p): No longer necessary to
> check for null before calling check_typedef.
>
> However, apparently check_typedef still crashes when passed NULL,
> and it can be passed NULL.
It doesn't crash -- it calls assert, therefore abort.
The debate at the time was whether it made more sense
to check for null before every call to check_typedef,
or simply to have check_typedef do the check for null
itself.
Makeing a change in one place seemed easier than makeing
a change in 100's of places.
And it's not clear that check_typedef can do anything
intelligent to recover if a null pointer is passed ---
hence the abort.
Probably calling error rather than abort would be acceptable.
>
> The original code, in fact, was in error too, because of this:
>
> return (TYPE_CODE (type) != TYPE_CODE_ARRAY
> && TYPE_CODE (type) != TYPE_CODE_STRUCT
> && TYPE_CODE (type) != TYPE_CODE_UNION);
>
> This will crash if 'type' is NULL. Testsuite fails to detect this because presently
> type is NULL only for C++ pseudo-fields ('public'/'private') and the code
> above is only executed for --simple-values.
>
> Does this clarify things?
>
> - Volodya