DWARF array bounds missing from C++ array definitions
diff mbox series

Message ID orv9txvpdm.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • DWARF array bounds missing from C++ array definitions
Related show

Commit Message

Alexandre Oliva Sept. 12, 2019, 9:24 a.m. UTC
A variable redeclaration or definition that provides additional type
information for it, e.g. outermost array bounds, is not reflected in
the debug information for the variable.  With this patch, the debug
info of the variable specialization gets a type attribute with the
adjusted type.

This patch affects mostly only array bounds.  However, when the
symbolic type used in a declaration and in a definition are different,
although they refer to the same type, debug information will end up
(correctly?) naming different symbolic types in the specification and
the definition.  Also, when a readonly declaration of an array loses
the readonly flag at the definition because of the initializer, the
definition may end up referencing a type while the specification
refers to a const-qualified version of that type.  If the type of the
variable is already const-qualified, e.g. an array of a const type,
the difference is meaningless.

Regstrapped on x86_64-linux-gnu.  Ok to install?


for  gcc/ChangeLog

	* dwarf2out.c (completing_type_p): New.
	(gen_variable_die): Use it.

for  gcc/testsuite/ChangeLog

	* gcc.dg/debug/dwarf2/array-0.c: New.
	* gcc.dg/debug/dwarf2/array-1.c: New.
	* gcc.dg/debug/dwarf2/array-2.c: New.
	* gcc.dg/debug/dwarf2/array-3.c: New.
	* g++.dg/debug/dwarf2/array-0.C: New.
	* g++.dg/debug/dwarf2/array-1.C: New.
	* g++.dg/debug/dwarf2/array-2.C: New.  Based on libstdc++-v3's
	src/c++98/pool_allocator.cc:__pool_alloc_base::_S_heap_size.
	* g++.dg/debug/dwarf2/array-3.C: New.  Based on
	gcc's config/i386/i386-features.c:xlogue_layout::s_instances.
	* g++.dg/debug/dwarf2/array-4.C: New.
---
 gcc/dwarf2out.c                             |   31 ++++++++++++++++++++++++++-
 gcc/testsuite/g++.dg/debug/dwarf2/array-0.C |   13 +++++++++++
 gcc/testsuite/g++.dg/debug/dwarf2/array-1.C |   13 +++++++++++
 gcc/testsuite/g++.dg/debug/dwarf2/array-2.C |   15 +++++++++++++
 gcc/testsuite/g++.dg/debug/dwarf2/array-3.C |   20 +++++++++++++++++
 gcc/testsuite/g++.dg/debug/dwarf2/array-4.C |   16 ++++++++++++++
 gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c |   10 +++++++++
 gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c |   10 +++++++++
 gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c |    8 +++++++
 gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c |    8 +++++++
 10 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-0.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-1.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-2.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-3.C
 create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-4.C
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c

Comments

Richard Biener Sept. 12, 2019, 10:18 a.m. UTC | #1
On Thu, Sep 12, 2019 at 11:24 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> A variable redeclaration or definition that provides additional type
> information for it, e.g. outermost array bounds, is not reflected in
> the debug information for the variable.  With this patch, the debug
> info of the variable specialization gets a type attribute with the
> adjusted type.
>
> This patch affects mostly only array bounds.  However, when the
> symbolic type used in a declaration and in a definition are different,
> although they refer to the same type, debug information will end up
> (correctly?) naming different symbolic types in the specification and
> the definition.  Also, when a readonly declaration of an array loses
> the readonly flag at the definition because of the initializer, the
> definition may end up referencing a type while the specification
> refers to a const-qualified version of that type.  If the type of the
> variable is already const-qualified, e.g. an array of a const type,
> the difference is meaningless.
>
> Regstrapped on x86_64-linux-gnu.  Ok to install?

Is this PR91507?  How do you get around the gdb issue?

Thanks,
Richard.

