[PATCH V3 0/5] Support for the CTF and BTF debug formats

[Richard Biener]

On Tue, Mar 9, 2021 at 12:29 PM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> Hi Richard.
> Thanks for looking at this :)
>
> [Today I sent a V4 of the series containing a couple of fixes to the BTF
>  code.  It doesn't contain big changes so the discussion below still
>  applies.]
>
> >> In turn, debug_format_do_cu traverses the tree of DIEs passed to it calling
> >> ctf_do_die on them.
> >>
> >> This conforms the debug format API:
> >>
> >>    FOO_debug_init ()
> >>      Initialize the debug format FOO.
> >>
> >>    FOO_debug_finalize (FILENAME)
> >>      Possibly write out, cleanup and finalization for debug format FOO.
> >>
> >>    FOO_do_die (DIE)
> >>      Process the given DIE.
> >>
> >> Note how the emission of DWARF is interrupted after that point, if no
> >> DWARF was requested by the user.
> >
> > I suppose since we're now supposed to write C++ the above fits
> > a RAII style
> >
> >  if (ctf_debug_info_level > ...)
> >   {
> >     ctf_debug ctf (filename);
> >     ctf.do_cu (comp_unit_die ());
> >     for (...)
> >        ctf.do_cu (node->die);
> >   }
> >
> > but then I have no strong feelings about this.
>
> We could turn it into a class, sure.
>
> >> dwarf2out - dwarf2ctf
> >> =====================
> >>
> >> The functions ctf_debug_init, ctf_do_die and ctf_debug_finalize, that
> >> implement the API described above, are all in gcc/dwarf2ctf.c.
> >>
> >> Obviously, these routines need access to the dwarf DIE data
> >> structures, and several functions which are defined in dwarf2out.[ch],
> >> many (most?) of which are private to that file: dw_die_ref, get_AT,
> >> etc.
> >>
> >> Therefore, in this implementation we opted by writing dwarf2ctf.c in a
> >> way it can just be #included in dwarf2ctf.c.
> >>
> >> A question remains: would it be better to abstract these types and
> >> functions in an API in dwarf2out.h?
> >
> > I think that refactoring the big dwarf2out.c would be indeed interesting.
> > Not only exposing more details from dwarf2out.h (though we could start
> > there).  One of the reason it's all private to dwarf2out.c is likely
> > optimization (somewhat moot when you LTO GCC nowadays).
> >
> > So one interesting question is what's the API details CTF/BTF need?
>
> In dwarf2ctf we use the following stuff defined in dwarf2out.c, which
> are functions providing access to the DIE attributes:
>
> - get_AT
> - get_AT_ref
> - get_AT_string
> - AT_class
> - AT_unsigned
>
> So what if we start by adding non-static version of these functions and
> export them in dwarf2out.h:
>
> - dw_die_get_AT
> - dw_die_get_AT_ref
> - dw_die_get_AT_string
> - dw_die_AT_class
> - dw_die_AT_unsigned

The latter two would be dw_AT_class and dw_AT_unsigned
(operate on attributes, not on dies).

There's the option to move all ^AT_* functions to some
dwarf2attrs.h as inline functions (inline in a way they get
comdat across modules) and/or make them member
functions of dw_attr_node (but that's bigger refactoring
unless we keep function-style wrappers in dwarf2out.c).

Having two names for the same thing doesn't look too appealing
in the long term at least.  Just exporting the non-mangled
current names from some dwarf2int.h header might also work
to avoid the .c #include.

> ?
>
> > We might move data structures and basic accessors to
> > dwarf2impl.{c,h} (as opposed to the already existing dwarf2.h).
> > Or some better name (or as said, simply keep it in dwarf2out.c for now).
> >
> > Including dwarf2ctf.c is a bit ugly.
>
> It is definitely ugly yes, but we didn't want to jump factoring an API
> for dwarf2out without discussing it here first :)

Yes, much appreciated.  I don't think it's a blocker for CTF, but of course.

> >> Command line options for debug formats
> >> ======================================
> >>
> >> This implementation adds the following command-line options to select the
> >> emission of CTF and BTF:
> >>
> >>      -gctf[123]
> >>      -gbtf[123]
> >>
> >> These options mimic the -g[123...] options for DWARF.
> >
> > Do -gcft and -gdwarf mix?  That is, can I have both in the same ELF
> > file?  Since that doesn't work for -gdwarf -gstabs for example I wonder
> > if we need to make this somehow different?
>
> The idea is for them to mix, yes.
>
> >> This involved adding new entries for debug_info_type:
> >>
> >>      CTF_DEBUG            - Write CTF debug info.
> >>      BTF_DEBUG            - Write BTF debug info.
> >>      CTF_AND_DWARF2_DEBUG - Write both CTF and DWARF info.
> >>
> >> Doing this, we just followed the trend initiated by vmsdbgout.c, which
> >> added VMS_DEBUG and VMS_AND_DWARF2_DEBUG.
> >>
> >> This approach is not very good, because debug_info_type was designed
> >> to cover different debug hook implementations; debug formats, in
> >> contrast, are a different thing.
> >>
> >> This translates to problems and fragile behavior:
> >>
> >> - Everywhere write_symbols is checked we have to expand the logic to
> >>   take the CTF values into account.  You can see that is the case in
> >>   this patch series.  This is very fragile and doesn't scale well: we
> >>   are most probably missing some checks.
> >
> > Would be nice to abstract most of the checks.  Like if the check
> > is for "do I need to generate DWARF DIEs" then use
> > need_dwarf_dies (), if it is for "do I want to output DWARF2 debug" then
> > check output_dwarf ().  I see 64 places that check write_symbols,
> > most in vmsdbgout.c ...
> >
> > Inventing predicates for the existing uses with your new uses in mind
> > could make future changes easier at least.
>
> Makes sense, we will look into abstracting these checks in suitable
> predicates.  That certainly will improve things :)

Thanks.

> >> - Backends select what debug hooks to use by defining constants like
> >>   DWARF2_DEBUGGING_INFO.  Since the new debug formats are based on the
> >>   DWARF debug hooks, that is the constant to define by the backends
> >>   wanting to support DWARF + debug infos.
> >>
> >>   However, some backends may want to use one of the debug formats by
> >>   default, i.e. for -g.  This is the case of the BPF backend, that
> >>   needs to generate BTF instead of DWARF.  Currently, there is no way
> >>   to specify this.
> >
> > It probably should define BTF_DEBUGGING_INFO and that should
> > enable parts guarded with DWARF2_DEBUGGING_INFO as well.
>
> Ok, so from a backend perspective there won't be any difference between
> formats based on the DWARF debug hooks and (to deprecate) formats using
> their own set of debug hooks...

Well, it's a topic that deserves extra thoughts when we actually start
deprecating things ...

Richard.