[Bug debug/97599] New: [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[Bug debug/97599] New: [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

            Bug ID: 97599
           Summary: [8/9/10/11 Regression] missing unspecified_parameters
                    DIE in DWARF for functions with variable arguments
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
                CC: aldyh at gcc dot gnu.org, woodard at redhat dot com
        Depends on: 71855
  Target Milestone: ---
            Target: x86-64-Linux

+++ This bug was initially created as a clone of Bug #71855 +++

Since PR71855, we emit bad debug info e.g. for
#include <stdarg.h>

static __attribute__((noinline)) int
foo (int unused, int args, ...)
{
  va_list ap;
  int ret = 0;
  va_start (ap, args);
  if (args)
    ret = va_arg (ap, int);
  va_end (ap);
  return ret;
}

int
bar (int x)
{
  return foo (0, 0) + foo (0, 1, 1) + foo (0, 1, 3) + foo (0, 0) + foo (0, 0,
4);
}

Also, with LTO:
#include <stdarg.h>

void
foo (int args, ...)
{
  va_list ap;
  va_start (ap, args);
  va_end (ap);
}
gcc -shared -g -fpic -flto test.c
As written in https://bugzilla.redhat.com/show_bug.cgi?id=1891787 , it seems it
isn't enough to rely on the DW_TAG_unspecified_parameters DIE being present
among the abstract origin DIE of the DW_TAG_subprogram's children; the debug
info consumer can't know if the clone of that original function is really still
variadic, or if e.g. the variadic-ness has been removed.

For the non-LTO case, it is I think fairly easy to do, subr_die != old_die
should be true if it is a fresh new DIE, but I'm afraid for in_lto_p we don't
really know easily if it is a fresh new DIE or not.

As we have nothing to lookup in the hash tables to find out cheaply if it is
there or not already, the following is a painful way of doing that.

Thoughts on that?

--- gcc/dwarf2out.c.jj  2020-10-26 10:53:56.000000000 +0100
+++ gcc/dwarf2out.c     2020-10-27 17:56:07.249711468 +0100
@@ -23365,6 +23365,35 @@ gen_subprogram_die (tree decl, dw_die_re
          else if (DECL_INITIAL (decl) == NULL_TREE)
            gen_unspecified_parameters_die (decl, subr_die);
        }
+      else if (prototype_p (TREE_TYPE (decl))
+              && stdarg_p (TREE_TYPE (decl)))
+       {
+         /* Emit DW_TAG_unspecified_parameters even in late DWARF,
+            but ensure to do it only when it isn't present already.
+            If subr_die != old_die, this is a new DIE to which it
+            should be added, otherwise if in_lto_p, we don't really
+            know if it is a freshly created one or something older,
+            so look it up.  */
+         bool emit = subr_die != old_die;
+         if (!emit && in_lto_p)
+           {
+             dw_die_ref c;
+             emit = true;
+             FOR_EACH_CHILD (subr_die, c,
+                             do
+                               {
+                                 if (c->die_tag
+                                     == DW_TAG_unspecified_parameters)
+                                   {
+                                     emit = false;
+                                     break;
+                                   }
+                               }
+                             while (0));
+           }
+         if (emit)
+           gen_unspecified_parameters_die (decl, subr_die);
+       }
     }

   if (subr_die != old_die)


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71855
[Bug 71855] duplicate unspecified_parameters DIE in DWARF for functions with
variable arguments

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5
                 CC|                            |rguenth at gcc dot gnu.org

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think the main confusion is as to how we represent IPA clones in debug,
namely making them have an abstract origin to the cloned function even though
their signatures do not match.  In reality we're creating a new function
[entry] with a different signature.  I think a better representation for
a function clone would be

 <1><ab>: Abbrev Number: 10 (DW_TAG_subprogram)
          DW_AT_artificial : 1
<clone signature>

 <2>      DW_TAG_inlined_subroutine <original function DIE>

plus the parameter mapping.  Thus represent a clone as what it really is,
a new function calling the old one with appropriate parameters.

This doesn't work for the function splitting tail though (IIRC we had another
PR about that).

The question is whether we need to have any debug for the formal parameters
of clones or if it is enough to appropriately specify the
DW_TAG_inlined_subroutine (that's where users would expect breakpoints as
well).