>
> for  gcc/ChangeLog
>
>         * dwarf2out.c (completing_type_p): New.
>         (gen_variable_die): Use it.
>
> for  gcc/testsuite/ChangeLog
>
>         * gcc.dg/debug/dwarf2/array-0.c: New.
>         * gcc.dg/debug/dwarf2/array-1.c: New.
>         * gcc.dg/debug/dwarf2/array-2.c: New.
>         * gcc.dg/debug/dwarf2/array-3.c: New.
>         * g++.dg/debug/dwarf2/array-0.C: New.
>         * g++.dg/debug/dwarf2/array-1.C: New.
>         * g++.dg/debug/dwarf2/array-2.C: New.  Based on libstdc++-v3's
>         src/c++98/pool_allocator.cc:__pool_alloc_base::_S_heap_size.
>         * g++.dg/debug/dwarf2/array-3.C: New.  Based on
>         gcc's config/i386/i386-features.c:xlogue_layout::s_instances.
>         * g++.dg/debug/dwarf2/array-4.C: New.
> ---
>  gcc/dwarf2out.c                             |   31 ++++++++++++++++++++++++++-
>  gcc/testsuite/g++.dg/debug/dwarf2/array-0.C |   13 +++++++++++
>  gcc/testsuite/g++.dg/debug/dwarf2/array-1.C |   13 +++++++++++
>  gcc/testsuite/g++.dg/debug/dwarf2/array-2.C |   15 +++++++++++++
>  gcc/testsuite/g++.dg/debug/dwarf2/array-3.C |   20 +++++++++++++++++
>  gcc/testsuite/g++.dg/debug/dwarf2/array-4.C |   16 ++++++++++++++
>  gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c |   10 +++++++++
>  gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c |   10 +++++++++
>  gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c |    8 +++++++
>  gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c |    8 +++++++
>  10 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-0.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-1.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-2.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-3.C
>  create mode 100644 gcc/testsuite/g++.dg/debug/dwarf2/array-4.C
>  create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c
>  create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c
>  create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index c359c2d4af981..ad533c14d2480 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -23687,6 +23687,33 @@ local_function_static (tree decl)
>      && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL;
>  }
>
> +/* Return true iff DECL completes (overrides) the type of OLD_DIE
> +   within CONTEXT_DIE.  */
> +
> +static bool
> +completing_type_p (tree decl, dw_die_ref old_die, dw_die_ref context_die)
> +{
> +  tree type = TREE_TYPE (decl);
> +  int cv_quals;
> +
> +  if (decl_by_reference_p (decl))
> +    {
> +      type = TREE_TYPE (type);
> +      cv_quals = TYPE_UNQUALIFIED;
> +    }
> +  else
> +    cv_quals = decl_quals (decl);
> +
> +  dw_die_ref type_die = modified_type_die (type,
> +                                          cv_quals | TYPE_QUALS (type),
> +                                          false,
> +                                          context_die);
> +
> +  dw_die_ref old_type_die = get_AT_ref (old_die, DW_AT_type);
> +
> +  return type_die != old_type_die;
> +}
> +
>  /* Generate a DIE to represent a declared data object.
>     Either DECL or ORIGIN must be non-null.  */
>
> @@ -23939,7 +23966,9 @@ gen_variable_die (tree decl, tree origin, dw_die_ref context_die)
>           && !DECL_ABSTRACT_P (decl_or_origin)
>           && variably_modified_type_p (TREE_TYPE (decl_or_origin),
>                                        decl_function_context
> -                                                       (decl_or_origin))))
> +                                      (decl_or_origin)))
> +      || (old_die && specialization_p
> +         && completing_type_p (decl_or_origin, old_die, context_die)))
>      {
>        tree type = TREE_TYPE (decl_or_origin);
>
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-0.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-0.C
> new file mode 100644
> index 0000000000000..a3458bd0d32a4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-0.C
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +struct S
> +{
> +  static int array[42];
> +};
> +
> +int S::array[42];
> +
> +/* Verify that we get only one DW_TAG_subrange_type with a
> +   DW_AT_upper_bound.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-1.C
> new file mode 100644
> index 0000000000000..e8fd6f8ffea56
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-1.C
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +struct S
> +{
> +  static int array[];
> +};
> +
> +int S::array[42];
> +
> +/* Verify that we get two DW_TAG_subrange_type, only one of which with
> +   a DW_AT_upper_bound.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 4 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-2.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-2.C
> new file mode 100644
> index 0000000000000..dd17812043898
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-2.C
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +struct S
> +{
> +  typedef int i_t;
> +  static i_t array[42];
> +};
> +
> +int S::array[42];
> +
> +/* Verify that we get two DW_TAG_subrange_type (plus abbrev), and two
> +   DW_AT_upper_bound, because a different symbolic name is used for
> +   the array element type.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 3 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 2 } } */
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-3.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-3.C
> new file mode 100644
> index 0000000000000..8db6133b76585
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-3.C
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +struct S
> +{
> +  S() {}
> +  ~S() {}
> +  static const S array[2];
> +};
> +
> +const S S::array[2] = { S(), S() };
> +
> +/* Verify that we get only one DW_TAG_subrange_type (plus the abbrev),
> +   and one DW_AT_upper_bound (non-abbrev), because the array
> +   definition loses the readonly wrapper for the array type because of
> +   the dynamic initializers.  The const types are 4: S, S*, int, and
> +   S[4], plus the abbrev.  A const version of S[4] doesn't make sense,
> +   but we output it.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_const_type" 5 } } */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-4.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-4.C
> new file mode 100644
> index 0000000000000..6b3f546c1b583
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-4.C
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +struct S
> +{
> +  S() {}
> +  ~S() {}
> +};
> +
> +const S array[2] = { S(), S() };
> +
> +/* Like array-3, but with a non-member array without a separate
> +   declaration, to check that we don't issue the nonsensical
> +   DW_TAG_const_type used by the member array declaration there.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_const_type" 4 } } */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c
> new file mode 100644
> index 0000000000000..b06392e04a2db
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +extern int array[42];
> +
> +int array[42];
> +
> +/* Verify that we get only one DW_TAG_subtrange_type (plus abbrev),
> +   with a DW_AT_upper_bound.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c
> new file mode 100644
> index 0000000000000..ad8f466ef08ad
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +extern int array[];
> +
> +int array[42];
> +
> +/* Verify that we get two DW_TAG_subtrange_type (each with an abbrev),
> +   but only one DW_AT_upper_bound.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 4 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c
> new file mode 100644
> index 0000000000000..5d1606f0889fc
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +int array[42];
> +
> +/* Verify that we get only one DW_TAG_subtrange_type (plus abbrev)
> +   with DW_AT_upper_bound.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c
> new file mode 100644
> index 0000000000000..077a62ef9a3bf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-2 -dA" } */
> +int array[] = { 0, 1, 2 };
> +
> +/* Verify that we get only one DW_TAG_subtrange_type (plus abbrev)
> +   with DW_AT_upper_bound.  */
> +/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
> +/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
>
>
> --
> Alexandre Oliva, freedom fighter  he/him   https://FSFLA.org/blogs/lxo
> Be the change, be Free!       FSF.org & FSF Latin America board member
> GNU Toolchain Engineer                        Free Software Evangelist
> Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara
Alexandre Oliva Sept. 12, 2019, 11:36 a.m. UTC | #2
On Sep 12, 2019, Richard Biener <richard.guenther@gmail.com> wrote:

> Is this PR91507?

Looks like it.  Interesting, I've had this patch sitting in my tree
since early June, waiting for the additional verification I completed
last night.  That was long before the PR was filed.

> How do you get around the gdb issue?

I was not even aware of one.  I focused on preserving at what I regarded
as the right place the information we currently dropped on the floor,
figuring if any consumers wouldn't take the type information from the
definition as overriding/completing that of the specification, they'd
eventually be adjusted to do so.


The approach I chose was to add the completion type to the definition,
not to the specification.  I figured leaving the specification alone,
reflecting the information available at its source location, and
providing the complete type information at the source location that
supplies it, was the right thing to do, regardless of whatever debug
information consumers were able to do with that information.

There's room for dispute as to the correctness of this approach,
however.  Someone might argue that we should have a separate (IMHO
excessive) completion non-defining declaration, pointing back to the
initial incomplete declaration with a DW_AT_specification, and perhaps
to omit the incomplete type from the incomplete specification, though
that doesn't seem to be in line with the common practice of overriding
declaration coordinates in the definition.
Richard Biener Sept. 12, 2019, 12:06 p.m. UTC | #3
On Thu, Sep 12, 2019 at 1:36 PM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Sep 12, 2019, Richard Biener <richard.guenther@gmail.com> wrote:
>
> > Is this PR91507?
>
> Looks like it.  Interesting, I've had this patch sitting in my tree
> since early June, waiting for the additional verification I completed
> last night.  That was long before the PR was filed.
>
> > How do you get around the gdb issue?
>
> I was not even aware of one.  I focused on preserving at what I regarded
> as the right place the information we currently dropped on the floor,
> figuring if any consumers wouldn't take the type information from the
> definition as overriding/completing that of the specification, they'd
> eventually be adjusted to do so.
>
>
> The approach I chose was to add the completion type to the definition,
> not to the specification.  I figured leaving the specification alone,
> reflecting the information available at its source location, and
> providing the complete type information at the source location that
> supplies it, was the right thing to do, regardless of whatever debug
> information consumers were able to do with that information.

Yes, I agree, this is what my patch in the PR does as well, albeit
just in the place where we add the link to the specification DIE
and the unconditionally if there's a disagreeing type DIE (we
force a type DIE earlier in the caller).  Your new predicate looks
a bit excessive here given it builds the type again?

