[PATCH][PR gdb/18706] Calculate size of array of stubbed type

[Hannes Domani]

 Am Dienstag, 28. April 2020, 17:10:29 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> On 2020-04-27 7:41 a.m., Hannes Domani via Gdb-patches wrote:
> > Sizes of stubbed types are calculated on demand in check_typedef, so the same
> > must also be done for arrays of stubbed types.
>
> For the uninitiated, can you please explain what's happening here, which types
> are involved, which ones are stubs, and what that means.  That would help me
> understand what happens here and be confident that the patch is correct.

Something like?:

A stubbed type is usually a structure that has only been forward declared, but
can also happen if the structure has a virtual function that's not inline in
the class definition.

For these stubbed types, the size must be recalculated once the full definition
is available.


> >
> > gdb/ChangeLog:
> >
> > 2020-04-27  Hannes Domani  <ssbssa@yahoo.de>
> >
> >     PR gdb/18706
> >     * gdbtypes.c (check_typedef): Calculate size of array of stubbed type.
> >
> > gdb/testsuite/ChangeLog:
> >
> > 2020-04-27  Hannes Domani  <ssbssa@yahoo.de>
> >
> >     PR gdb/18706
> >     * gdb.cp/stub-array-size.cc: New test.
> >     * gdb.cp/stub-array-size.exp: New file.
> >     * gdb.cp/stub-array-size.h: New test.
> >     * gdb.cp/stub-array-size2.cc: New test.
> > ---
> >  gdb/gdbtypes.c                          | 16 ++++++++++++++
> >  gdb/testsuite/gdb.cp/stub-array-size.cc  | 25 ++++++++++++++++++++++
> >  gdb/testsuite/gdb.cp/stub-array-size.exp | 27 ++++++++++++++++++++++++
> >  gdb/testsuite/gdb.cp/stub-array-size.h  | 21 ++++++++++++++++++
> >  gdb/testsuite/gdb.cp/stub-array-size2.cc | 22 +++++++++++++++++++
> >  5 files changed, 111 insertions(+)
> >  create mode 100644 gdb/testsuite/gdb.cp/stub-array-size.cc
> >  create mode 100644 gdb/testsuite/gdb.cp/stub-array-size.exp
> >  create mode 100644 gdb/testsuite/gdb.cp/stub-array-size.h
> >  create mode 100644 gdb/testsuite/gdb.cp/stub-array-size2.cc
> >
> > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> > index 157b3c5e61..5e9de2ccb1 100644
> > --- a/gdb/gdbtypes.c
> > +++ b/gdb/gdbtypes.c
> > @@ -2637,6 +2637,22 @@ check_typedef (struct type *type)
> >        TYPE_LENGTH (type) = TYPE_LENGTH (target_type);
> >        TYPE_TARGET_STUB (type) = 0;
> >      }
> > +      else if (TYPE_CODE (type) == TYPE_CODE_ARRAY
> > +          && TYPE_NFIELDS (type) == 1)
>
> Why is the `TYPE_NFIELDS (type) == 1` check necessary here?

This is de define of TYPE_INDEX_TYPE:
#define TYPE_INDEX_TYPE(type) TYPE_FIELD_TYPE (type, 0)

So I assumed I had to check if the field exists before using it.

This check is not always done (resolve_dynamic_array_or_string), sometimes
with gdb_assert (is_dynamic_type_internal), or like I did
(is_scalar_type_recursive).

I don't know which is the most correct way.


> > +    {
> > +      struct type *range_type = check_typedef (TYPE_INDEX_TYPE (type));
> > +      if (TYPE_CODE (range_type) == TYPE_CODE_RANGE
> > +          && has_static_range (TYPE_RANGE_DATA (range_type)))
>
> Is it possible to have an array type where the index type is not TYPE_CODE_RANGE?
>
> If it is not, then we should assert `TYPE_CODE (range_type) == TYPE_CODE_RANGE`.  If it
> can be of another type, what are the other possibilities?

Again here, this check is done differently in various functions.

Either gdb_assert (resolve_dynamic_range), or explicit check
(is_scalar_type_recursive).

If I see these different approaches, I tend to use the safest myself.


> > diff --git a/gdb/testsuite/gdb.cp/stub-array-size.exp b/gdb/testsuite/gdb.cp/stub-array-size.exp
> > new file mode 100644
> > index 0000000000..cb486cd459
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.cp/stub-array-size.exp
> > @@ -0,0 +1,27 @@
> > +# Copyright 2020 Free Software Foundation, Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > +
> > +# This file is part of the gdb testsuite.
>
>
> Please add a short introductory comment that explains what this intends to test.

Like?:

Test size of arrays of stubbed types (structures where the full definition is
not immediately available).


Hannes