So in the end I don't believe the current situation is "fixable" since we
cannot distinguish clones from abstract vs. concrete instances in the
DWARF itself.  And the abstract origin we have on the tree level is just
misleading.

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Implementation-wise we'd have to put a wrapping inline BLOCK in function clones
plus the appropriate inlined PARM_DECLs / debug stmts to initialize the
call parameters.  And then simply forgo with setting DECL_ABSTRACT_ORIGIN
and make the clone DECL_ARTIFICIAL.

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
           Keywords|                            |wrong-debug

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |mark at gcc dot gnu.org,
                   |                            |palves at redhat dot com

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, let's first discuss how we want to represent these in DWARF, I've added
further folks on CC, thoughts on that?

If the function is the same rather than some clone with possibly changed
calling convention, does your reading of DWARF suggest that each
DW_TAG_subprogram should have its own DW_TAG_unspecified_parameters child, or
e.g. DW_AT_calling_convention attribute, or is that something that can be
inherited through DW_AT_abstract_origin and only overridden if different from
the abstract origin?

Then there is the question what to do with function clones, which, while can
have different arguments and calling convention, but otherwise they still
represent the whole function.

And finally, the question of just outlined regions of functions, whether it is
the outlined part of partial inlining, OpenMP/OpenACC etc. outlined regions
etc.
At least for this third set I think we really want some attribute that says
they are partial (e.g. implementations should expect their start to be the
start of the function) and with DW_AT_abstract_origin pointing to something
more useful (e.g. the DW_TAG_lexical_block they are representing?).

As for the implementation of this patch if DW_TAG_unspecified_parameters and/or
DW_AT_calling_convention aren't inherited through DW_AT_abstract_origin by
consumers, I think rather than what I'm doing in the above patch for LTO we
could immediately after old_die = lookup_die (decl); remember for in_lto_p if
old_die->die_child was NULL, that would stand for a freshly created DIE and
then could be used as if (subr_die != old_die || old_die_was_empty) to decide
whether to add DW_TAG_unspecified_parameters or not.

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Another version of the patch:
--- gcc/dwarf2out.c.jj  2020-10-27 18:38:00.001979404 +0100
+++ gcc/dwarf2out.c     2020-10-28 10:52:29.618796758 +0100
@@ -22756,6 +22756,7 @@ gen_subprogram_die (tree decl, dw_die_re
   tree origin = decl_ultimate_origin (decl);
   dw_die_ref subr_die;
   dw_die_ref old_die = lookup_decl_die (decl);
+  bool old_die_had_no_children = false;

   /* This function gets called multiple times for different stages of
      the debug process.  For example, for func() in this code:
@@ -22846,6 +22847,9 @@ gen_subprogram_die (tree decl, dw_die_re
   if (old_die && declaration)
     return;

+  if (in_lto_p && old_die && old_die->die_child == NULL)
+    old_die_had_no_children = true;
+
   /* Now that the C++ front end lazily declares artificial member fns, we
      might need to retrofit the declaration into its class.  */
   if (!declaration && !origin && !old_die
@@ -23365,6 +23369,10 @@ gen_subprogram_die (tree decl, dw_die_re
          else if (DECL_INITIAL (decl) == NULL_TREE)
            gen_unspecified_parameters_die (decl, subr_die);
        }
+      else if ((subr_die != old_die || old_die_had_no_children)
+              && prototype_p (TREE_TYPE (decl))
+              && stdarg_p (TREE_TYPE (decl)))
+       gen_unspecified_parameters_die (decl, subr_die);
     }

   if (subr_die != old_die)

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aoliva at gcc dot gnu.org

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Adding Alex for debug-stmt / inline-entry etc. issues.

Note that clones are materialized only during LTRANS where for example we
have no way of generating "nice" DWARF for a clone of a C++ class method.
What we _can_ do is create DWARF for an aritificial wrapper function
whose body contains an inline copy of said C++ class method (plus required
argument marshalling).

User (dwarf consumer) expectation is another issue, breakpoints on a
function should work and also break on clones.

User expectation for OMP outlines is less clear to me.

For split functions we shouldn't get two breakpoints (inlined header
plus tail) but only break on the header.  Ideally the frame of the
tail wouldn't even be visible...  Note that we do not necessarily
outline full scopes in the splitting case, so even abstract origins
to scopes might not be a good match there.  Maybe don't try to represent
the call to the tail as call but handle it as a jump like hot/cold
parts of a function (though technically there is a frame).

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:2873c8af66e1248734bb638a49e6bc53f5e45382