But that's implementation detail I guess.

So I'm obviously fine with your patch and I guess if we independently
arrive at this solution that answers my question what "correct DWARF"
is by a majority decision ;)

So - maybe we can have the patch a bit cleaner by adding
a flag to add_type_attribute saying we only want it if it's
different from that already present (on the specification DIE)?

Thanks,
Richard.

> There's room for dispute as to the correctness of this approach,
> however.  Someone might argue that we should have a separate (IMHO
> excessive) completion non-defining declaration, pointing back to the
> initial incomplete declaration with a DW_AT_specification, and perhaps
> to omit the incomplete type from the incomplete specification, though
> that doesn't seem to be in line with the common practice of overriding
> declaration coordinates in the definition.
>
> --
> Alexandre Oliva, freedom fighter  he/him   https://FSFLA.org/blogs/lxo
> Be the change, be Free!       FSF.org & FSF Latin America board member
> GNU Toolchain Engineer                        Free Software Evangelist
> Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara
Alexandre Oliva Sept. 12, 2019, 11:31 p.m. UTC | #4
On Sep 12, 2019, Richard Biener <richard.guenther@gmail.com> wrote:

> Your new predicate looks a bit excessive here given it builds the type
> again?

Err, there seems to be some misunderstanding here.  The predicate avoids
outputting a type for the definition if it's the same DIE that would go
in the specification.  Now, when it's a different DIE, sometimes it
might still refer to the same type, as in array-2.C, but I think that's
not just acceptable, it's desirable, for it reflects the different ways
used to denote the same type.