commit r11-5028-g2873c8af66e1248734bb638a49e6bc53f5e45382
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Nov 14 09:14:19 2020 +0100

    dwarf2: Emit DW_TAG_unspecified_parameters even in late DWARF [PR97599]

    Aldy's PR71855 fix avoided emitting multiple redundant
    DW_TAG_unspecified_parameters sub-DIEs of a single DIE by restricting
    it to early dwarf only.  That unfortunately means if we need to emit
    another DIE for the function (whether it is for LTO, or e.g. because of
    IPA cloning), we don't emit DW_TAG_unspecified_parameters, it remains
    solely in the DW_AT_abstract_origin's referenced DIE.
    But DWARF consumers don't really use DW_TAG_unspecified_parameters
    from there, like we duplicate DW_TAG_formal_parameter sub-DIEs even in the
    clones because either they have some more specific location, or e.g.
    a function clone could have fewer or different argument types etc.,
    they need to assume that originally stdarg function isn't later stdarg etc.
    Unfortunately, while for DW_TAG_formal_parameter sub-DIEs, we can use the
    hash tabs to look the PARM_DECLs if we already have the DIEs, for
    DW_TAG_unspecified_parameters we don't have an easy way to look it up.

    The following patch handles it by trying to figure out if we are creating a
    fresh new DIE (in that case we add DW_TAG_unspecified_parameters if it is
    stdarg), or if gen_subprogram_die is called again on an pre-existing DIE
    to fill in some further details (then it will not touch it).

    Except for lto, subr_die != old_die would be good enough, but unfortunately
    for LTO the new DIE that will refer to early dwarf created DIE is created
    on the fly during lookup_decl_die.  So the patch tracks if the DIE has
    no children before any children are added to it.

    2020-11-14  Jakub Jelinek  <jakub@redhat.com>

            PR debug/97599
            * dwarf2out.c (gen_subprogram_die): Call
            gen_unspecified_parameters_die even if not early dwarf, but only
            if subr_die is a newly created DIE.