Before posting the patch, I added an inform() to the case in which
completing_type_p would return true (bringing about a debug info
change), and reviewed all of the messages in a bootstrap.  The only ones
that weren't just adding array element count were those that inspired
array-2.C and array-3.C.

> So I'm obviously fine with your patch and I guess if we independently
> arrive at this solution that answers my question what "correct DWARF"
> is by a majority decision ;)

Good.  Once we clear up the misunderstanding (I'm not sure whether you
misunderstood the patch, or I misunderstood your response), I'll be glad
to put it in.

> So - maybe we can have the patch a bit cleaner by adding
> a flag to add_type_attribute saying we only want it if it's
> different from that already present (on the specification DIE)?

That's exactly what I meant completing_type_p to check.  Do you mean it
should be stricter, do it more cheaply, or what?
Richard Biener Sept. 13, 2019, 12:53 p.m. UTC | #5
On Fri, Sep 13, 2019 at 1:32 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Sep 12, 2019, Richard Biener <richard.guenther@gmail.com> wrote:
>
> > Your new predicate looks a bit excessive here given it builds the type
> > again?
>
> Err, there seems to be some misunderstanding here.  The predicate avoids
> outputting a type for the definition if it's the same DIE that would go
> in the specification.  Now, when it's a different DIE, sometimes it
> might still refer to the same type, as in array-2.C, but I think that's
> not just acceptable, it's desirable, for it reflects the different ways
> used to denote the same type.

Yes.

> Before posting the patch, I added an inform() to the case in which
> completing_type_p would return true (bringing about a debug info
> change), and reviewed all of the messages in a bootstrap.  The only ones
> that weren't just adding array element count were those that inspired
> array-2.C and array-3.C.
>
> > So I'm obviously fine with your patch and I guess if we independently
> > arrive at this solution that answers my question what "correct DWARF"
> > is by a majority decision ;)
>
> Good.  Once we clear up the misunderstanding (I'm not sure whether you
> misunderstood the patch, or I misunderstood your response), I'll be glad
> to put it in.
>
> > So - maybe we can have the patch a bit cleaner by adding
> > a flag to add_type_attribute saying we only want it if it's
> > different from that already present (on the specification DIE)?
>
> That's exactly what I meant completing_type_p to check.  Do you mean it
> should be stricter, do it more cheaply, or what?

I meant to do it more cheaply, also the name completing_type_p is
misleading IMHO since it simply checks (re-)creating the type DIE
will yield a different one.  In case the FE would be so stupid to
first dwarf2out_early_global_decl with a complete type and then
later with an incomplete we'd still place a type DIE with the now
incomplete type on the specification DIE ...

In my view what the FE does is fishy (re-use the DECL node but
replace its type).  What we'd need here is probably the
merge_decls equivalent in the dwarf2out API, but not exactly
sure how that would look like (create a new variable DIE and
splice out "common" parts into an abstract DIE used by
both the old and the new DIE?)

Anyhow, I arrived at the same solution - if the type DIE we
created in gen_decl_die is different than the one already
present and we're creating a specification, put that type
on the new DIE, "shadowing" the declaration ones.

Richard.


>
> --
> Alexandre Oliva, freedom fighter  he/him   https://FSFLA.org/blogs/lxo
> Be the change, be Free!       FSF.org & FSF Latin America board member
> GNU Toolchain Engineer                        Free Software Evangelist
> Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara

Patch
diff mbox series

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index c359c2d4af981..ad533c14d2480 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -23687,6 +23687,33 @@  local_function_static (tree decl)
     && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL;
 }
 
+/* Return true iff DECL completes (overrides) the type of OLD_DIE
+   within CONTEXT_DIE.  */
+
+static bool
+completing_type_p (tree decl, dw_die_ref old_die, dw_die_ref context_die)
+{
+  tree type = TREE_TYPE (decl);
+  int cv_quals;
+
+  if (decl_by_reference_p (decl))
+    {
+      type = TREE_TYPE (type);
+      cv_quals = TYPE_UNQUALIFIED;
+    }
+  else
+    cv_quals = decl_quals (decl);
+
+  dw_die_ref type_die = modified_type_die (type,
+					   cv_quals | TYPE_QUALS (type),
+					   false,
+					   context_die);
+
+  dw_die_ref old_type_die = get_AT_ref (old_die, DW_AT_type);
+
+  return type_die != old_type_die;
+}
+
 /* Generate a DIE to represent a declared data object.
    Either DECL or ORIGIN must be non-null.  */
 