[Bug debug/97599] [8/9/10/11 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:812258b07c1a80e8dc79a0beb3ec0e17be62f5e5

commit r10-9076-g812258b07c1a80e8dc79a0beb3ec0e17be62f5e5
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Nov 14 09:14:19 2020 +0100

    dwarf2: Emit DW_TAG_unspecified_parameters even in late DWARF [PR97599]

    Aldy's PR71855 fix avoided emitting multiple redundant
    DW_TAG_unspecified_parameters sub-DIEs of a single DIE by restricting
    it to early dwarf only.  That unfortunately means if we need to emit
    another DIE for the function (whether it is for LTO, or e.g. because of
    IPA cloning), we don't emit DW_TAG_unspecified_parameters, it remains
    solely in the DW_AT_abstract_origin's referenced DIE.
    But DWARF consumers don't really use DW_TAG_unspecified_parameters
    from there, like we duplicate DW_TAG_formal_parameter sub-DIEs even in the
    clones because either they have some more specific location, or e.g.
    a function clone could have fewer or different argument types etc.,
    they need to assume that originally stdarg function isn't later stdarg etc.
    Unfortunately, while for DW_TAG_formal_parameter sub-DIEs, we can use the
    hash tabs to look the PARM_DECLs if we already have the DIEs, for
    DW_TAG_unspecified_parameters we don't have an easy way to look it up.

    The following patch handles it by trying to figure out if we are creating a
    fresh new DIE (in that case we add DW_TAG_unspecified_parameters if it is
    stdarg), or if gen_subprogram_die is called again on an pre-existing DIE
    to fill in some further details (then it will not touch it).

    Except for lto, subr_die != old_die would be good enough, but unfortunately
    for LTO the new DIE that will refer to early dwarf created DIE is created
    on the fly during lookup_decl_die.  So the patch tracks if the DIE has
    no children before any children are added to it.

    2020-11-14  Jakub Jelinek  <jakub@redhat.com>

            PR debug/97599
            * dwarf2out.c (gen_subprogram_die): Call
            gen_unspecified_parameters_die even if not early dwarf, but only
            if subr_die is a newly created DIE.

    (cherry picked from commit 2873c8af66e1248734bb638a49e6bc53f5e45382)

[Bug debug/97599] [8/9 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-11-25
            Summary|[8/9/10/11 Regression]      |[8/9 Regression] missing
                   |missing                     |unspecified_parameters DIE
                   |unspecified_parameters DIE  |in DWARF for functions with
                   |in DWARF for functions with |variable arguments
                   |variable arguments          |
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.3+ and 11.1+ so far.

[Bug debug/97599] [8/9 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:7af5ea39232b04cb4a392cc869aaa7d66d9fabf8

commit r9-9393-g7af5ea39232b04cb4a392cc869aaa7d66d9fabf8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Nov 14 09:14:19 2020 +0100

    dwarf2: Emit DW_TAG_unspecified_parameters even in late DWARF [PR97599]

    Aldy's PR71855 fix avoided emitting multiple redundant
    DW_TAG_unspecified_parameters sub-DIEs of a single DIE by restricting
    it to early dwarf only.  That unfortunately means if we need to emit
    another DIE for the function (whether it is for LTO, or e.g. because of
    IPA cloning), we don't emit DW_TAG_unspecified_parameters, it remains
    solely in the DW_AT_abstract_origin's referenced DIE.
    But DWARF consumers don't really use DW_TAG_unspecified_parameters
    from there, like we duplicate DW_TAG_formal_parameter sub-DIEs even in the
    clones because either they have some more specific location, or e.g.
    a function clone could have fewer or different argument types etc.,
    they need to assume that originally stdarg function isn't later stdarg etc.
    Unfortunately, while for DW_TAG_formal_parameter sub-DIEs, we can use the
    hash tabs to look the PARM_DECLs if we already have the DIEs, for
    DW_TAG_unspecified_parameters we don't have an easy way to look it up.

    The following patch handles it by trying to figure out if we are creating a
    fresh new DIE (in that case we add DW_TAG_unspecified_parameters if it is
    stdarg), or if gen_subprogram_die is called again on an pre-existing DIE
    to fill in some further details (then it will not touch it).

    Except for lto, subr_die != old_die would be good enough, but unfortunately
    for LTO the new DIE that will refer to early dwarf created DIE is created
    on the fly during lookup_decl_die.  So the patch tracks if the DIE has
    no children before any children are added to it.

    2020-11-14  Jakub Jelinek  <jakub@redhat.com>

            PR debug/97599
            * dwarf2out.c (gen_subprogram_die): Call
            gen_unspecified_parameters_die even if not early dwarf, but only
            if subr_die is a newly created DIE.

    (cherry picked from commit 812258b07c1a80e8dc79a0beb3ec0e17be62f5e5)

[Bug debug/97599] [8/9 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:1a89fa601b0335cedb8ed415f1ef209cf1544002

commit r8-10863-g1a89fa601b0335cedb8ed415f1ef209cf1544002
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Nov 14 09:14:19 2020 +0100

    dwarf2: Emit DW_TAG_unspecified_parameters even in late DWARF [PR97599]

    Aldy's PR71855 fix avoided emitting multiple redundant
    DW_TAG_unspecified_parameters sub-DIEs of a single DIE by restricting
    it to early dwarf only.  That unfortunately means if we need to emit
    another DIE for the function (whether it is for LTO, or e.g. because of
    IPA cloning), we don't emit DW_TAG_unspecified_parameters, it remains
    solely in the DW_AT_abstract_origin's referenced DIE.
    But DWARF consumers don't really use DW_TAG_unspecified_parameters
    from there, like we duplicate DW_TAG_formal_parameter sub-DIEs even in the
    clones because either they have some more specific location, or e.g.
    a function clone could have fewer or different argument types etc.,
    they need to assume that originally stdarg function isn't later stdarg etc.
    Unfortunately, while for DW_TAG_formal_parameter sub-DIEs, we can use the
    hash tabs to look the PARM_DECLs if we already have the DIEs, for
    DW_TAG_unspecified_parameters we don't have an easy way to look it up.

    The following patch handles it by trying to figure out if we are creating a
    fresh new DIE (in that case we add DW_TAG_unspecified_parameters if it is
    stdarg), or if gen_subprogram_die is called again on an pre-existing DIE
    to fill in some further details (then it will not touch it).

    Except for lto, subr_die != old_die would be good enough, but unfortunately
    for LTO the new DIE that will refer to early dwarf created DIE is created
    on the fly during lookup_decl_die.  So the patch tracks if the DIE has
    no children before any children are added to it.

    2020-11-14  Jakub Jelinek  <jakub@redhat.com>

            PR debug/97599
            * dwarf2out.c (gen_subprogram_die): Call
            gen_unspecified_parameters_die even if not early dwarf, but only
            if subr_die is a newly created DIE.

    (cherry picked from commit 812258b07c1a80e8dc79a0beb3ec0e17be62f5e5)

[Bug debug/97599] [8/9 Regression] missing unspecified_parameters DIE in DWARF for functions with variable arguments

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97599

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.