@@ -23939,7 +23966,9 @@  gen_variable_die (tree decl, tree origin, dw_die_ref context_die)
 	  && !DECL_ABSTRACT_P (decl_or_origin)
 	  && variably_modified_type_p (TREE_TYPE (decl_or_origin),
 				       decl_function_context
-							(decl_or_origin))))
+				       (decl_or_origin)))
+      || (old_die && specialization_p
+	  && completing_type_p (decl_or_origin, old_die, context_die)))
     {
       tree type = TREE_TYPE (decl_or_origin);
 
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-0.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-0.C
new file mode 100644
index 0000000000000..a3458bd0d32a4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-0.C
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+struct S
+{
+  static int array[42];
+};
+
+int S::array[42];
+
+/* Verify that we get only one DW_TAG_subrange_type with a
+   DW_AT_upper_bound.  */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-1.C
new file mode 100644
index 0000000000000..e8fd6f8ffea56
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-1.C
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+struct S
+{
+  static int array[];
+};
+
+int S::array[42];
+
+/* Verify that we get two DW_TAG_subrange_type, only one of which with
+   a DW_AT_upper_bound.  */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 4 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-2.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-2.C
new file mode 100644
index 0000000000000..dd17812043898
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-2.C
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+struct S
+{
+  typedef int i_t;
+  static i_t array[42];
+};
+
+int S::array[42];
+
+/* Verify that we get two DW_TAG_subrange_type (plus abbrev), and two
+   DW_AT_upper_bound, because a different symbolic name is used for
+   the array element type.  */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 3 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 2 } } */
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-3.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-3.C
new file mode 100644
index 0000000000000..8db6133b76585
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-3.C
@@ -0,0 +1,20 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+struct S
+{
+  S() {}
+  ~S() {}
+  static const S array[2];
+};
+
+const S S::array[2] = { S(), S() };
+
+/* Verify that we get only one DW_TAG_subrange_type (plus the abbrev),
+   and one DW_AT_upper_bound (non-abbrev), because the array
+   definition loses the readonly wrapper for the array type because of
+   the dynamic initializers.  The const types are 4: S, S*, int, and
+   S[4], plus the abbrev.  A const version of S[4] doesn't make sense,
+   but we output it.  */
+/* { dg-final { scan-assembler-times " DW_TAG_const_type" 5 } } */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/array-4.C b/gcc/testsuite/g++.dg/debug/dwarf2/array-4.C
new file mode 100644
index 0000000000000..6b3f546c1b583
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/array-4.C
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+struct S
+{
+  S() {}
+  ~S() {}
+};
+
+const S array[2] = { S(), S() };
+
+/* Like array-3, but with a non-member array without a separate
+   declaration, to check that we don't issue the nonsensical
+   DW_TAG_const_type used by the member array declaration there.  */
+/* { dg-final { scan-assembler-times " DW_TAG_const_type" 4 } } */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c
new file mode 100644
index 0000000000000..b06392e04a2db
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-0.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+extern int array[42];
+
+int array[42];
+
+/* Verify that we get only one DW_TAG_subtrange_type (plus abbrev),
+   with a DW_AT_upper_bound.  */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c
new file mode 100644
index 0000000000000..ad8f466ef08ad
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-1.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+extern int array[];
+
+int array[42];
+
+/* Verify that we get two DW_TAG_subtrange_type (each with an abbrev),
+   but only one DW_AT_upper_bound.  */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 4 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c
new file mode 100644
index 0000000000000..5d1606f0889fc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-2.c
@@ -0,0 +1,8 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+int array[42];
+
+/* Verify that we get only one DW_TAG_subtrange_type (plus abbrev)
+   with DW_AT_upper_bound.  */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c b/gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c
new file mode 100644
index 0000000000000..077a62ef9a3bf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/array-3.c
@@ -0,0 +1,8 @@ 
+/* { dg-do compile } */
+/* { dg-options "-gdwarf-2 -dA" } */
+int array[] = { 0, 1, 2 };
+
+/* Verify that we get only one DW_TAG_subtrange_type (plus abbrev)
+   with DW_AT_upper_bound.  */
+/* { dg-final { scan-assembler-times " DW_TAG_subrange_type" 2 } } */
+/* { dg-final { scan-assembler-times " DW_AT_upper_bound" 1 } } */